[ipxe-devel] [PATCH 0/2] [vlan] Support 802.1Q VLAN 0 priority tagging

[Ladi Prosek]

ping

On Thu, Apr 21, 2016 at 2:48 PM, Ladi Prosek <lprosek at redhat.com> wrote:
> On Thu, Apr 21, 2016 at 12:18 PM, Michael Brown <mcb30 at ipxe.org> wrote:
>> On 21/04/16 09:36, Ladi Prosek wrote:
>>>
>>> Following up on our IRC conversation, here's what I get when playing
>>> with a Linux kernel with no VLAN support (!CONFIG_VLAN_8021Q &&
>>> !CONFIG_VLAN_8021Q_MODULE).
>>>
>>> Receiving VLAN 0 packets works. The tag is stripped and the packet
>>> passes through the networking stack as usual. Here's a VLAN 0 ping
>>> request and response, 10.34.1.105 is running the VLAN-less kernel:
>>>
>>>    1   0.000000  10.34.1.136 -> 10.34.1.105  ICMP 50 Echo (ping)
>>> request  id=0x03e8, seq=0/0, ttl=255
>>>    2   0.000080  10.34.1.105 -> 10.34.1.136  ICMP 46 Echo (ping) reply
>>>    id=0x03e8, seq=0/0, ttl=64 (request in 1)
>>>
>>> Note that the response is 4 bytes shorter because it doesn't have the
>>> 802.1Q header.
>>
>>
>> Fantastic; thank you for doing this testing.  I am suitably convinced that
>> we ought to treat packets on VLAN 0 in the same way as packets with no VLAN
>> tag.
>>
>> Unfortunately, this is going to be difficult to do.  Your patch handles
>> packets that get processed by iPXE, but doesn't help with packets passed via
>> the UNDI API.
>>
>> We do allow for a PXE NBP to use the UNDI API on a VLAN device.  Only the
>> selected VLAN device gets frozen (via netdev_freeze()).  Packets arriving on
>> the unfrozen trunk device are processed by iPXE as normal: the VLAN tag is
>> stripped and the packet is requeued on the appropriately frozen VLAN device,
>> whence pxenv_undi_isr() can dequeue it.
>>
>> This model breaks if we have to handle VLAN 0.  There is no separate VLAN
>> device, so the trunk device itself will be frozen.  The packets will
>> therefore end up being passed out directly via PXENV_UNDI_ISR; the VLAN tag
>> will never be inspected or removed.  This will break most PXE NBPs.
>>
>> I can't immediately see a clean way to do this.  Potentially we would need
>> to parse and strip the VLAN header at the point that netdev_rx() is called,
>> instead of treating VLANs as a net_protocol.
>
> Interesting, thanks. Why not strip it inside pxenv_undi_isr then? This
> function already peeks into the network layer protocol so it doesn't
> feel that unnatural to special case 802.1Q. Maybe without the goto :)
> vlan_strip would be the guts of vlan_rx factored out to a separate
> function doing pretty much everything except re-enquing the packet.
>
> @@ -967,6 +968,7 @@ static PXENV_EXIT_t pxenv_undi_isr ( struct
> s_PXENV_UNDI_ISR *undi_isr ) {
>
>                 /* Strip link-layer header */
>                 ll_protocol = pxe_netdev->ll_protocol;
> +       strip_ll_header:
>                 if ( ( rc = ll_protocol->pull ( pxe_netdev, iobuf, &ll_dest,
>                                                 &ll_source, &net_proto,
>                                                 &flags ) ) != 0 ) {
> @@ -978,6 +980,13 @@ static PXENV_EXIT_t pxenv_undi_isr ( struct
> s_PXENV_UNDI_ISR *undi_isr ) {
>
>                 /* Determine network-layer protocol */
>                 switch ( net_proto ) {
> +               case htons ( ETH_P_8021Q ):
> +                       if ( ( rc = vlan_strip ( pxe_netdev, iobuf, &ll_dest,
> +                                                &ll_source ) ) != 0 ) {
> +                               undi_isr->Status = PXENV_STATUS_FAILURE;
> +                               return PXENV_EXIT_FAILURE;
> +                       }
> +                       goto strip_ll_header;
>                 case htons ( ETH_P_IP ):
>                         net_protocol = &ipv4_protocol;
>                         prottype = P_IP;