Re: [patch net-next v2 5/9] net: introduce dummy switch

2 messages, 2 authors, 2014-09-20 · open the first message on its own page

Re: [patch net-next v2 5/9] net: introduce dummy switch

From: Florian Fainelli <hidden>
Date: 2014-09-20 05:21:00

On 09/19/14 06:49, Jiri Pirko wrote:
Dummy switch implementation using switchdev interface
This really looks like a DSA driver that has 0 ports, and is not 
attached to an useful network interface, and which is registering its 
own set of rtnl operations for a purpose that is unclear to me.

I think registering these rtnl ops is misleading as it leads to a false 
idea that this is allowed, and that people are actually encouraged to do 
that for custom switch drivers, and this completely defeats the purpose 
of coming up with a generic API.

If we are to go that route anyway, I really prefer the way Felix did it 
in swconfig, and the fake switch driver did do something useful being 
attached to the loopback interface:

http://lists.openwall.net/netdev/2013/10/22/103
http://patchwork.ozlabs.org/patch/285478/
quoted hunk
Signed-off-by: Jiri Pirko <redacted>
---
  drivers/net/Kconfig          |   7 +++
  drivers/net/Makefile         |   1 +
  drivers/net/dummyswitch.c    | 130 +++++++++++++++++++++++++++++++++++++++++++
  include/uapi/linux/if_link.h |   9 +++
  4 files changed, 147 insertions(+)
  create mode 100644 drivers/net/dummyswitch.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index c6f6f69..7822c74 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -71,6 +71,13 @@ config DUMMY
  	  To compile this driver as a module, choose M here: the module
  	  will be called dummy.

+config NET_DUMMY_SWITCH
+	tristate "Dummy switch net driver support"
+	depends on NET_SWITCHDEV
+	---help---
+	  To compile this driver as a module, choose M here: the module
+	  will be called dummyswitch.
+
  config EQUALIZER
  	tristate "EQL (serial line load balancing) support"
  	---help---
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 61aefdd..3c835ba 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -7,6 +7,7 @@
  #
  obj-$(CONFIG_BONDING) += bonding/
  obj-$(CONFIG_DUMMY) += dummy.o
+obj-$(CONFIG_NET_DUMMY_SWITCH) += dummyswitch.o
  obj-$(CONFIG_EQUALIZER) += eql.o
  obj-$(CONFIG_IFB) += ifb.o
  obj-$(CONFIG_MACVLAN) += macvlan.o
diff --git a/drivers/net/dummyswitch.c b/drivers/net/dummyswitch.c
new file mode 100644
index 0000000..e7a48f4
--- /dev/null
+++ b/drivers/net/dummyswitch.c
@@ -0,0 +1,130 @@
+/*
+ * drivers/net/dummyswitch.c - Dummy switch device
+ * Copyright (c) 2014 Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
+#include <net/rtnetlink.h>
+
+struct dummyswport_priv {
+	struct netdev_phys_item_id psid;
+};
+
+static netdev_tx_t dummyswport_start_xmit(struct sk_buff *skb,
+					  struct net_device *dev)
+{
+	dev_kfree_skb(skb);
+	return NETDEV_TX_OK;
+}
+
+static int dummyswport_swdev_id_get(struct net_device *dev,
+				    struct netdev_phys_item_id *psid)
+{
+	struct dummyswport_priv *dsp = netdev_priv(dev);
+
+	memcpy(psid, &dsp->psid, sizeof(*psid));
+	return 0;
+}
+
+static int dummyswport_change_carrier(struct net_device *dev, bool new_carrier)
+{
+	if (new_carrier)
+		netif_carrier_on(dev);
+	else
+		netif_carrier_off(dev);
+	return 0;
+}
+
+static const struct net_device_ops dummyswport_netdev_ops = {
+	.ndo_start_xmit		= dummyswport_start_xmit,
+	.ndo_swdev_id_get	= dummyswport_swdev_id_get,
+	.ndo_change_carrier	= dummyswport_change_carrier,
+};
+
+static void dummyswport_setup(struct net_device *dev)
+{
+	ether_setup(dev);
+
+	/* Initialize the device structure. */
+	dev->netdev_ops = &dummyswport_netdev_ops;
+	dev->destructor = free_netdev;
+
+	/* Fill in device structure with ethernet-generic values. */
+	dev->tx_queue_len = 0;
+	dev->flags |= IFF_NOARP;
+	dev->flags &= ~IFF_MULTICAST;
+	dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
+	dev->features	|= NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_TSO;
+	dev->features	|= NETIF_F_HW_CSUM | NETIF_F_HIGHDMA | NETIF_F_LLTX;
+	eth_hw_addr_random(dev);
+}
+
+static int dummyswport_validate(struct nlattr *tb[], struct nlattr *data[])
+{
+	if (tb[IFLA_ADDRESS])
+		return -EINVAL;
+	if (!data || !data[IFLA_DUMMYSWPORT_PHYS_SWITCH_ID])
+		return -EINVAL;
+	return 0;
+}
+
+static int dummyswport_newlink(struct net *src_net, struct net_device *dev,
+			       struct nlattr *tb[], struct nlattr *data[])
+{
+	struct dummyswport_priv *dsp = netdev_priv(dev);
+	int err;
+
+	dsp->psid.id_len = nla_len(data[IFLA_DUMMYSWPORT_PHYS_SWITCH_ID]);
+	memcpy(dsp->psid.id, nla_data(data[IFLA_DUMMYSWPORT_PHYS_SWITCH_ID]),
+	       dsp->psid.id_len);
+
+	err = register_netdevice(dev);
+	if (err)
+		return err;
+
+	netif_carrier_on(dev);
+
+	return 0;
+}
+
+static const struct nla_policy dummyswport_policy[IFLA_DUMMYSWPORT_MAX + 1] = {
+	[IFLA_DUMMYSWPORT_PHYS_SWITCH_ID] = { .type = NLA_BINARY,
+					      .len = MAX_PHYS_ITEM_ID_LEN },
+};
+
+static struct rtnl_link_ops dummyswport_link_ops __read_mostly = {
+	.kind		= "dummyswport",
+	.priv_size	= sizeof(struct dummyswport_priv),
+	.setup		= dummyswport_setup,
+	.validate	= dummyswport_validate,
+	.newlink	= dummyswport_newlink,
+	.policy		= dummyswport_policy,
+	.maxtype	= IFLA_DUMMYSWPORT_MAX,
+};
+
+static int __init dummysw_module_init(void)
+{
+	return rtnl_link_register(&dummyswport_link_ops);
+}
+
+static void __exit dummysw_module_exit(void)
+{
+	rtnl_link_unregister(&dummyswport_link_ops);
+}
+
+module_init(dummysw_module_init);
+module_exit(dummysw_module_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Jiri Pirko <jiri-rHqAuBHg3fBzbRFIqnYvSA@public.gmane.org>");
+MODULE_DESCRIPTION("Dummy switch device");
+MODULE_ALIAS_RTNL_LINK("dummyswport");
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index c5ca3b9..bd24d69 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -574,4 +574,13 @@ enum {

  #define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1)

+/* DUMMYSWPORT section */
+enum {
+	IFLA_DUMMYSWPORT_UNSPEC,
+	IFLA_DUMMYSWPORT_PHYS_SWITCH_ID,
+	__IFLA_DUMMYSWPORT_MAX,
+};
+
+#define IFLA_DUMMYSWPORT_MAX (__IFLA_DUMMYSWPORT_MAX - 1)
+
  #endif /* _UAPI_LINUX_IF_LINK_H */

Re: [patch net-next v2 5/9] net: introduce dummy switch

From: Jiri Pirko <jiri@resnulli.us>
Date: 2014-09-20 07:37:26

Sat, Sep 20, 2014 at 07:21:00AM CEST, f.fainelli@gmail.com wrote:
On 09/19/14 06:49, Jiri Pirko wrote:
quoted
Dummy switch implementation using switchdev interface
This really looks like a DSA driver that has 0 ports, and is not attached to
an useful network interface, and which is registering its own set of rtnl
operations for a purpose that is unclear to me.

I think registering these rtnl ops is misleading as it leads to a false idea
that this is allowed, and that people are actually encouraged to do that for
custom switch drivers, and this completely defeats the purpose of coming up
with a generic API.

If we are to go that route anyway, I really prefer the way Felix did it in
swconfig, and the fake switch driver did do something useful being attached
to the loopback interface:

http://lists.openwall.net/netdev/2013/10/22/103
http://patchwork.ozlabs.org/patch/285478/
I will drop dummyswitch because it serves primary as an example. But
since rocker is here, this is no longer needed.

quoted
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/Kconfig          |   7 +++
 drivers/net/Makefile         |   1 +
 drivers/net/dummyswitch.c    | 130 +++++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/if_link.h |   9 +++
 4 files changed, 147 insertions(+)
 create mode 100644 drivers/net/dummyswitch.c
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index c6f6f69..7822c74 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -71,6 +71,13 @@ config DUMMY
 	  To compile this driver as a module, choose M here: the module
 	  will be called dummy.

+config NET_DUMMY_SWITCH
+	tristate "Dummy switch net driver support"
+	depends on NET_SWITCHDEV
+	---help---
+	  To compile this driver as a module, choose M here: the module
+	  will be called dummyswitch.
+
 config EQUALIZER
 	tristate "EQL (serial line load balancing) support"
 	---help---
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 61aefdd..3c835ba 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -7,6 +7,7 @@
 #
 obj-$(CONFIG_BONDING) += bonding/
 obj-$(CONFIG_DUMMY) += dummy.o
+obj-$(CONFIG_NET_DUMMY_SWITCH) += dummyswitch.o
 obj-$(CONFIG_EQUALIZER) += eql.o
 obj-$(CONFIG_IFB) += ifb.o
 obj-$(CONFIG_MACVLAN) += macvlan.o
diff --git a/drivers/net/dummyswitch.c b/drivers/net/dummyswitch.c
new file mode 100644
index 0000000..e7a48f4
--- /dev/null
+++ b/drivers/net/dummyswitch.c
@@ -0,0 +1,130 @@
+/*
+ * drivers/net/dummyswitch.c - Dummy switch device
+ * Copyright (c) 2014 Jiri Pirko <jiri@resnulli.us>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
+#include <net/rtnetlink.h>
+
+struct dummyswport_priv {
+	struct netdev_phys_item_id psid;
+};
+
+static netdev_tx_t dummyswport_start_xmit(struct sk_buff *skb,
+					  struct net_device *dev)
+{
+	dev_kfree_skb(skb);
+	return NETDEV_TX_OK;
+}
+
+static int dummyswport_swdev_id_get(struct net_device *dev,
+				    struct netdev_phys_item_id *psid)
+{
+	struct dummyswport_priv *dsp = netdev_priv(dev);
+
+	memcpy(psid, &dsp->psid, sizeof(*psid));
+	return 0;
+}
+
+static int dummyswport_change_carrier(struct net_device *dev, bool new_carrier)
+{
+	if (new_carrier)
+		netif_carrier_on(dev);
+	else
+		netif_carrier_off(dev);
+	return 0;
+}
+
+static const struct net_device_ops dummyswport_netdev_ops = {
+	.ndo_start_xmit		= dummyswport_start_xmit,
+	.ndo_swdev_id_get	= dummyswport_swdev_id_get,
+	.ndo_change_carrier	= dummyswport_change_carrier,
+};
+
+static void dummyswport_setup(struct net_device *dev)
+{
+	ether_setup(dev);
+
+	/* Initialize the device structure. */
+	dev->netdev_ops = &dummyswport_netdev_ops;
+	dev->destructor = free_netdev;
+
+	/* Fill in device structure with ethernet-generic values. */
+	dev->tx_queue_len = 0;
+	dev->flags |= IFF_NOARP;
+	dev->flags &= ~IFF_MULTICAST;
+	dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
+	dev->features	|= NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_TSO;
+	dev->features	|= NETIF_F_HW_CSUM | NETIF_F_HIGHDMA | NETIF_F_LLTX;
+	eth_hw_addr_random(dev);
+}
+
+static int dummyswport_validate(struct nlattr *tb[], struct nlattr *data[])
+{
+	if (tb[IFLA_ADDRESS])
+		return -EINVAL;
+	if (!data || !data[IFLA_DUMMYSWPORT_PHYS_SWITCH_ID])
+		return -EINVAL;
+	return 0;
+}
+
+static int dummyswport_newlink(struct net *src_net, struct net_device *dev,
+			       struct nlattr *tb[], struct nlattr *data[])
+{
+	struct dummyswport_priv *dsp = netdev_priv(dev);
+	int err;
+
+	dsp->psid.id_len = nla_len(data[IFLA_DUMMYSWPORT_PHYS_SWITCH_ID]);
+	memcpy(dsp->psid.id, nla_data(data[IFLA_DUMMYSWPORT_PHYS_SWITCH_ID]),
+	       dsp->psid.id_len);
+
+	err = register_netdevice(dev);
+	if (err)
+		return err;
+
+	netif_carrier_on(dev);
+
+	return 0;
+}
+
+static const struct nla_policy dummyswport_policy[IFLA_DUMMYSWPORT_MAX + 1] = {
+	[IFLA_DUMMYSWPORT_PHYS_SWITCH_ID] = { .type = NLA_BINARY,
+					      .len = MAX_PHYS_ITEM_ID_LEN },
+};
+
+static struct rtnl_link_ops dummyswport_link_ops __read_mostly = {
+	.kind		= "dummyswport",
+	.priv_size	= sizeof(struct dummyswport_priv),
+	.setup		= dummyswport_setup,
+	.validate	= dummyswport_validate,
+	.newlink	= dummyswport_newlink,
+	.policy		= dummyswport_policy,
+	.maxtype	= IFLA_DUMMYSWPORT_MAX,
+};
+
+static int __init dummysw_module_init(void)
+{
+	return rtnl_link_register(&dummyswport_link_ops);
+}
+
+static void __exit dummysw_module_exit(void)
+{
+	rtnl_link_unregister(&dummyswport_link_ops);
+}
+
+module_init(dummysw_module_init);
+module_exit(dummysw_module_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Jiri Pirko <jiri@resnulli.us>");
+MODULE_DESCRIPTION("Dummy switch device");
+MODULE_ALIAS_RTNL_LINK("dummyswport");
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index c5ca3b9..bd24d69 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -574,4 +574,13 @@ enum {

 #define IFLA_HSR_MAX (__IFLA_HSR_MAX - 1)

+/* DUMMYSWPORT section */
+enum {
+	IFLA_DUMMYSWPORT_UNSPEC,
+	IFLA_DUMMYSWPORT_PHYS_SWITCH_ID,
+	__IFLA_DUMMYSWPORT_MAX,
+};
+
+#define IFLA_DUMMYSWPORT_MAX (__IFLA_DUMMYSWPORT_MAX - 1)
+
 #endif /* _UAPI_LINUX_IF_LINK_H */
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help