| Message ID | 1489390713-2634-4-git-send-email-jiri@resnulli.us |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On 3/13/17 1:38 AM, Jiri Pirko wrote: > From: Ido Schimmel <idosch@mellanox.com> > > Drivers capable of offloading VRF configurations need to know the ports > are enslaved to an actual VRF device and not some other L3 master. > > Add a flag to indicate netdev is a VRF master and a corresponding > helper. > > Signed-off-by: Ido Schimmel <idosch@mellanox.com> > Signed-off-by: Jiri Pirko <jiri@mellanox.com> > --- > drivers/net/vrf.c | 2 +- > include/linux/netdevice.h | 8 ++++++++ > 2 files changed, 9 insertions(+), 1 deletion(-) > IFF_VRF_MASTER was renamed IFF_L3MDEV_MASTER 18 months ago.
On Mon, Mar 13, 2017 at 08:39:19AM -0600, David Ahern wrote: > On 3/13/17 1:38 AM, Jiri Pirko wrote: > > From: Ido Schimmel <idosch@mellanox.com> > > > > Drivers capable of offloading VRF configurations need to know the ports > > are enslaved to an actual VRF device and not some other L3 master. > > > > Add a flag to indicate netdev is a VRF master and a corresponding > > helper. > > > > Signed-off-by: Ido Schimmel <idosch@mellanox.com> > > Signed-off-by: Jiri Pirko <jiri@mellanox.com> > > --- > > drivers/net/vrf.c | 2 +- > > include/linux/netdevice.h | 8 ++++++++ > > 2 files changed, 9 insertions(+), 1 deletion(-) > > > > IFF_VRF_MASTER was renamed IFF_L3MDEV_MASTER 18 months ago. But IFF_L3MDEV_MASTER isn't specific to the VRF driver. It can be set by other drivers including future ones that might be introduced. I need to allow enslavement to a VRF master, but reject others.
On 3/13/17 9:01 AM, Ido Schimmel wrote: > On Mon, Mar 13, 2017 at 08:39:19AM -0600, David Ahern wrote: >> On 3/13/17 1:38 AM, Jiri Pirko wrote: >>> From: Ido Schimmel <idosch@mellanox.com> >>> >>> Drivers capable of offloading VRF configurations need to know the ports >>> are enslaved to an actual VRF device and not some other L3 master. >>> >>> Add a flag to indicate netdev is a VRF master and a corresponding >>> helper. >>> >>> Signed-off-by: Ido Schimmel <idosch@mellanox.com> >>> Signed-off-by: Jiri Pirko <jiri@mellanox.com> >>> --- >>> drivers/net/vrf.c | 2 +- >>> include/linux/netdevice.h | 8 ++++++++ >>> 2 files changed, 9 insertions(+), 1 deletion(-) >>> >> >> IFF_VRF_MASTER was renamed IFF_L3MDEV_MASTER 18 months ago. > > But IFF_L3MDEV_MASTER isn't specific to the VRF driver. It can be set by > other drivers including future ones that might be introduced. I need to > allow enslavement to a VRF master, but reject others. > Why isn't an L3MDEV associated with a FIB table sufficient? ie., the L3MDEV_MASTER flag is set and the driver impements l3mdev_fib_table. At that point, what is specific to a VRF device that the offload relies on?
On Mon, Mar 13, 2017 at 09:15:32AM -0600, David Ahern wrote: > On 3/13/17 9:01 AM, Ido Schimmel wrote: > > On Mon, Mar 13, 2017 at 08:39:19AM -0600, David Ahern wrote: > >> On 3/13/17 1:38 AM, Jiri Pirko wrote: > >>> From: Ido Schimmel <idosch@mellanox.com> > >>> > >>> Drivers capable of offloading VRF configurations need to know the ports > >>> are enslaved to an actual VRF device and not some other L3 master. > >>> > >>> Add a flag to indicate netdev is a VRF master and a corresponding > >>> helper. > >>> > >>> Signed-off-by: Ido Schimmel <idosch@mellanox.com> > >>> Signed-off-by: Jiri Pirko <jiri@mellanox.com> > >>> --- > >>> drivers/net/vrf.c | 2 +- > >>> include/linux/netdevice.h | 8 ++++++++ > >>> 2 files changed, 9 insertions(+), 1 deletion(-) > >>> > >> > >> IFF_VRF_MASTER was renamed IFF_L3MDEV_MASTER 18 months ago. > > > > But IFF_L3MDEV_MASTER isn't specific to the VRF driver. It can be set by > > other drivers including future ones that might be introduced. I need to > > allow enslavement to a VRF master, but reject others. > > > > Why isn't an L3MDEV associated with a FIB table sufficient? ie., the > L3MDEV_MASTER flag is set and the driver impements l3mdev_fib_table. At > that point, what is specific to a VRF device that the offload relies on? The one thing specific to the VRF driver is that it only does that. If tomorrow a new driver is introduced and in addition to packet forwarding according to l3mdev_fib_table it also does something else, then I want to be sure mlxsw will be able to support it. Current approach seems cleaner to me, but I don't mind dropping this patch and introduce it when it's actually needed (if at all). OK?
diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c index 22379da..5c9a98e 100644 --- a/drivers/net/vrf.c +++ b/drivers/net/vrf.c @@ -1257,7 +1257,7 @@ static int vrf_newlink(struct net *src_net, struct net_device *dev, if (vrf->tb_id == RT_TABLE_UNSPEC) return -EINVAL; - dev->priv_flags |= IFF_L3MDEV_MASTER; + dev->priv_flags |= IFF_L3MDEV_MASTER | IFF_VRF_MASTER; err = register_netdevice(dev); if (err) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 97456b25..7908d31 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -1356,6 +1356,7 @@ struct net_device_ops { * @IFF_PHONY_HEADROOM: the headroom value is controlled by an external * entity (i.e. the master device for bridged veth) * @IFF_MACSEC: device is a MACsec device + * @IFF_VRF_MASTER: device is a VRF master device */ enum netdev_priv_flags { IFF_802_1Q_VLAN = 1<<0, @@ -1386,6 +1387,7 @@ enum netdev_priv_flags { IFF_RXFH_CONFIGURED = 1<<25, IFF_PHONY_HEADROOM = 1<<26, IFF_MACSEC = 1<<27, + IFF_VRF_MASTER = 1<<28, }; #define IFF_802_1Q_VLAN IFF_802_1Q_VLAN @@ -1415,6 +1417,7 @@ enum netdev_priv_flags { #define IFF_TEAM IFF_TEAM #define IFF_RXFH_CONFIGURED IFF_RXFH_CONFIGURED #define IFF_MACSEC IFF_MACSEC +#define IFF_VRF_MASTER IFF_VRF_MASTER /** * struct net_device - The DEVICE structure. @@ -4155,6 +4158,11 @@ static inline bool netif_supports_nofcs(struct net_device *dev) return dev->priv_flags & IFF_SUPP_NOFCS; } +static inline bool netif_is_vrf_master(const struct net_device *dev) +{ + return dev->priv_flags & IFF_VRF_MASTER; +} + static inline bool netif_is_l3_master(const struct net_device *dev) { return dev->priv_flags & IFF_L3MDEV_MASTER;