| Message ID | 20201111193737.1793-6-pablo@netfilter.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | netfilter: flowtable bridge and vlan enhancements | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 7710 this patch: 7710 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 43 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 8075 this patch: 8075 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On 11/11/2020 21:37, Pablo Neira Ayuso wrote: > Add .ndo_fill_forward_path for bridge devices. > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > --- > include/linux/netdevice.h | 1 + > net/bridge/br_device.c | 24 ++++++++++++++++++++++++ > 2 files changed, 25 insertions(+) > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > index ca8525a1a797..d26de9a3d382 100644 > --- a/include/linux/netdevice.h > +++ b/include/linux/netdevice.h > @@ -836,6 +836,7 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev, > enum net_device_path_type { > DEV_PATH_ETHERNET = 0, > DEV_PATH_VLAN, > + DEV_PATH_BRIDGE, > }; > > struct net_device_path { > diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c > index 387403931a63..4c3a5334abe0 100644 > --- a/net/bridge/br_device.c > +++ b/net/bridge/br_device.c > @@ -391,6 +391,29 @@ static int br_del_slave(struct net_device *dev, struct net_device *slave_dev) > return br_del_if(br, slave_dev); > } > > +static int br_fill_forward_path(struct net_device_path_ctx *ctx, > + struct net_device_path *path) > +{ > + struct net_bridge_fdb_entry *f; > + struct net_bridge_port *dst; > + struct net_bridge *br; > + > + if (netif_is_bridge_port(ctx->dev)) > + return -1; > + > + br = netdev_priv(ctx->dev); > + f = br_fdb_find_rcu(br, ctx->daddr, 0); > + if (!f || !f->dst) > + return -1; > + > + dst = READ_ONCE(f->dst); While this is ok, I meant that you have to test the ptr after. In theory between the !f->dst test above and now it could've become null, so to make it future-proof please test the null dst after deref, after the READ_ONCE(). I realize currently there are still problems after the change but we'll fix them. > + path->type = DEV_PATH_BRIDGE; > + path->dev = dst->br->dev; > + ctx->dev = dst->dev; > + > + return 0; > +} > + > static const struct ethtool_ops br_ethtool_ops = { > .get_drvinfo = br_getinfo, > .get_link = ethtool_op_get_link, > @@ -425,6 +448,7 @@ static const struct net_device_ops br_netdev_ops = { > .ndo_bridge_setlink = br_setlink, > .ndo_bridge_dellink = br_dellink, > .ndo_features_check = passthru_features_check, > + .ndo_fill_forward_path = br_fill_forward_path, > }; > > static struct device_type br_type = { >
On 12/11/2020 02:53, Nikolay Aleksandrov wrote: > On 11/11/2020 21:37, Pablo Neira Ayuso wrote: >> Add .ndo_fill_forward_path for bridge devices. >> >> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> >> --- >> include/linux/netdevice.h | 1 + >> net/bridge/br_device.c | 24 ++++++++++++++++++++++++ >> 2 files changed, 25 insertions(+) >> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h >> index ca8525a1a797..d26de9a3d382 100644 >> --- a/include/linux/netdevice.h >> +++ b/include/linux/netdevice.h >> @@ -836,6 +836,7 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev, >> enum net_device_path_type { >> DEV_PATH_ETHERNET = 0, >> DEV_PATH_VLAN, >> + DEV_PATH_BRIDGE, >> }; >> >> struct net_device_path { >> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c >> index 387403931a63..4c3a5334abe0 100644 >> --- a/net/bridge/br_device.c >> +++ b/net/bridge/br_device.c >> @@ -391,6 +391,29 @@ static int br_del_slave(struct net_device *dev, struct net_device *slave_dev) >> return br_del_if(br, slave_dev); >> } >> >> +static int br_fill_forward_path(struct net_device_path_ctx *ctx, >> + struct net_device_path *path) >> +{ >> + struct net_bridge_fdb_entry *f; >> + struct net_bridge_port *dst; >> + struct net_bridge *br; >> + >> + if (netif_is_bridge_port(ctx->dev)) >> + return -1; >> + >> + br = netdev_priv(ctx->dev); >> + f = br_fdb_find_rcu(br, ctx->daddr, 0); >> + if (!f || !f->dst) >> + return -1; >> + >> + dst = READ_ONCE(f->dst); > > While this is ok, I meant that you have to test the ptr after. In theory between > the !f->dst test above and now it could've become null, so to make it future-proof > please test the null dst after deref, after the READ_ONCE(). I realize currently > there are still problems after the change but we'll fix them. > On a second read this sounds too vague. :) I meant that there are still similar issues in the bridge code today which I'll take care of. For this patch what I suggested should be enough. Thanks, Nik >> + path->type = DEV_PATH_BRIDGE; >> + path->dev = dst->br->dev; >> + ctx->dev = dst->dev; >> + >> + return 0; >> +} >> + >> static const struct ethtool_ops br_ethtool_ops = { >> .get_drvinfo = br_getinfo, >> .get_link = ethtool_op_get_link, >> @@ -425,6 +448,7 @@ static const struct net_device_ops br_netdev_ops = { >> .ndo_bridge_setlink = br_setlink, >> .ndo_bridge_dellink = br_dellink, >> .ndo_features_check = passthru_features_check, >> + .ndo_fill_forward_path = br_fill_forward_path, >> }; >> >> static struct device_type br_type = { >> >
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index ca8525a1a797..d26de9a3d382 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -836,6 +836,7 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev, enum net_device_path_type { DEV_PATH_ETHERNET = 0, DEV_PATH_VLAN, + DEV_PATH_BRIDGE, }; struct net_device_path { diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c index 387403931a63..4c3a5334abe0 100644 --- a/net/bridge/br_device.c +++ b/net/bridge/br_device.c @@ -391,6 +391,29 @@ static int br_del_slave(struct net_device *dev, struct net_device *slave_dev) return br_del_if(br, slave_dev); } +static int br_fill_forward_path(struct net_device_path_ctx *ctx, + struct net_device_path *path) +{ + struct net_bridge_fdb_entry *f; + struct net_bridge_port *dst; + struct net_bridge *br; + + if (netif_is_bridge_port(ctx->dev)) + return -1; + + br = netdev_priv(ctx->dev); + f = br_fdb_find_rcu(br, ctx->daddr, 0); + if (!f || !f->dst) + return -1; + + dst = READ_ONCE(f->dst); + path->type = DEV_PATH_BRIDGE; + path->dev = dst->br->dev; + ctx->dev = dst->dev; + + return 0; +} + static const struct ethtool_ops br_ethtool_ops = { .get_drvinfo = br_getinfo, .get_link = ethtool_op_get_link, @@ -425,6 +448,7 @@ static const struct net_device_ops br_netdev_ops = { .ndo_bridge_setlink = br_setlink, .ndo_bridge_dellink = br_dellink, .ndo_features_check = passthru_features_check, + .ndo_fill_forward_path = br_fill_forward_path, }; static struct device_type br_type = {
Add .ndo_fill_forward_path for bridge devices. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- include/linux/netdevice.h | 1 + net/bridge/br_device.c | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+)