On Sun, Apr 17, 2016 at 6:14 PM, Ladi Prosek <lpro...@redhat.com> wrote:
> Thank you for the quick response.
>
> On Fri, Apr 15, 2016 at 7:03 PM, Michael Brown <mc...@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
_______________________________________________
ipxe-devel mailing list
ipxe-devel@lists.ipxe.org
https://lists.ipxe.org/mailman/listinfo.cgi/ipxe-devel
