netdev - Re: [patch net-next-2.6 37/47] igb: do vlan cleanup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEP_g=9j3=s74_VQ6RQxVRGOOs-mVR94s31ETXJ1S5P7aQV4iQ@mail.gmail.com>
Date:	Wed, 20 Jul 2011 16:58:10 -0700
From:	Jesse Gross <jesse@...ira.com>
To:	Jiri Pirko <jpirko@...hat.com>
Cc:	netdev@...r.kernel.org, davem@...emloft.net,
	shemminger@...ux-foundation.org, eric.dumazet@...il.com,
	greearb@...delatech.com, mirqus@...il.com,
	jeffrey.t.kirsher@...el.com, jesse.brandeburg@...el.com,
	peter.p.waskiewicz.jr@...el.com, bruce.w.allan@...el.com,
	carolyn.wyborny@...el.com, donald.c.skidmore@...el.com,
	gregory.v.rose@...el.com, alexander.h.duyck@...el.com,
	john.ronciak@...el.com, e1000-devel@...ts.sourceforge.net
Subject: Re: [patch net-next-2.6 37/47] igb: do vlan cleanup

On Wed, Jul 20, 2011 at 12:10 PM, Jiri Pirko <jpirko@...hat.com> wrote:
> Wed, Jul 20, 2011 at 07:35:33PM CEST, jesse@...ira.com wrote:
>>On Wed, Jul 20, 2011 at 7:54 AM, Jiri Pirko <jpirko@...hat.com> wrote:
>>> @@ -2943,7 +2944,7 @@ static void igb_rlpml_set(struct igb_adapter *adapter)
>>>        struct e1000_hw *hw = &adapter->hw;
>>>        u16 pf_id = adapter->vfs_allocated_count;
>>>
>>> -       if (adapter->vlgrp)
>>> +       if (igb_vlan_used(adapter))
>>>                max_frame_size += VLAN_TAG_SIZE;
>>
>>There are similar issues here as with the VF driver.  I think you're
>>also confusing vlan acceleration with vlan filtering.  If no vlan
>>filters are in use but the card is in promiscuous mode, the buffer
>>will be undersized and we lose tagged packets.
>
> I'm certainly not confusing vlan accel and filtering. Here is the
> intension is the behaviour remains intact as well. I believe it's true.

I believe the underlying issue for all three of these threads is the
same, so I'll just respond to them all here.

I agree that this doesn't change the behavior of the driver but I
don't think that should be the goal.  When I originally designed this
new vlan model my intention was to eliminate a whole class of driver
bugs that I was repeatedly hitting in various forms.  In the example
above, if you run tcpdump on this device without configuring a vlan
group on it then you will see that MTU sized packets are missing
because the receive buffer was undersized.

The common theme for these problems is that they all occur in
situations where vlans are not configured on the device and the driver
does something different as a result of this.  The solution was to
prevent drivers from changing their behavior in such situations by
completely removing the concept of a vlan group from them and letting
the networking core tell them when to make the changes instead of
doing it implicitly.  That's why I don't see the fact that this change
essentially emulates the knowledge of configuring a group to be a
plus.  By the way, plenty of your other patches change the behavior of
the drivers - on any of the NICs that always enable stripping, try
running tcpdump on the interface without configuring a vlan group.
Before the change you will see that tags have disappeared and
afterwards the tags are intact.  So I think that changing the behavior
of drivers in this regard is a positive thing.

As an aside, thank you for taking the time to work on all of these
drivers.  The only reason why I'm complaining about these few drivers
is because I'd like to close the door on this class of problems, which
is finally in reach thanks to your work.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html