public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: roopa <roopa@cumulusnetworks•com>
To: Scott Feldman <sfeldma@gmail•com>
Cc: Jiri Pirko <jiri@resnulli•us>, Netdev <netdev@vger•kernel.org>,
	"David S. Miller" <davem@davemloft•net>
Subject: Re: [PATCH net-next] rocker: check for BRIDGE_FLAGS_SELF in bridge setlink handler
Date: Thu, 05 Mar 2015 13:40:22 -0800	[thread overview]
Message-ID: <54F8CD46.4010507@cumulusnetworks.com> (raw)
In-Reply-To: <54F8C00C.2010303@cumulusnetworks.com>

On 3/5/15, 12:43 PM, roopa wrote:
> On 3/5/15, 12:06 PM, Scott Feldman wrote:
>> On Thu, Mar 5, 2015 at 6:55 AM, roopa <roopa@cumulusnetworks•com> wrote:
>>> On 3/5/15, 12:02 AM, Jiri Pirko wrote:
>>>> Wed, Mar 04, 2015 at 09:51:20AM CET, roopa@cumulusnetworks•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 ?
>
and...since you brought up IFLA_PROTINFO...i would like to also add 
that, .....my rocker patch in this current thread is nothing but rocker 
driver telling that "i accept IFLA_PROTINFO attributes only if the 
BRIDGE_FLAGS_SELF is set". I had the check around IFLA_PROTINFO 
initially...but since you only seem to be looking at IFLA_PROTINFO in 
the function, I put the check at the beginning of the function.

Thanks,
Roopa

  reply	other threads:[~2015-03-05 21:40 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-04  0:15 [PATCH net-next] rocker: check for BRIDGE_FLAGS_SELF in bridge setlink handler roopa
2015-03-04  4:15 ` John Fastabend
2015-03-04  7:02 ` Scott Feldman
2015-03-04  8:51   ` roopa
2015-03-04 16:24     ` Scott Feldman
2015-03-05  0:31       ` roopa
2015-03-05  8:02     ` Jiri Pirko
2015-03-05 14:55       ` roopa
2015-03-05 20:06         ` Scott Feldman
2015-03-05 20:43           ` roopa
2015-03-05 21:40             ` roopa [this message]
2015-03-06  9:52             ` Scott Feldman
2015-03-08 14:19               ` roopa
2015-03-08 23:17                 ` Scott Feldman
2015-03-09  0:20                   ` roopa
     [not found]                   ` <CAJieiUhHdXOZjWkb4s_GviLwzq5Gct-1o8xv8b-JeM46S4e-dg@mail.gmail.com>
2015-03-09  6:40                     ` Jiri Pirko
2015-03-09 15:59                       ` Arad, Ronen
2015-03-09 16:07                         ` Jiri Pirko
2015-03-10  0:51                           ` Arad, Ronen
2015-03-10  6:39                             ` Jiri Pirko
2015-03-10  8:02                               ` Arad, Ronen
2015-03-10  8:28                                 ` Jiri Pirko
2015-03-16 22:01                                   ` John Fastabend
2015-03-17  7:00                                     ` Jiri Pirko
2015-03-17 14:31                                       ` John Fastabend
2015-03-17 20:27                                         ` roopa
2015-03-18  0:16                                           ` John Fastabend
2015-03-18  6:29                                             ` roopa
2015-03-18 15:24                                               ` John Fastabend
2015-03-18 16:55                                                 ` John Fastabend
2015-03-19  5:03                                                 ` roopa
2015-03-19  5:49                                                 ` Scott Feldman
2015-03-19 13:29                                                   ` roopa
2015-03-19 13:59                                                     ` John Fastabend
     [not found]                         ` <CAJieiUhcdfGitY7rbG11Vt_Beemz8dy3=gKtvbyVLS8O0DkgNw@mail.gmail.com>
2015-03-09 23:23                           ` Roopa Prabhu
2015-03-05  8:36 ` Jiri Pirko
2015-03-05 15:01   ` roopa
2015-03-05 15:09     ` roopa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=54F8CD46.4010507@cumulusnetworks.com \
    --to=roopa@cumulusnetworks$(echo .)com \
    --cc=davem@davemloft$(echo .)net \
    --cc=jiri@resnulli$(echo .)us \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=sfeldma@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox