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

[Ladi Prosek]

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;