netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Samudrala, Sridhar" <sridhar.samudrala@intel.com>
To: Jamal Hadi Salim <jhs@mojatatu.com>,
	sfeldma@gmail.com, netdev@vger.kernel.org
Subject: Re: [PATCH net-next] switchdev: add support for fdb add/del/dump via switchdev_port_obj ops.
Date: Fri, 08 May 2015 13:44:26 -0700	[thread overview]
Message-ID: <554D202A.5090207@intel.com> (raw)
In-Reply-To: <554CA674.8040905@mojatatu.com>

On 5/8/2015 5:05 AM, Jamal Hadi Salim wrote:
> On 05/07/15 00:42, Samudrala, Sridhar wrote:
>>
>>
>> On 5/6/2015 4:35 PM, Jamal Hadi Salim wrote:
>>> On 05/06/15 17:54, Sridhar Samudrala wrote:
>
>>> So i raised this earlier. DaveM also chimed in - but it seems still
>>> in there.
>>> i havent been following the discussion and i may have missed
>>> the agreement to keep the new IDs. Could we not just have used netlink
>>>  IDs (as opposed to a new SWITCHDEV_OBJ_PORT_FDB id)?
>>
>> I think you are referring to switch port attributes.  See Scott's
>> response here on
>> using netlink IDs for attributes.
>> http://thread.gmane.org/gmane.linux.network/357694/focus=357921
>>
>> This patch is adding 'fdb' as new switch port object.  It is similar to
>> other
>> objects like 'VLAN' and 'FIB' that are added by Scott's patches.
>
> Sorry I missed it - but I am not making sense of Scott's answer.
> The danger of adding these visible APIs is it would be too late
> to take them out once they hit the wild (Dave's famous "the horse
> has left the barn" or look at Jiri's netconf presentation). I hate
> to do this but taking longer to discuss these issues is important.
>
> I will agree that we need some common id to represent the grouping
> of the fdb entries - i will contend using netlink verb-nouns is
> sufficient. Example: RTM_NEWNEIGH is clearly refering to a
> modify or create on an fdb.
> The other issue is how to define optionality.
> So lets start with the fdb object:
> While i would agree they are common, Vlans are optional in an fdb
> entry. The only two items that must be present for an fdb entry are a
> dstmac address and an egress port.
> some low end switches dont do vlans. Therefore there are two IDs
> that must always be present:
>
> The object id for a port is the ifindex.
> The object id for a mac address is NDA_LLADDR
An fdb object is tied to a particular switch port and is associated with a
netdev. So we don't need a port in the fdb object.

>
> The object id for a vlan is NDA_VLAN
> Then there are of course a gazillion other features which may
> be one-of such as the fdb partition id in the netgear switch Ben was
> playing.
>
> I understand the earlier arguement of not needing to have
> multiple parsings of the same thing. But providing parseable
> options means never having to change what the object struct
> looks like. Just provide a scatter list of pointers to the
> different netlink attr-val pairs so it doesnt need to be
> re-parsed.

I think Scott introduced switchdev object abstraction to make the 
switchdev code simpler
and also to enable direct calls to add/delete objects without having to 
create dummy netlink
messages with attributes.
Scott should be able to provide better reasoning behind this abstraction.

>
> Otherwise we are re-inventing things and start to look like
> vendor SDKs.
>
> cheers,
> jamal
>

  reply	other threads:[~2015-05-08 20:44 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-06 21:54 [PATCH net-next] switchdev: add support for fdb add/del/dump via switchdev_port_obj ops Sridhar Samudrala
2015-05-06 23:35 ` Jamal Hadi Salim
2015-05-07  4:42   ` Samudrala, Sridhar
2015-05-07 12:51     ` roopa
2015-05-08 12:05     ` Jamal Hadi Salim
2015-05-08 20:44       ` Samudrala, Sridhar [this message]
2015-05-08 20:58       ` Scott Feldman
2015-05-11 13:15         ` Jamal Hadi Salim
2015-05-12 22:20 ` David Miller
2015-05-13  0:07   ` Samudrala, Sridhar

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=554D202A.5090207@intel.com \
    --to=sridhar.samudrala@intel.com \
    --cc=jhs@mojatatu.com \
    --cc=netdev@vger.kernel.org \
    --cc=sfeldma@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).