netdev - Re: [PATCH net-next] rocker: check for BRIDGE_FLAGS

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: <CAE4R7bAaLxCebc7cuYB9NQ_Duo8a6LvxWDwWOD9Wn8ROWjvDGw@mail.gmail.com>
Date:	Fri, 6 Mar 2015 01:52:50 -0800
From:	Scott Feldman <sfeldma@...il.com>
To:	roopa <roopa@...ulusnetworks.com>
Cc:	Jiri Pirko <jiri@...nulli.us>, Netdev <netdev@...r.kernel.org>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH net-next] rocker: check for BRIDGE_FLAGS_SELF in bridge
 setlink handler

On Thu, Mar 5, 2015 at 12:43 PM, roopa <roopa@...ulusnetworks.com> wrote:
> On 3/5/15, 12:06 PM, Scott Feldman wrote:
>>
>> On Thu, Mar 5, 2015 at 6:55 AM, roopa <roopa@...ulusnetworks.com> wrote:
>>>
>>> On 3/5/15, 12:02 AM, Jiri Pirko wrote:
>>>>
>>>> Wed, Mar 04, 2015 at 09:51:20AM CET, roopa@...ulusnetworks.com wrote:
>>>>>
>>>>> On 3/3/15, 11:02 PM, Scott Feldman wrote:
>>
>> [cut]
>>
>>>>>> Rocker setlink wasn't broken prior to the NETIF_F_HW_SWITCH_OFFLOAD
>>>>>> patches.
>>>>>>    Now it is,
>>>>>
>>>>> sure, I can submit a patch to remove the flag on rocker ports if thats
>>>>> what
>>>>> you prefer.
>>>>
>>>> I don't believe that Scott prefers that. The offload flag is there not
>>>> only for this case, but also for many future switch offloading cases.
>>>
>>> I very well understand that.
>>>>
>>>> Rocker port have to have that port set by default. I do not really
>>>> understand why you suggest removing it...
>>>>
>>> I was just making sure scott is on board with the use of the flag.
>>> I added it on rocker ports for l2 and learning might be broken because of
>>> that. Which i am trying to fix.
>>> also, scotts l3 patches dont seem to use the flag.
>>
>> That's an oversight due to work starting on L3 patches way before
>> NETIF_F_HW_SWITCH_OFFLOAD was introduced ; I'll add checks for that
>> flag to the L3 patches.
>
> ok, i was not sure if you were planning to. I did post a comment on v2.
>>>
>>> So, if the patch in this thread is not the right way to fix it..,.i was
>>> just
>>> trying to give scott an option to remove the flag to keep l2 working and
>>> for
>>> him to bring back the flag when he is ready.
>>
>> When I'm ready for what?  I ready for it to work like it did before
>> NETIF_F_HW_SWITCH_OFFLOAD and your setlink changes went in.  ;-)
>
> well, to be fair scott, my current patch in this thread is trying to get to
> it, if you let me :)
>
>>
>> We want two sets of IFLA_PROTINFO attrs for a bridge port.   The first
>> set is the bridge side of the port, so these attrs like learning or
>> flooding control the how the bridge (software) manages the port.  The
>> second set is the device side of the port, so attrs here control how
>> the (offload) device manages the port.  The two sets are independent.
>> For example, you could have learning turned OFF on the bridge side and
>> learning turned ON on the device side.  The device side will
>> initialize its set.  The bridge will initialize its set.   The user
>> can see both sets with cmd
>>
>>      bridge -d link show.
>>
>> The user can set the bridge's attr with one of cmds:
>>
>>      bridge link set <attr> dev <port>
>>      bridge link set <attr> dev <port> master
>>
>> The user can set the device's attr with cmd:
>>
>>      bridge link set <attr> dev <port> self
>>
>> The driver setlink/getlink ops should only be called in the SELF
>> context because the driver is the SELF side of the port.  Somehow we
>> got away from that.
>
> True, If all attributes that the bridge setlink sets need to go separately
> to kernel and hardware and
>  we will never mirror them. That is not the case however. vlans is an
> example and there will be more bridge port attributes in the future.
>
>>   We talked about IFLA_AF_SPEC being handled
>> differently for VLANs.  For IFLA_PROTINFO, what do we need to get back
>> to what I have above?
>
>
> If I understand you correctly, you are saying that ndo_bridge_setlink when
> it comes to offloads
> should only be used for IFLA_PROTINFO ?

I'm saying port driver's ndo_bridge_setlink shouldn't be called unless
SELF is set.  The port driver shouldn't have to check if it's SELF,
that's my main beef with this patch.  If the app (iproute2) wants to
mirror an attribute, then it can set both MASTER and SELF.  This is
consistent with ndo_fdb ops, where MASTER targets the bridge's FDB and
SELF targets the device's FDB.
--
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