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] [day] [month] [year] [list]

Message-ID: <CAEP_g=-Gd0fPObLGWKTdFQLG0epiaDMMvsDZjkSMfQ-hQ-a7iw@mail.gmail.com>
Date:	Thu, 21 Jul 2011 14:45:13 -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 11:57 PM, Jiri Pirko <jpirko@...hat.com> wrote:
> Thu, Jul 21, 2011 at 01:58:10AM CEST, jesse@...ira.com wrote:
>>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.
>
>
> Okay now it's clear to me. I tried to stay with the code as much similar
> as unpatched. But I see your arguments. I will review and repost
> patches which are enabling/disabling vlan accel on add_vid/kill_vid and
> convert it to set_features.
>
> Thanks. Jesse.

All the new changes look good to me, thanks Jiri.
--
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