| Message ID | 1430949253-6358-1-git-send-email-sridhar.samudrala@intel.com |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On 05/06/15 17:54, Sridhar Samudrala wrote: > - introduce port fdb obj and generic switchdev_port_fdb_add/del/dump() > - use switchdev_port_fdb_add/del/dump in rocker/team/bonding ndo ops. > - add support for fdb obj in switchdev_port_obj_add/del/dump() > - switch rocker to implement fdb ops via switchdev_ops > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com> > Signed-off-by: Scott Feldman <sfeldma@gmail.com> > --- 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)? > +int switchdev_port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[], > + struct net_device *dev, const unsigned char *addr, > + u16 vid, u16 nlm_flags) > +{ > + struct switchdev_obj obj = { > + .id = SWITCHDEV_OBJ_PORT_FDB, > + .fdb = { > + .addr = addr, > + .vid = vid, > + }, > + }; > + > + return switchdev_port_obj_add(dev, &obj); > +} cheers, jamal -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 5/6/2015 4:35 PM, Jamal Hadi Salim wrote: > On 05/06/15 17:54, Sridhar Samudrala wrote: >> - introduce port fdb obj and generic switchdev_port_fdb_add/del/dump() >> - use switchdev_port_fdb_add/del/dump in rocker/team/bonding ndo ops. >> - add support for fdb obj in switchdev_port_obj_add/del/dump() >> - switch rocker to implement fdb ops via switchdev_ops >> >> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com> >> Signed-off-by: Scott Feldman <sfeldma@gmail.com> >> --- > > 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. > >> +int switchdev_port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[], >> + struct net_device *dev, const unsigned char *addr, >> + u16 vid, u16 nlm_flags) >> +{ >> + struct switchdev_obj obj = { >> + .id = SWITCHDEV_OBJ_PORT_FDB, >> + .fdb = { >> + .addr = addr, >> + .vid = vid, >> + }, >> + }; >> + >> + return switchdev_port_obj_add(dev, &obj); >> +} > > cheers, > jamal -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 5/6/15, 9:42 PM, Samudrala, Sridhar wrote: > > > On 5/6/2015 4:35 PM, Jamal Hadi Salim wrote: >> On 05/06/15 17:54, Sridhar Samudrala wrote: >>> - introduce port fdb obj and generic switchdev_port_fdb_add/del/dump() >>> - use switchdev_port_fdb_add/del/dump in rocker/team/bonding ndo ops. >>> - add support for fdb obj in switchdev_port_obj_add/del/dump() >>> - switch rocker to implement fdb ops via switchdev_ops >>> >>> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com> >>> Signed-off-by: Scott Feldman <sfeldma@gmail.com> >>> --- >> >> 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 > If you are using a global namespace for all switchdev objects/apis (as with scotts new patches), yes, using existing netlink id's and structures will not work as scott mentions in the link you point to above. But, if you were designing the api's for specific networking subsystems like it was initially, then netlink id's or existing structures will work. And i am guessing Jamal is referring to the latter. I did raise similar concerns earlier. Scotts patches do make the code way cleaner and less confusing than it was before (Thanks scott!). But, It does add this additional layer of mapping between kernel structures/objects and newly introduced switchdev structures/objects. The advantage of being in the kernel is access to existing kernel structures/objects. We lose that benefit. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 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. Otherwise we are re-inventing things and start to look like vendor SDKs. cheers, jamal -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 8, 2015 at 5:05 AM, Jamal Hadi Salim <jhs@mojatatu.com> 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 > > 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. > > Otherwise we are re-inventing things and start to look like > vendor SDKs. Bottom line is netlink is not great for in-kernel driver APIs. Look at the fdb and bridge_set/get/dellink ndo ops in netdevice.h to see what kind of horse already left the barn. It's a swayback nag blind in one eye and deaf in the other eye. The problems are: 1) Inconsistent call arguments, some strange mix of parsed and raw netlink attrs. This make it impossible to provide generic wrappers to handle higher-level operations such as the prepare-commit transaction model. 2) Netlink attr parsing/policy checks are now split/duplicated between core code and driver code. Parsing/policy checks should be done in one place: the core code. 3) Driver dumps ops duplicate the netlink skb building (see recent patch to fix MULTI flag usage and how it touched every driver). This skb building should be in core code, not in each driver. 4) netlink-based APIs are hard to call from kernel context where no netlink msgs exists. For example, consider the task of flushing FDBs from the device. Today, we'd need to synthesis netlink message to call into ndo_fdb_del to delete the FDB entry. 5) In some cases, there is a lot of processing done on the netlink msg before the call into the driver, and we don't want to duplicate all of that processing again in each driver. For example, consider the IPv4 fib API. A netlink msg originates the call into the driver, but the actual arguments of the call into the driver are packaged quite differently than the originating netlink msg. This call to the driver is made inline deep down in the logic of fib processing. 6) There are some switchdev attrs that don't originate as netlink msgs. Example is STP state. That's an internal call from the bridge driver. Don't get me wrong: I love netlink for user-space/kernel APIs. But for in-kernel APIs it's awkward and we want a clean API for switchdev. -scott -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/08/15 16:58, Scott Feldman wrote: > On Fri, May 8, 2015 at 5:05 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote: >> On 05/07/15 00:42, Samudrala, Sridhar wrote: >>> > Bottom line is netlink is not great for in-kernel driver APIs. Look > at the fdb and bridge_set/get/dellink ndo ops in netdevice.h to see > what kind of horse already left the barn. It's a swayback nag blind > in one eye and deaf in the other eye. The problems are: > Scott, I dont disagree with you in regarding to the fdb/brport. The barn has left the deaf-in-the-other-eye horse on that one (yes, I said that on purpose;->) and it may even make sense to write a different interface from user space. I could add more to your list (why are 8 bits being used to represent a single bit flag for example, etc). But what i am worried about is you are setting a trend. I would hate for this to start looking like SAI when we've had shit working really well. You'll find the L3 stuff is more solid in terms of its netlink usage. But even for the bridge stuff, describing an object as, hiearachy of An "add" looking like: RTM_NEWNEIGH, {pointer to attributeid, pointer to attribute value}+ whereas the pointers are to the original TLV (which is still sitting somewhere in memory) sounds useful. I dont want to slow you down and i dont have code to demonstrate it - but hoping I dont need to. cheers, jamal -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Sridhar Samudrala <sridhar.samudrala@intel.com> Date: Wed, 6 May 2015 14:54:13 -0700 > - introduce port fdb obj and generic switchdev_port_fdb_add/del/dump() > - use switchdev_port_fdb_add/del/dump in rocker/team/bonding ndo ops. > - add support for fdb obj in switchdev_port_obj_add/del/dump() > - switch rocker to implement fdb ops via switchdev_ops > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com> > Signed-off-by: Scott Feldman <sfeldma@gmail.com> What is this patch even against? I don't see switchdev_port_obj_add() in any tree I maintain, yet this patch's context shows references to it which should exist already. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 5/12/2015 3:20 PM, David Miller wrote: > From: Sridhar Samudrala <sridhar.samudrala@intel.com> > Date: Wed, 6 May 2015 14:54:13 -0700 > >> - introduce port fdb obj and generic switchdev_port_fdb_add/del/dump() >> - use switchdev_port_fdb_add/del/dump in rocker/team/bonding ndo ops. >> - add support for fdb obj in switchdev_port_obj_add/del/dump() >> - switch rocker to implement fdb ops via switchdev_ops >> >> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com> >> Signed-off-by: Scott Feldman <sfeldma@gmail.com> > What is this patch even against? I don't see switchdev_port_obj_add() > in any tree I maintain, yet this patch's context shows references to > it which should exist already. Sorry, I should have mentioned that this patch is on top of v5 version of Scott's switchdev spring cleanup patches. Now that v7 version is merged, i will resubmit on top of net-next. Thanks Sridhar -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 5215e63..f63d078 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -4039,6 +4039,9 @@ static const struct net_device_ops bond_netdev_ops = { .ndo_bridge_setlink = switchdev_port_bridge_setlink, .ndo_bridge_getlink = switchdev_port_bridge_getlink, .ndo_bridge_dellink = switchdev_port_bridge_dellink, + .ndo_fdb_add = switchdev_port_fdb_add, + .ndo_fdb_del = switchdev_port_fdb_del, + .ndo_fdb_dump = switchdev_port_fdb_dump, .ndo_features_check = passthru_features_check, }; diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c index e9ee372..ba9011e 100644 --- a/drivers/net/ethernet/rocker/rocker.c +++ b/drivers/net/ethernet/rocker/rocker.c @@ -4153,108 +4153,6 @@ static int rocker_port_vlan_rx_kill_vid(struct net_device *dev, return rocker_port_vlan(rocker_port, ROCKER_OP_FLAG_REMOVE, vid); } -static int rocker_port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[], - struct net_device *dev, - const unsigned char *addr, u16 vid, - u16 nlm_flags) -{ - struct rocker_port *rocker_port = netdev_priv(dev); - __be16 vlan_id = rocker_port_vid_to_vlan(rocker_port, vid, NULL); - int flags = 0; - - if (!rocker_port_is_bridged(rocker_port)) - return -EINVAL; - - return rocker_port_fdb(rocker_port, addr, vlan_id, flags); -} - -static int rocker_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[], - struct net_device *dev, - const unsigned char *addr, u16 vid) -{ - struct rocker_port *rocker_port = netdev_priv(dev); - __be16 vlan_id = rocker_port_vid_to_vlan(rocker_port, vid, NULL); - int flags = ROCKER_OP_FLAG_REMOVE; - - if (!rocker_port_is_bridged(rocker_port)) - return -EINVAL; - - return rocker_port_fdb(rocker_port, addr, vlan_id, flags); -} - -static int rocker_fdb_fill_info(struct sk_buff *skb, - struct rocker_port *rocker_port, - const unsigned char *addr, u16 vid, - u32 portid, u32 seq, int type, - unsigned int flags) -{ - struct nlmsghdr *nlh; - struct ndmsg *ndm; - - nlh = nlmsg_put(skb, portid, seq, type, sizeof(*ndm), flags); - if (!nlh) - return -EMSGSIZE; - - ndm = nlmsg_data(nlh); - ndm->ndm_family = AF_BRIDGE; - ndm->ndm_pad1 = 0; - ndm->ndm_pad2 = 0; - ndm->ndm_flags = NTF_SELF; - ndm->ndm_type = 0; - ndm->ndm_ifindex = rocker_port->dev->ifindex; - ndm->ndm_state = NUD_REACHABLE; - - if (nla_put(skb, NDA_LLADDR, ETH_ALEN, addr)) - goto nla_put_failure; - - if (vid && nla_put_u16(skb, NDA_VLAN, vid)) - goto nla_put_failure; - - nlmsg_end(skb, nlh); - return 0; - -nla_put_failure: - nlmsg_cancel(skb, nlh); - return -EMSGSIZE; -} - -static int rocker_port_fdb_dump(struct sk_buff *skb, - struct netlink_callback *cb, - struct net_device *dev, - struct net_device *filter_dev, - int idx) -{ - struct rocker_port *rocker_port = netdev_priv(dev); - struct rocker *rocker = rocker_port->rocker; - struct rocker_fdb_tbl_entry *found; - struct hlist_node *tmp; - int bkt; - unsigned long lock_flags; - const unsigned char *addr; - u16 vid; - int err; - - spin_lock_irqsave(&rocker->fdb_tbl_lock, lock_flags); - hash_for_each_safe(rocker->fdb_tbl, bkt, tmp, found, entry) { - if (found->key.pport != rocker_port->pport) - continue; - if (idx < cb->args[0]) - goto skip; - addr = found->key.addr; - vid = rocker_port_vlan_to_vid(rocker_port, found->key.vlan_id); - err = rocker_fdb_fill_info(skb, rocker_port, addr, vid, - NETLINK_CB(cb->skb).portid, - cb->nlh->nlmsg_seq, - RTM_NEWNEIGH, NLM_F_MULTI); - if (err < 0) - break; -skip: - ++idx; - } - spin_unlock_irqrestore(&rocker->fdb_tbl_lock, lock_flags); - return idx; -} - static int rocker_port_get_phys_port_name(struct net_device *dev, char *buf, size_t len) { @@ -4277,12 +4175,12 @@ static const struct net_device_ops rocker_port_netdev_ops = { .ndo_set_mac_address = rocker_port_set_mac_address, .ndo_vlan_rx_add_vid = rocker_port_vlan_rx_add_vid, .ndo_vlan_rx_kill_vid = rocker_port_vlan_rx_kill_vid, - .ndo_fdb_add = rocker_port_fdb_add, - .ndo_fdb_del = rocker_port_fdb_del, - .ndo_fdb_dump = rocker_port_fdb_dump, .ndo_bridge_getlink = switchdev_port_bridge_getlink, .ndo_bridge_setlink = switchdev_port_bridge_setlink, .ndo_bridge_dellink = switchdev_port_bridge_dellink, + .ndo_fdb_add = switchdev_port_fdb_add, + .ndo_fdb_del = switchdev_port_fdb_del, + .ndo_fdb_dump = switchdev_port_fdb_dump, .ndo_get_phys_port_name = rocker_port_get_phys_port_name, }; @@ -4405,6 +4303,18 @@ static int rocker_port_vlans_add(struct rocker_port *rocker_port, return 0; } +static int rocker_port_fdb_add(struct rocker_port *rocker_port, + struct switchdev_obj_fdb *fdb) +{ + __be16 vlan_id = rocker_port_vid_to_vlan(rocker_port, fdb->vid, NULL); + int flags = 0; + + if (!rocker_port_is_bridged(rocker_port)) + return -EINVAL; + + return rocker_port_fdb(rocker_port, fdb->addr, vlan_id, flags); +} + static int rocker_port_obj_add(struct net_device *dev, struct switchdev_obj *obj) { @@ -4435,6 +4345,9 @@ static int rocker_port_obj_add(struct net_device *dev, fib4->dst_len, fib4->fi, fib4->tb_id, 0); break; + case SWITCHDEV_OBJ_PORT_FDB: + err = rocker_port_fdb_add(rocker_port, &obj->fdb); + break; default: err = -EOPNOTSUPP; break; @@ -4473,6 +4386,18 @@ static int rocker_port_vlans_del(struct rocker_port *rocker_port, return 0; } +static int rocker_port_fdb_del(struct rocker_port *rocker_port, + struct switchdev_obj_fdb *fdb) +{ + __be16 vlan_id = rocker_port_vid_to_vlan(rocker_port, fdb->vid, NULL); + int flags = ROCKER_OP_FLAG_REMOVE; + + if (!rocker_port_is_bridged(rocker_port)) + return -EINVAL; + + return rocker_port_fdb(rocker_port, fdb->addr, vlan_id, flags); +} + static int rocker_port_obj_del(struct net_device *dev, struct switchdev_obj *obj) { @@ -4491,6 +4416,53 @@ static int rocker_port_obj_del(struct net_device *dev, fib4->tb_id, ROCKER_OP_FLAG_REMOVE); break; + case SWITCHDEV_OBJ_PORT_FDB: + err = rocker_port_fdb_del(rocker_port, &obj->fdb); + break; + default: + err = -EOPNOTSUPP; + break; + } + + return err; +} + +static int rocker_port_fdb_dump(struct rocker_port *rocker_port, + struct switchdev_obj *obj) +{ + struct rocker *rocker = rocker_port->rocker; + struct rocker_fdb_tbl_entry *found; + struct hlist_node *tmp; + unsigned long lock_flags; + int bkt; + int err = 0; + + spin_lock_irqsave(&rocker->fdb_tbl_lock, lock_flags); + hash_for_each_safe(rocker->fdb_tbl, bkt, tmp, found, entry) { + if (found->key.pport != rocker_port->pport) + continue; + obj->fdb.addr = found->key.addr; + obj->fdb.vid = rocker_port_vlan_to_vid(rocker_port, + found->key.vlan_id); + err = obj->cb(rocker_port->dev, obj); + if (err) + break; + } + spin_unlock_irqrestore(&rocker->fdb_tbl_lock, lock_flags); + + return err; +} + +static int rocker_port_obj_dump(struct net_device *dev, + struct switchdev_obj *obj) +{ + struct rocker_port *rocker_port = netdev_priv(dev); + int err = 0; + + switch (obj->id) { + case SWITCHDEV_OBJ_PORT_FDB: + err = rocker_port_fdb_dump(rocker_port, obj); + break; default: err = -EOPNOTSUPP; break; @@ -4504,6 +4476,7 @@ static const struct switchdev_ops rocker_port_switchdev_ops = { .switchdev_port_attr_set = rocker_port_attr_set, .switchdev_port_obj_add = rocker_port_obj_add, .switchdev_port_obj_del = rocker_port_obj_del, + .switchdev_port_obj_dump = rocker_port_obj_dump, }; /******************** diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c index 1ec035a..daa054b 100644 --- a/drivers/net/team/team.c +++ b/drivers/net/team/team.c @@ -1980,6 +1980,9 @@ static const struct net_device_ops team_netdev_ops = { .ndo_bridge_setlink = switchdev_port_bridge_setlink, .ndo_bridge_getlink = switchdev_port_bridge_getlink, .ndo_bridge_dellink = switchdev_port_bridge_dellink, + .ndo_fdb_add = switchdev_port_fdb_add, + .ndo_fdb_del = switchdev_port_fdb_del, + .ndo_fdb_dump = switchdev_port_fdb_dump, .ndo_features_check = passthru_features_check, }; diff --git a/include/net/switchdev.h b/include/net/switchdev.h index 3b217b4..0dba9a7 100644 --- a/include/net/switchdev.h +++ b/include/net/switchdev.h @@ -47,11 +47,13 @@ enum switchdev_obj_id { SWITCHDEV_OBJ_UNDEFINED, SWITCHDEV_OBJ_PORT_VLAN, SWITCHDEV_OBJ_IPV4_FIB, + SWITCHDEV_OBJ_PORT_FDB, }; struct switchdev_obj { enum switchdev_obj_id id; enum switchdev_trans trans; + int (*cb)(struct net_device *dev, struct switchdev_obj *obj); union { struct switchdev_obj_vlan { /* PORT_VLAN */ u16 flags; @@ -67,6 +69,10 @@ struct switchdev_obj { u32 nlflags; u32 tb_id; } ipv4_fib; + struct switchdev_obj_fdb { /* PORT_FDB */ + const unsigned char *addr; + u16 vid; + } fdb; }; }; @@ -80,6 +86,8 @@ struct switchdev_obj { * @switchdev_port_obj_add: Add an object to port (see switchdev_obj). * * @switchdev_port_obj_del: Delete an object from port (see switchdev_obj). + * + * @switchdev_port_obj_dump: Dump port objects (see switchdev_obj). */ struct switchdev_ops { int (*switchdev_port_attr_get)(struct net_device *dev, @@ -90,6 +98,8 @@ struct switchdev_ops { struct switchdev_obj *obj); int (*switchdev_port_obj_del)(struct net_device *dev, struct switchdev_obj *obj); + int (*switchdev_port_obj_dump)(struct net_device *dev, + struct switchdev_obj *obj); }; enum switchdev_notifier_type { @@ -121,6 +131,7 @@ int switchdev_port_attr_set(struct net_device *dev, struct switchdev_attr *attr); int switchdev_port_obj_add(struct net_device *dev, struct switchdev_obj *obj); int switchdev_port_obj_del(struct net_device *dev, struct switchdev_obj *obj); +int switchdev_port_obj_dump(struct net_device *dev, struct switchdev_obj *obj); int register_switchdev_notifier(struct notifier_block *nb); int unregister_switchdev_notifier(struct notifier_block *nb); int call_switchdev_notifiers(unsigned long val, struct net_device *dev, @@ -137,6 +148,15 @@ int switchdev_fib_ipv4_add(u32 dst, int dst_len, struct fib_info *fi, int switchdev_fib_ipv4_del(u32 dst, int dst_len, struct fib_info *fi, u8 tos, u8 type, u32 tb_id); void switchdev_fib_ipv4_abort(struct fib_info *fi); +int switchdev_port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[], + struct net_device *dev, const unsigned char *addr, + u16 vid, u16 nlm_flags); +int switchdev_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[], + struct net_device *dev, const unsigned char *addr, + u16 vid); +int switchdev_port_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb, + struct net_device *dev, + struct net_device *filter_dev, int idx); #else @@ -164,6 +184,12 @@ static inline int switchdev_port_obj_del(struct net_device *dev, return -EOPNOTSUPP; } +static inline int switchdev_port_obj_dump(struct net_device *dev, + struct switchdev_obj *obj) +{ + return -EOPNOTSUPP; +} + static inline int register_switchdev_notifier(struct notifier_block *nb) { return 0; @@ -221,6 +247,30 @@ static inline void switchdev_fib_ipv4_abort(struct fib_info *fi) { } +static inline int switchdev_port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[], + struct net_device *dev, + const unsigned char *addr, + u16 vid, u16 nlm_flags) +{ + return -EOPNOTSUPP; +} + +static inline int switchdev_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[], + struct net_device *dev, + const unsigned char *addr, u16 vid) +{ + return -EOPNOTSUPP; +} + +static inline int switchdev_port_fdb_dump(struct sk_buff *skb, + struct netlink_callback *cb, + struct net_device *dev, + struct net_device *filter_dev, + int idx) +{ + return -EOPNOTSUPP; +} + #endif #endif /* _LINUX_SWITCHDEV_H_ */ diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c index 65d49d4..3f92a5e 100644 --- a/net/switchdev/switchdev.c +++ b/net/switchdev/switchdev.c @@ -295,6 +295,36 @@ int switchdev_port_obj_del(struct net_device *dev, struct switchdev_obj *obj) } EXPORT_SYMBOL_GPL(switchdev_port_obj_del); +/** + * switchdev_port_obj_dump - Dump port objects + * + * @dev: port device + * @obj: object to dump + */ +int switchdev_port_obj_dump(struct net_device *dev, struct switchdev_obj *obj) +{ + const struct switchdev_ops *ops = dev->switchdev_ops; + struct net_device *lower_dev; + struct list_head *iter; + int err = -EOPNOTSUPP; + + if (ops && ops->switchdev_port_obj_dump) + return ops->switchdev_port_obj_dump(dev, obj); + + /* Switch device port(s) may be stacked under + * bond/team/vlan dev, so recurse down to dump objects on + * first port at bottom of stack. + */ + + netdev_for_each_lower_dev(dev, lower_dev, iter) { + err = switchdev_port_obj_dump(lower_dev, obj); + break; + } + + return err; +} +EXPORT_SYMBOL_GPL(switchdev_port_obj_dump); + static DEFINE_MUTEX(switchdev_mutex); static RAW_NOTIFIER_HEAD(switchdev_notif_chain); @@ -564,6 +594,151 @@ int switchdev_port_bridge_dellink(struct net_device *dev, } EXPORT_SYMBOL_GPL(switchdev_port_bridge_dellink); +/** + * switchdev_port_fdb_add - Add FDB (MAC/VLAN) entry to port + * + * @ndmsg: netlink hdr + * @nlattr: netlink attributes + * @dev: port device + * @addr: MAC address to add + * @vid: VLAN to add + * + * Add FDB entry to switch device. + */ +int switchdev_port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[], + struct net_device *dev, const unsigned char *addr, + u16 vid, u16 nlm_flags) +{ + struct switchdev_obj obj = { + .id = SWITCHDEV_OBJ_PORT_FDB, + .fdb = { + .addr = addr, + .vid = vid, + }, + }; + + return switchdev_port_obj_add(dev, &obj); +} +EXPORT_SYMBOL_GPL(switchdev_port_fdb_add); + +/** + * switchdev_port_fdb_del - Delete FDB (MAC/VLAN) entry from port + * + * @ndmsg: netlink hdr + * @nlattr: netlink attributes + * @dev: port device + * @addr: MAC address to delete + * @vid: VLAN to delete + * + * Delete FDB entry from switch device. + */ +int switchdev_port_fdb_del(struct ndmsg *ndm, struct nlattr *tb[], + struct net_device *dev, const unsigned char *addr, + u16 vid) +{ + struct switchdev_obj obj = { + .id = SWITCHDEV_OBJ_PORT_FDB, + .fdb = { + .addr = addr, + .vid = vid, + }, + }; + + return switchdev_port_obj_del(dev, &obj); +} +EXPORT_SYMBOL_GPL(switchdev_port_fdb_del); + +struct switchdev_fdb_dump { + struct switchdev_obj obj; + struct sk_buff *skb; + struct netlink_callback *cb; + struct net_device *filter_dev; + int idx; +}; + +static int switchdev_port_fdb_dump_cb(struct net_device *dev, + struct switchdev_obj *obj) +{ + struct switchdev_fdb_dump *dump = + container_of(obj, struct switchdev_fdb_dump, obj); + u32 portid = NETLINK_CB(dump->cb->skb).portid; + u32 seq = dump->cb->nlh->nlmsg_seq; + struct nlmsghdr *nlh; + struct ndmsg *ndm; + struct net_device *master = netdev_master_upper_dev_get(dev); + + if (dump->idx < dump->cb->args[0]) + goto skip; + + if (master && dump->filter_dev != master) + goto skip; + + nlh = nlmsg_put(dump->skb, portid, seq, RTM_NEWNEIGH, + sizeof(*ndm), NLM_F_MULTI); + if (!nlh) + return -EMSGSIZE; + + ndm = nlmsg_data(nlh); + ndm->ndm_family = AF_BRIDGE; + ndm->ndm_pad1 = 0; + ndm->ndm_pad2 = 0; + ndm->ndm_flags = NTF_SELF; + ndm->ndm_type = 0; + ndm->ndm_ifindex = dev->ifindex; + ndm->ndm_state = NUD_REACHABLE; + + if (nla_put(dump->skb, NDA_LLADDR, ETH_ALEN, obj->fdb.addr)) + goto nla_put_failure; + + if (obj->fdb.vid && nla_put_u16(dump->skb, NDA_VLAN, obj->fdb.vid)) + goto nla_put_failure; + + nlmsg_end(dump->skb, nlh); + +skip: + dump->idx++; + return 0; + +nla_put_failure: + nlmsg_cancel(dump->skb, nlh); + return -EMSGSIZE; +} + +/** + * switchdev_port_fdb_dump - Dump port FDB (MAC/VLAN) entries + * + * @skb: netlink skb + * @cb: netlink callback + * @dev: port device + * @filter_dev: filter device + * @idx: + * + * Delete FDB entry from switch device. + */ +int switchdev_port_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb, + struct net_device *dev, + struct net_device *filter_dev, int idx) +{ + struct switchdev_fdb_dump dump = { + .obj = { + .id = SWITCHDEV_OBJ_PORT_FDB, + .cb = switchdev_port_fdb_dump_cb, + }, + .skb = skb, + .cb = cb, + .filter_dev = filter_dev, + .idx = idx, + }; + int err; + + err = switchdev_port_obj_dump(dev, &dump.obj); + if (err) + return err; + + return dump.idx; +} +EXPORT_SYMBOL_GPL(switchdev_port_fdb_dump); + static struct net_device *switchdev_get_lowest_dev(struct net_device *dev) { const struct switchdev_ops *ops = dev->switchdev_ops;