| Message ID | 1416911328-10979-18-git-send-email-jiri@resnulli.us |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On 11/25/14 05:28, Jiri Pirko wrote: > From: Scott Feldman <sfeldma@gmail.com> > > Rocker ports will use new "swdev" hwmode for bridge port offload policy. > Current supported policy settings are BR_LEARNING and BR_LEARNING_SYNC. > User can turn on/off device port FDB learning and syncing to bridge. > > Signed-off-by: Scott Feldman <sfeldma@gmail.com> > Signed-off-by: Jiri Pirko <jiri@resnulli.us> as previous comments - please submit rocker separately 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 Tue, Nov 25, 2014 at 6:09 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote: > On 11/25/14 05:28, Jiri Pirko wrote: >> >> From: Scott Feldman <sfeldma@gmail.com> >> >> Rocker ports will use new "swdev" hwmode for bridge port offload policy. >> Current supported policy settings are BR_LEARNING and BR_LEARNING_SYNC. >> User can turn on/off device port FDB learning and syncing to bridge. >> >> Signed-off-by: Scott Feldman <sfeldma@gmail.com> >> Signed-off-by: Jiri Pirko <jiri@resnulli.us> > > > as previous comments - please submit rocker separately I disagree. API changes need a reference implementation to show usage and for testing. If you have have an alternate switch implementation that achieves the same goal, bring it forward. > 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 11/25/14 13:55, Scott Feldman wrote: > I disagree. API changes need a reference implementation to show usage > and for testing. If you have have an alternate switch implementation > that achieves the same goal, bring it forward. > Yes, point conceded ;-> /me waits for the next guy who is going to smirk at me for saying the above and tell Jiri to fix his typo ;-> 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
Tue, Nov 25, 2014 at 08:00:33PM CET, jhs@mojatatu.com wrote: >On 11/25/14 13:55, Scott Feldman wrote: > >>I disagree. API changes need a reference implementation to show usage >>and for testing. If you have have an alternate switch implementation >>that achieves the same goal, bring it forward. >> > >Yes, point conceded ;-> > >/me waits for the next guy who is going to smirk at me for saying the >above and tell Jiri to fix his typo ;-> Yep, people do not read replies :) -- 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 11/25/14 at 11:28am, Jiri Pirko wrote: > @@ -3657,6 +3693,64 @@ skip: > return idx; > } > > +static int rocker_port_bridge_setlink(struct net_device *dev, > + struct nlmsghdr *nlh) > +{ > + struct rocker_port *rocker_port = netdev_priv(dev); > + struct nlattr *protinfo; > + struct nlattr *afspec; > + struct nlattr *attr; > + u16 mode; > + int err; > + > + protinfo = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), > + IFLA_PROTINFO); > + afspec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC); > + > + if (afspec) { > + attr = nla_find_nested(afspec, IFLA_BRIDGE_MODE); > + if (attr) { > + mode = nla_get_u16(attr); > + if (mode != BRIDGE_MODE_SWDEV) > + return -EINVAL; > + } > + } The Netlink message is completely unverified at this point. All rtnl_bridge_setlink() does is verify that msgsize >= ifinfomsg. All of the drivers but br_setlink() need fixing in this regard. -- 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
Wed, Nov 26, 2014 at 12:07:09PM CET, tgraf@suug.ch wrote: >On 11/25/14 at 11:28am, Jiri Pirko wrote: >> @@ -3657,6 +3693,64 @@ skip: >> return idx; >> } >> >> +static int rocker_port_bridge_setlink(struct net_device *dev, >> + struct nlmsghdr *nlh) >> +{ >> + struct rocker_port *rocker_port = netdev_priv(dev); >> + struct nlattr *protinfo; >> + struct nlattr *afspec; >> + struct nlattr *attr; >> + u16 mode; >> + int err; >> + >> + protinfo = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), >> + IFLA_PROTINFO); >> + afspec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC); >> + >> + if (afspec) { >> + attr = nla_find_nested(afspec, IFLA_BRIDGE_MODE); >> + if (attr) { >> + mode = nla_get_u16(attr); >> + if (mode != BRIDGE_MODE_SWDEV) >> + return -EINVAL; >> + } >> + } > >The Netlink message is completely unverified at this point. All >rtnl_bridge_setlink() does is verify that msgsize >= ifinfomsg. >All of the drivers but br_setlink() need fixing in this regard. I believe that we should fix this for all drivers in a follow-up patch. -- 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 11/26/14 at 12:27pm, Jiri Pirko wrote: > Wed, Nov 26, 2014 at 12:07:09PM CET, tgraf@suug.ch wrote: > >The Netlink message is completely unverified at this point. All > >rtnl_bridge_setlink() does is verify that msgsize >= ifinfomsg. > >All of the drivers but br_setlink() need fixing in this regard. > > I believe that we should fix this for all drivers in a follow-up patch. I'm working on this. Will send fixes later today. -- 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
Wed, Nov 26, 2014 at 12:30:24PM CET, tgraf@suug.ch wrote: >On 11/26/14 at 12:27pm, Jiri Pirko wrote: >> Wed, Nov 26, 2014 at 12:07:09PM CET, tgraf@suug.ch wrote: >> >The Netlink message is completely unverified at this point. All >> >rtnl_bridge_setlink() does is verify that msgsize >= ifinfomsg. >> >All of the drivers but br_setlink() need fixing in this regard. >> >> I believe that we should fix this for all drivers in a follow-up patch. > >I'm working on this. Will send fixes later today. Feel free to fix rocker as well. I'll take your patch into my queue. -- 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/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c index 1434497..6d56960 100644 --- a/drivers/net/ethernet/rocker/rocker.c +++ b/drivers/net/ethernet/rocker/rocker.c @@ -203,6 +203,7 @@ struct rocker_port { u32 lport; __be16 internal_vlan_id; int stp_state; + u32 brport_flags; bool ctrls[ROCKER_CTRL_MAX]; unsigned long vlan_bitmap[ROCKER_VLAN_BITMAP_LEN]; struct napi_struct napi_tx; @@ -1629,6 +1630,30 @@ rocker_cmd_set_port_settings_macaddr_prep(struct rocker *rocker, return 0; } +static int +rocker_cmd_set_port_learning_prep(struct rocker *rocker, + struct rocker_port *rocker_port, + struct rocker_desc_info *desc_info, + void *priv) +{ + struct rocker_tlv *cmd_info; + + if (rocker_tlv_put_u16(desc_info, ROCKER_TLV_CMD_TYPE, + ROCKER_TLV_CMD_TYPE_SET_PORT_SETTINGS)) + return -EMSGSIZE; + cmd_info = rocker_tlv_nest_start(desc_info, ROCKER_TLV_CMD_INFO); + if (!cmd_info) + return -EMSGSIZE; + if (rocker_tlv_put_u32(desc_info, ROCKER_TLV_CMD_PORT_SETTINGS_LPORT, + rocker_port->lport)) + return -EMSGSIZE; + if (rocker_tlv_put_u8(desc_info, ROCKER_TLV_CMD_PORT_SETTINGS_LEARNING, + !!(rocker_port->brport_flags & BR_LEARNING))) + return -EMSGSIZE; + rocker_tlv_nest_end(desc_info, cmd_info); + return 0; +} + static int rocker_cmd_get_port_settings_ethtool(struct rocker_port *rocker_port, struct ethtool_cmd *ecmd) { @@ -1663,6 +1688,13 @@ static int rocker_cmd_set_port_settings_macaddr(struct rocker_port *rocker_port, macaddr, NULL, NULL, false); } +static int rocker_port_set_learning(struct rocker_port *rocker_port) +{ + return rocker_cmd_exec(rocker_port->rocker, rocker_port, + rocker_cmd_set_port_learning_prep, + NULL, NULL, NULL, false); +} + static int rocker_cmd_flow_tbl_add_ig_port(struct rocker_desc_info *desc_info, struct rocker_flow_tbl_entry *entry) { @@ -2995,6 +3027,7 @@ static int rocker_port_fdb_learn(struct rocker_port *rocker_port, u32 out_lport = rocker_port->lport; u32 tunnel_id = 0; u32 group_id = ROCKER_GROUP_NONE; + bool syncing = !!(rocker_port->brport_flags & BR_LEARNING_SYNC); bool copy_to_cpu = false; int err; @@ -3009,6 +3042,9 @@ static int rocker_port_fdb_learn(struct rocker_port *rocker_port, return err; } + if (!syncing) + return 0; + if (!rocker_port_is_bridged(rocker_port)) return 0; @@ -3657,6 +3693,64 @@ skip: return idx; } +static int rocker_port_bridge_setlink(struct net_device *dev, + struct nlmsghdr *nlh) +{ + struct rocker_port *rocker_port = netdev_priv(dev); + struct nlattr *protinfo; + struct nlattr *afspec; + struct nlattr *attr; + u16 mode; + int err; + + protinfo = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), + IFLA_PROTINFO); + afspec = nlmsg_find_attr(nlh, sizeof(struct ifinfomsg), IFLA_AF_SPEC); + + if (afspec) { + attr = nla_find_nested(afspec, IFLA_BRIDGE_MODE); + if (attr) { + mode = nla_get_u16(attr); + if (mode != BRIDGE_MODE_SWDEV) + return -EINVAL; + } + } + + if (protinfo) { + attr = nla_find_nested(protinfo, IFLA_BRPORT_LEARNING); + if (attr) { + if (nla_get_u8(attr)) + rocker_port->brport_flags |= BR_LEARNING; + else + rocker_port->brport_flags &= ~BR_LEARNING; + err = rocker_port_set_learning(rocker_port); + if (err) + return err; + } + attr = nla_find_nested(protinfo, IFLA_BRPORT_LEARNING_SYNC); + if (attr) { + if (nla_get_u8(attr)) + rocker_port->brport_flags |= BR_LEARNING_SYNC; + else + rocker_port->brport_flags &= ~BR_LEARNING_SYNC; + } + } + + return 0; +} + +static int rocker_port_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq, + struct net_device *dev, + u32 filter_mask) +{ + struct rocker_port *rocker_port = netdev_priv(dev); + u16 mode = BRIDGE_MODE_SWDEV; + u32 mask = BR_LEARNING | BR_LEARNING_SYNC; + + return ndo_dflt_bridge_getlink(skb, pid, seq, dev, mode, + rocker_port->brport_flags, mask); +} + static int rocker_port_switch_parent_id_get(struct net_device *dev, struct netdev_phys_item_id *psid) { @@ -3685,6 +3779,8 @@ static const struct net_device_ops rocker_port_netdev_ops = { .ndo_fdb_add = rocker_port_fdb_add, .ndo_fdb_del = rocker_port_fdb_del, .ndo_fdb_dump = rocker_port_fdb_dump, + .ndo_bridge_setlink = rocker_port_bridge_setlink, + .ndo_bridge_getlink = rocker_port_bridge_getlink, .ndo_switch_parent_id_get = rocker_port_switch_parent_id_get, .ndo_switch_port_stp_update = rocker_port_switch_port_stp_update, }; @@ -3885,6 +3981,7 @@ static int rocker_probe_port(struct rocker *rocker, unsigned int port_number) rocker_port->rocker = rocker; rocker_port->port_number = port_number; rocker_port->lport = port_number + 1; + rocker_port->brport_flags = BR_LEARNING | BR_LEARNING_SYNC; rocker_port_dev_addr_init(rocker, rocker_port); dev->netdev_ops = &rocker_port_netdev_ops; @@ -3904,6 +4001,8 @@ static int rocker_probe_port(struct rocker *rocker, unsigned int port_number) } rocker->ports[port_number] = rocker_port; + rocker_port_set_learning(rocker_port); + rocker_port->internal_vlan_id = rocker_port_internal_vlan_id_get(rocker_port, dev->ifindex); err = rocker_port_ig_tbl(rocker_port, 0); diff --git a/drivers/net/ethernet/rocker/rocker.h b/drivers/net/ethernet/rocker/rocker.h index 5251cf8..8d2865b 100644 --- a/drivers/net/ethernet/rocker/rocker.h +++ b/drivers/net/ethernet/rocker/rocker.h @@ -139,6 +139,7 @@ enum { ROCKER_TLV_CMD_PORT_SETTINGS_AUTONEG, /* u8 */ ROCKER_TLV_CMD_PORT_SETTINGS_MACADDR, /* binary */ ROCKER_TLV_CMD_PORT_SETTINGS_MODE, /* u8 */ + ROCKER_TLV_CMD_PORT_SETTINGS_LEARNING, /* u8 */ __ROCKER_TLV_CMD_PORT_SETTINGS_MAX, ROCKER_TLV_CMD_PORT_SETTINGS_MAX =