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

[Ladi Prosek]

On Sun, Apr 17, 2016 at 6:14 PM, Ladi Prosek <lprosek at redhat.com> wrote:
> Thank you for the quick response.
>
> On Fri, Apr 15, 2016 at 7:03 PM, Michael Brown <mcb30 at ipxe.org> wrote:
>> On 15/04/16 17:48, Michael Brown wrote:
>>>
>>> On 15/04/16 17:19, Ladi Prosek wrote:
>>>>
>>>> These patches add a small tweak to vlan_rx to make it accept
>>>> priority tagged packets. Since this should be supported even
>>>> without full VLAN support,
>>>
>>>
>>> Why must this be supported when VLAN is not enabled as a feature?
>
> I wouldn't say it must be supported but one could argue that
> conceptually this is not really related to VLANs. It reuses the same
> header yes, but networks with this kind of traffic don't necessarily
> need to have any real VLANs configured.
>
> Weighing the amount of added code to all binaries built without
> VLAN_CMD against the chances that stuff will just work for people out
> of the box (as opposed to scratching their head wondering why they
> need to enable *_CMD without actually having to use any commands), I'm
> leaning towards the it just works option.

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.

And just in case, trying to set up a real VLAN on this host fails as expected:

$ sudo vconfig add br0 5
WARNING:  Could not open /proc/net/vlan/config.  Maybe you need to
load the 8021q module, or maybe you are not using PROCFS??
ERROR: trying to add VLAN #5 to IF -:br0:-  error: Package not installed

>> I have a preference for a simpler patch such as:
>>
>> diff --git a/src/net/vlan.c b/src/net/vlan.c
>> index f515c2d..a30a0d7 100644
>> --- a/src/net/vlan.c
>> +++ b/src/net/vlan.c
>> @@ -203,6 +203,11 @@ struct net_device * vlan_find ( struct net_device
>> *trunk, unsigned int tag ) {
>>         struct net_device *netdev;
>>         struct vlan_device *vlan;
>>
>> +       /* VLAN 0 represents a priority-tagged packet on the trunk device */
>> +       if ( ! tag )
>> +               return trunk;
>> +
>> +       /* Find VLAN device */
>>         for_each_netdev ( netdev ) {
>>                 if ( netdev->op != &vlan_operations )
>>                         continue;
>
> Is it desirable to affect other callers of vlan_find? Genuine
> question, I'm not suggesting it's not. Should hermon_eth_complete_recv
> work for source->vlan == 0?
>
>> @@ -340,6 +346,12 @@ int vlan_create ( struct net_device *trunk, unsigned
>> int tag,
>>         struct vlan_device *vlan;
>>         int rc;
>>
>> +       /* VLAN 0 is not permitted */
>> +       if ( ! tag ) {
>> +               DBGC ( trunk, "VLAN %s cannot create VLAN 0\n", trunk->name
>> );
>> +               return -ENOTTY;
>> +       }
>> +
>>         /* If VLAN already exists, just update the priority */
>>         if ( ( netdev = vlan_find ( trunk, tag ) ) != NULL ) {
>>                 vlan = netdev->priv;
>>
>>
>> but I'm prepared to be swayed if there are good reasons otherwise.
>>
>> Michael
>
> Thanks!
> Ladi