[net-next PATCH v6 0/3] net: dummy: Introduce dummy virtual functions

[net-next PATCH v6 0/3] net: dummy: Introduce dummy virtual functions

[Phil Sutter]

This series adds VF support to dummy device driver after adding the
necessary infrastructure changes:

Patch 1 adds a netdevice callback for device-specific VF count
retrieval. Patch 2 then changes dev_num_vf() implementation to make use
of that new callback (if implemented), falling back to the old
behaviour. Patch 3 then implements VF support in dummy, without the fake
PCI parent device hack from v5.

Phil Sutter (3):
  net: net_device_ops: Introduce ndo_get_vf_count
  net: rtnetlink: Use a local dev_num_vf() implementation
  net: dummy: Introduce dummy virtual functions

 drivers/net/dummy.c       | 178 +++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/netdevice.h |   5 ++
 include/linux/pci.h       |   2 -
 net/core/rtnetlink.c      |  37 ++++++----
 4 files changed, 205 insertions(+), 17 deletions(-)

-- 
2.11.0

[net-next PATCH v6 1/3] net: net_device_ops: Introduce ndo_get_vf_count

[Phil Sutter]

The idea is to allow drivers to implement this callback in order to
provide a custom way to return the number of virtual functions present
on the device.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v5:
- Introduced this patch.
---
 include/linux/netdevice.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ecd78b3c9abad..a04a693f55065 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -964,6 +964,10 @@ struct netdev_xdp {
  *      with PF and querying it may introduce a theoretical security risk.
  * int (*ndo_set_vf_rss_query_en)(struct net_device *dev, int vf, bool setting);
  * int (*ndo_get_vf_port)(struct net_device *dev, int vf, struct sk_buff *skb);
+ * int (*ndo_get_vf_count)(const struct net_device *dev);
+ *	Return the number of VFs present on this device instead of having
+ *	rtnetlink use pci_num_vf() on the PCI parent device.
+ *
  * int (*ndo_setup_tc)(struct net_device *dev, u8 tc)
  * 	Called to setup 'tc' number of traffic classes in the net device. This
  * 	is always called from the stack with the rtnl lock held and netif tx
@@ -1218,6 +1222,7 @@ struct net_device_ops {
 	int			(*ndo_set_vf_rss_query_en)(
 						   struct net_device *dev,
 						   int vf, bool setting);
+	int			(*ndo_get_vf_count)(const struct net_device *dev);
 	int			(*ndo_setup_tc)(struct net_device *dev,
 						u32 handle,
 						__be16 protocol,
-- 
2.11.0

[net-next PATCH v6 2/3] net: rtnetlink: Use a local dev_num_vf() implementation

[Phil Sutter]

Promote dev_num_vf() to be no longer PCI device specific but use
ndo_get_vf_count() if implemented and only fall back to pci_num_vf()
like the old dev_num_vf() did.

Since this implementation no longer requires a parent device to be
present, don't pass the parent but the actual device to it and have it
check for parent existence only in the fallback case. This in turn
allows to eliminate parent existence checks in callers.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v5:
- Introduced this patch.
---
 include/linux/pci.h  |  2 --
 net/core/rtnetlink.c | 37 ++++++++++++++++++++++++-------------
 2 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index e2d1a124216a9..adbc859fe7c4c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -885,7 +885,6 @@ void pcibios_setup_bridge(struct pci_bus *bus, unsigned long type);
 void pci_sort_breadthfirst(void);
 #define dev_is_pci(d) ((d)->bus == &pci_bus_type)
 #define dev_is_pf(d) ((dev_is_pci(d) ? to_pci_dev(d)->is_physfn : false))
-#define dev_num_vf(d) ((dev_is_pci(d) ? pci_num_vf(to_pci_dev(d)) : 0))
 
 /* Generic PCI functions exported to card drivers */
 
@@ -1630,7 +1629,6 @@ static inline int pci_get_new_domain_nr(void) { return -ENOSYS; }
 
 #define dev_is_pci(d) (false)
 #define dev_is_pf(d) (false)
-#define dev_num_vf(d) (0)
 #endif /* CONFIG_PCI */
 
 /* Include architecture-dependent settings and functions */
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 18b5aae99becf..84294593e0306 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -833,13 +833,24 @@ static void copy_rtnl_link_stats(struct rtnl_link_stats *a,
 	a->rx_nohandler = b->rx_nohandler;
 }
 
+static int dev_num_vf(const struct net_device *dev)
+{
+	if (dev->netdev_ops->ndo_get_vf_count)
+		return dev->netdev_ops->ndo_get_vf_count(dev);
+#ifdef CONFIG_PCI
+	if (dev->dev.parent && dev_is_pci(dev->dev.parent))
+		return pci_num_vf(to_pci_dev(dev->dev.parent));
+#endif
+	return 0;
+}
+
 /* All VF info */
 static inline int rtnl_vfinfo_size(const struct net_device *dev,
 				   u32 ext_filter_mask)
 {
-	if (dev->dev.parent && dev_is_pci(dev->dev.parent) &&
-	    (ext_filter_mask & RTEXT_FILTER_VF)) {
-		int num_vfs = dev_num_vf(dev->dev.parent);
+	int num_vfs = dev_num_vf(dev);
+
+	if (num_vfs && (ext_filter_mask & RTEXT_FILTER_VF)) {
 		size_t size = nla_total_size(0);
 		size += num_vfs *
 			(nla_total_size(0) +
@@ -889,12 +900,12 @@ static size_t rtnl_port_size(const struct net_device *dev,
 	size_t port_self_size = nla_total_size(sizeof(struct nlattr))
 		+ port_size;
 
-	if (!dev->netdev_ops->ndo_get_vf_port || !dev->dev.parent ||
+	if (!dev->netdev_ops->ndo_get_vf_port ||
 	    !(ext_filter_mask & RTEXT_FILTER_VF))
 		return 0;
-	if (dev_num_vf(dev->dev.parent))
+	if (dev_num_vf(dev))
 		return port_self_size + vf_ports_size +
-			vf_port_size * dev_num_vf(dev->dev.parent);
+			vf_port_size * dev_num_vf(dev);
 	else
 		return port_self_size;
 }
@@ -962,7 +973,7 @@ static int rtnl_vf_ports_fill(struct sk_buff *skb, struct net_device *dev)
 	if (!vf_ports)
 		return -EMSGSIZE;
 
-	for (vf = 0; vf < dev_num_vf(dev->dev.parent); vf++) {
+	for (vf = 0; vf < dev_num_vf(dev); vf++) {
 		vf_port = nla_nest_start(skb, IFLA_VF_PORT);
 		if (!vf_port)
 			goto nla_put_failure;
@@ -1012,7 +1023,7 @@ static int rtnl_port_fill(struct sk_buff *skb, struct net_device *dev,
 {
 	int err;
 
-	if (!dev->netdev_ops->ndo_get_vf_port || !dev->dev.parent ||
+	if (!dev->netdev_ops->ndo_get_vf_port ||
 	    !(ext_filter_mask & RTEXT_FILTER_VF))
 		return 0;
 
@@ -1020,7 +1031,7 @@ static int rtnl_port_fill(struct sk_buff *skb, struct net_device *dev,
 	if (err)
 		return err;
 
-	if (dev_num_vf(dev->dev.parent)) {
+	if (dev_num_vf(dev)) {
 		err = rtnl_vf_ports_fill(skb, dev);
 		if (err)
 			return err;
@@ -1351,15 +1362,15 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 	if (rtnl_fill_stats(skb, dev))
 		goto nla_put_failure;
 
-	if (dev->dev.parent && (ext_filter_mask & RTEXT_FILTER_VF) &&
-	    nla_put_u32(skb, IFLA_NUM_VF, dev_num_vf(dev->dev.parent)))
+	if (ext_filter_mask & RTEXT_FILTER_VF &&
+	    nla_put_u32(skb, IFLA_NUM_VF, dev_num_vf(dev)))
 		goto nla_put_failure;
 
-	if (dev->netdev_ops->ndo_get_vf_config && dev->dev.parent &&
+	if (dev->netdev_ops->ndo_get_vf_config &&
 	    ext_filter_mask & RTEXT_FILTER_VF) {
 		int i;
 		struct nlattr *vfinfo;
-		int num_vfs = dev_num_vf(dev->dev.parent);
+		int num_vfs = dev_num_vf(dev);
 
 		vfinfo = nla_nest_start(skb, IFLA_VFINFO_LIST);
 		if (!vfinfo)
-- 
2.11.0

[net-next PATCH v6 3/3] net: dummy: Introduce dummy virtual functions

[Phil Sutter]

The idea for this was born when testing VF support in iproute2 which was
impeded by hardware requirements. In fact, not every VF-capable hardware
driver implements all netdev ops, so testing the interface is still hard
to do even with a well-sorted hardware shelf.

To overcome this and allow for testing the user-kernel interface, this
patch allows to turn dummy into a PF with a configurable amount of VFs.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v5:
- Got rid of fake PCI parent hack altogether, implement ndo_get_vf_count
  instead.

Changes since v4:
- Initialize pci_pdev.sriov at runtime - older gcc versions don't allow
  initializing fields of anonymous unions at declaration time.
- Rebased onto current net-next/master.

Changes since v3:
- Changed type of vf_mac field from unsigned char to u8.
- Column-aligned structs' field names.

Changes since v2:
- Fixed oops on reboot (need to initialize parent device mutex).
- Got rid of potential mem leak noticed by Eric Dumazet.
- Dropped stray newline insertion.

Changes since v1:
- Fixed issues reported by kbuild test robot:
  - pci_dev->sriov is only present if CONFIG_PCI_ATS is active.
  - pci_bus_type does not exist if CONFIG_PCI is not defined.
---
 drivers/net/dummy.c | 178 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 176 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
index 6421835f11b7e..8da0a97ff7cee 100644
--- a/drivers/net/dummy.c
+++ b/drivers/net/dummy.c
@@ -42,6 +42,25 @@
 #define DRV_VERSION	"1.0"
 
 static int numdummies = 1;
+static int num_vfs;
+
+struct vf_data_storage {
+	u8	vf_mac[ETH_ALEN];
+	u16	pf_vlan; /* When set, guest VLAN config not allowed. */
+	u16	pf_qos;
+	__be16	vlan_proto;
+	u16	min_tx_rate;
+	u16	max_tx_rate;
+	u8	spoofchk_enabled;
+	bool	rss_query_enabled;
+	u8	trusted;
+	int	link_state;
+};
+
+struct dummy_priv {
+	int			num_vfs;
+	struct vf_data_storage	*vfinfo;
+};
 
 /* fake multicast ability */
 static void set_multicast_list(struct net_device *dev)
@@ -91,10 +110,25 @@ static netdev_tx_t dummy_xmit(struct sk_buff *skb, struct net_device *dev)
 
 static int dummy_dev_init(struct net_device *dev)
 {
+	struct dummy_priv *priv = netdev_priv(dev);
+
 	dev->dstats = netdev_alloc_pcpu_stats(struct pcpu_dstats);
 	if (!dev->dstats)
 		return -ENOMEM;
 
+	priv->num_vfs = num_vfs;
+	priv->vfinfo = NULL;
+
+	if (!num_vfs)
+		return 0;
+
+	priv->vfinfo = kcalloc(num_vfs, sizeof(struct vf_data_storage),
+			       GFP_KERNEL);
+	if (!priv->vfinfo) {
+		free_percpu(dev->dstats);
+		return -ENOMEM;
+	}
+
 	return 0;
 }
 
@@ -112,6 +146,124 @@ static int dummy_change_carrier(struct net_device *dev, bool new_carrier)
 	return 0;
 }
 
+static int dummy_set_vf_mac(struct net_device *dev, int vf, u8 *mac)
+{
+	struct dummy_priv *priv = netdev_priv(dev);
+
+	if (!is_valid_ether_addr(mac) || (vf >= priv->num_vfs))
+		return -EINVAL;
+
+	memcpy(priv->vfinfo[vf].vf_mac, mac, ETH_ALEN);
+
+	return 0;
+}
+
+static int dummy_set_vf_vlan(struct net_device *dev, int vf,
+			     u16 vlan, u8 qos, __be16 vlan_proto)
+{
+	struct dummy_priv *priv = netdev_priv(dev);
+
+	if ((vf >= priv->num_vfs) || (vlan > 4095) || (qos > 7))
+		return -EINVAL;
+
+	priv->vfinfo[vf].pf_vlan = vlan;
+	priv->vfinfo[vf].pf_qos = qos;
+	priv->vfinfo[vf].vlan_proto = vlan_proto;
+
+	return 0;
+}
+
+static int dummy_set_vf_rate(struct net_device *dev, int vf, int min, int max)
+{
+	struct dummy_priv *priv = netdev_priv(dev);
+
+	if (vf >= priv->num_vfs)
+		return -EINVAL;
+
+	priv->vfinfo[vf].min_tx_rate = min;
+	priv->vfinfo[vf].max_tx_rate = max;
+
+	return 0;
+}
+
+static int dummy_set_vf_spoofchk(struct net_device *dev, int vf, bool val)
+{
+	struct dummy_priv *priv = netdev_priv(dev);
+
+	if (vf >= priv->num_vfs)
+		return -EINVAL;
+
+	priv->vfinfo[vf].spoofchk_enabled = val;
+
+	return 0;
+}
+
+static int dummy_set_vf_rss_query_en(struct net_device *dev, int vf, bool val)
+{
+	struct dummy_priv *priv = netdev_priv(dev);
+
+	if (vf >= priv->num_vfs)
+		return -EINVAL;
+
+	priv->vfinfo[vf].rss_query_enabled = val;
+
+	return 0;
+}
+
+static int dummy_set_vf_trust(struct net_device *dev, int vf, bool val)
+{
+	struct dummy_priv *priv = netdev_priv(dev);
+
+	if (vf >= priv->num_vfs)
+		return -EINVAL;
+
+	priv->vfinfo[vf].trusted = val;
+
+	return 0;
+}
+
+static int dummy_get_vf_config(struct net_device *dev,
+			       int vf, struct ifla_vf_info *ivi)
+{
+	struct dummy_priv *priv = netdev_priv(dev);
+
+	if (vf >= priv->num_vfs)
+		return -EINVAL;
+
+	ivi->vf = vf;
+	memcpy(&ivi->mac, priv->vfinfo[vf].vf_mac, ETH_ALEN);
+	ivi->vlan = priv->vfinfo[vf].pf_vlan;
+	ivi->qos = priv->vfinfo[vf].pf_qos;
+	ivi->spoofchk = priv->vfinfo[vf].spoofchk_enabled;
+	ivi->linkstate = priv->vfinfo[vf].link_state;
+	ivi->min_tx_rate = priv->vfinfo[vf].min_tx_rate;
+	ivi->max_tx_rate = priv->vfinfo[vf].max_tx_rate;
+	ivi->rss_query_en = priv->vfinfo[vf].rss_query_enabled;
+	ivi->trusted = priv->vfinfo[vf].trusted;
+	ivi->vlan_proto = priv->vfinfo[vf].vlan_proto;
+
+	return 0;
+}
+
+static int dummy_set_vf_link_state(struct net_device *dev, int vf, int state)
+{
+	struct dummy_priv *priv = netdev_priv(dev);
+
+	if (vf >= priv->num_vfs)
+		return -EINVAL;
+
+	priv->vfinfo[vf].link_state = state;
+
+	return 0;
+}
+
+static int dummy_get_vf_count(const struct net_device *dev)
+{
+	struct dummy_priv *priv = netdev_priv(dev);
+
+	return priv->num_vfs;
+}
+
 static const struct net_device_ops dummy_netdev_ops = {
 	.ndo_init		= dummy_dev_init,
 	.ndo_uninit		= dummy_dev_uninit,
@@ -121,6 +273,15 @@ static const struct net_device_ops dummy_netdev_ops = {
 	.ndo_set_mac_address	= eth_mac_addr,
 	.ndo_get_stats64	= dummy_get_stats64,
 	.ndo_change_carrier	= dummy_change_carrier,
+	.ndo_set_vf_mac		= dummy_set_vf_mac,
+	.ndo_set_vf_vlan	= dummy_set_vf_vlan,
+	.ndo_set_vf_rate	= dummy_set_vf_rate,
+	.ndo_set_vf_spoofchk	= dummy_set_vf_spoofchk,
+	.ndo_set_vf_trust	= dummy_set_vf_trust,
+	.ndo_get_vf_config	= dummy_get_vf_config,
+	.ndo_set_vf_link_state	= dummy_set_vf_link_state,
+	.ndo_set_vf_rss_query_en = dummy_set_vf_rss_query_en,
+	.ndo_get_vf_count	= dummy_get_vf_count,
 };
 
 static void dummy_get_drvinfo(struct net_device *dev,
@@ -134,6 +295,14 @@ static const struct ethtool_ops dummy_ethtool_ops = {
 	.get_drvinfo            = dummy_get_drvinfo,
 };
 
+static void dummy_free_netdev(struct net_device *dev)
+{
+	struct dummy_priv *priv = netdev_priv(dev);
+
+	kfree(priv->vfinfo);
+	free_netdev(dev);
+}
+
 static void dummy_setup(struct net_device *dev)
 {
 	ether_setup(dev);
@@ -141,7 +310,7 @@ static void dummy_setup(struct net_device *dev)
 	/* Initialize the device structure. */
 	dev->netdev_ops = &dummy_netdev_ops;
 	dev->ethtool_ops = &dummy_ethtool_ops;
-	dev->destructor = free_netdev;
+	dev->destructor = dummy_free_netdev;
 
 	/* Fill in device structure with ethernet-generic values. */
 	dev->flags |= IFF_NOARP;
@@ -172,6 +341,7 @@ static int dummy_validate(struct nlattr *tb[], struct nlattr *data[])
 
 static struct rtnl_link_ops dummy_link_ops __read_mostly = {
 	.kind		= DRV_NAME,
+	.priv_size	= sizeof(struct dummy_priv),
 	.setup		= dummy_setup,
 	.validate	= dummy_validate,
 };
@@ -180,12 +350,16 @@ static struct rtnl_link_ops dummy_link_ops __read_mostly = {
 module_param(numdummies, int, 0);
 MODULE_PARM_DESC(numdummies, "Number of dummy pseudo devices");
 
+module_param(num_vfs, int, 0);
+MODULE_PARM_DESC(num_vfs, "Number of dummy VFs per dummy device");
+
 static int __init dummy_init_one(void)
 {
 	struct net_device *dev_dummy;
 	int err;
 
-	dev_dummy = alloc_netdev(0, "dummy%d", NET_NAME_UNKNOWN, dummy_setup);
+	dev_dummy = alloc_netdev(sizeof(struct dummy_priv),
+				 "dummy%d", NET_NAME_UNKNOWN, dummy_setup);
 	if (!dev_dummy)
 		return -ENOMEM;
 
-- 
2.11.0

Re: [net-next PATCH v6 0/3] net: dummy: Introduce dummy virtual functions

[David Miller]

From: Phil Sutter <phil@nwl.cc>
Date: Thu,  5 Jan 2017 20:09:10 +0100

> This series adds VF support to dummy device driver after adding the
> necessary infrastructure changes:
> 
> Patch 1 adds a netdevice callback for device-specific VF count
> retrieval. Patch 2 then changes dev_num_vf() implementation to make use
> of that new callback (if implemented), falling back to the old
> behaviour. Patch 3 then implements VF support in dummy, without the fake
> PCI parent device hack from v5.

Please don't make this a netdev specific method and interface.

Put the method in "struct bus_device", thereby making it a generic
"device" layer thing.

So the pci BUS type will implement pci_bus_type.num_vf().  And you'll
make a bus type for the dummy device to attach to which will implement
it's own.

Re: [net-next PATCH v6 0/3] net: dummy: Introduce dummy virtual functions

[Phil Sutter]

Hi David,

On Fri, Jan 06, 2017 at 08:38:04PM -0500, David Miller wrote:
> From: Phil Sutter <phil@nwl.cc>
> Date: Thu,  5 Jan 2017 20:09:10 +0100
> 
> > This series adds VF support to dummy device driver after adding the
> > necessary infrastructure changes:
> > 
> > Patch 1 adds a netdevice callback for device-specific VF count
> > retrieval. Patch 2 then changes dev_num_vf() implementation to make use
> > of that new callback (if implemented), falling back to the old
> > behaviour. Patch 3 then implements VF support in dummy, without the fake
> > PCI parent device hack from v5.
> 
> Please don't make this a netdev specific method and interface.
> 
> Put the method in "struct bus_device", thereby making it a generic
> "device" layer thing.
> 
> So the pci BUS type will implement pci_bus_type.num_vf().  And you'll
> make a bus type for the dummy device to attach to which will implement
> it's own.

Following your approach, I'm running into a weird issue with conflicting
sysfs symlink names after calling register_netdevice for the dummy
device which has dev->dev.bus set to the dummy bus type I introduced:

In netdev_register_kobject, dev->class is set to &net_class. This means
that later in device_add, the call to device_add_class_symlinks will
create symlink to the class named devices/virtual/net/dummy0/subsystem.

The following call to bus_add_device by device_add though tries to
create a symlink to the bus with identical name.

This seems like a bug in device_add, but things apparently work for
other devices and I don't get what's different for them. Can you maybe
give me a hint what to look for or do you have an idea what I am
missing?

Thanks, Phil

Re: [net-next PATCH v6 0/3] net: dummy: Introduce dummy virtual functions

[Phil Sutter]

On Wed, Jan 11, 2017 at 02:38:57PM +0100, Phil Sutter wrote:
> On Fri, Jan 06, 2017 at 08:38:04PM -0500, David Miller wrote:
> > From: Phil Sutter <phil@nwl.cc>
> > Date: Thu,  5 Jan 2017 20:09:10 +0100
> > 
> > > This series adds VF support to dummy device driver after adding the
> > > necessary infrastructure changes:
> > > 
> > > Patch 1 adds a netdevice callback for device-specific VF count
> > > retrieval. Patch 2 then changes dev_num_vf() implementation to make use
> > > of that new callback (if implemented), falling back to the old
> > > behaviour. Patch 3 then implements VF support in dummy, without the fake
> > > PCI parent device hack from v5.
> > 
> > Please don't make this a netdev specific method and interface.
> > 
> > Put the method in "struct bus_device", thereby making it a generic
> > "device" layer thing.
> > 
> > So the pci BUS type will implement pci_bus_type.num_vf().  And you'll
> > make a bus type for the dummy device to attach to which will implement
> > it's own.
> 
> Following your approach, I'm running into a weird issue with conflicting
> sysfs symlink names after calling register_netdevice for the dummy
> device which has dev->dev.bus set to the dummy bus type I introduced:
> 
> In netdev_register_kobject, dev->class is set to &net_class. This means
> that later in device_add, the call to device_add_class_symlinks will
> create symlink to the class named devices/virtual/net/dummy0/subsystem.
> 
> The following call to bus_add_device by device_add though tries to
> create a symlink to the bus with identical name.

Ah, nevermind - it's actually the netdev's parent which should have the
bus type set. At first I assumed this would be a PCI specific thing.

Thanks, Phil