Re: [PATCH net-next 11/17] net: dsa: Refactor selection of tag ops into a function

[Vivien Didelot]

Andrew Lunn  writes:

> Replace the two switch statements with an array lookup, and store the
> result in the dsa tree structure. The drivers no longer need to know
> the selected tag protocol, so remove it from the dsa switch structure.
>
> Signed-off-by: Andrew Lunn 

Reviewed-by: Vivien Didelot 

Re: [PATCH net-next 11/17] net: dsa: Refactor selection of tag ops into a function

[Florian Fainelli]

On 06/03/2016 09:44 AM, Andrew Lunn wrote:
> Replace the two switch statements with an array lookup, and store the
> result in the dsa tree structure. The drivers no longer need to know
> the selected tag protocol, so remove it from the dsa switch structure.
> 
> Signed-off-by: Andrew Lunn 

Reviewed-by: Florian Fainelli 
-- 
Florian

[PATCH net-next 11/17] net: dsa: Refactor selection of tag ops into a function

[Andrew Lunn]

Replace the two switch statements with an array lookup, and store the
result in the dsa tree structure. The drivers no longer need to know
the selected tag protocol, so remove it from the dsa switch structure.

Signed-off-by: Andrew Lunn 
---
rfc->v1:
Rename _DSA_TAG_LAST to DSA_TAG_LAST and add a comment.
---
 include/net/dsa.h  |  8 +-
 net/dsa/dsa.c  | 71 ++
 net/dsa/dsa_priv.h |  1 +
 net/dsa/slave.c| 35 +--
 4 files changed, 54 insertions(+), 61 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index a306a17b7f2e..11e8f09d32e3 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -26,6 +26,7 @@ enum dsa_tag_protocol {
DSA_TAG_PROTO_TRAILER,
DSA_TAG_PROTO_EDSA,
DSA_TAG_PROTO_BRCM,
+   DSA_TAG_LAST,   /* MUST BE LAST */
 };
 
 #define DSA_MAX_SWITCHES   4
@@ -99,7 +100,6 @@ struct dsa_switch_tree {
   struct net_device *dev,
   struct packet_type *pt,
   struct net_device *orig_dev);
-   enum dsa_tag_protocol   tag_protocol;
 
/*
 * Original copy of the master netdev ethtool_ops
@@ -116,6 +116,12 @@ struct dsa_switch_tree {
 * Data for the individual switch chips.
 */
struct dsa_switch   *ds[DSA_MAX_SWITCHES];
+
+   /*
+* Tagging protocol operations for adding and removing an
+* encapsulation tag.
+*/
+   const struct dsa_device_ops *tag_ops;
 };
 
 struct dsa_port {
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 7140de475c07..221ebde4318d 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -29,6 +29,33 @@
 
 char dsa_driver_version[] = "0.1";
 
+static struct sk_buff *dsa_slave_notag_xmit(struct sk_buff *skb,
+   struct net_device *dev)
+{
+   /* Just return the original SKB */
+   return skb;
+}
+
+static const struct dsa_device_ops none_ops = {
+   .xmit   = dsa_slave_notag_xmit,
+   .rcv= NULL,
+};
+
+const struct dsa_device_ops *dsa_device_ops[DSA_TAG_LAST] = {
+#ifdef CONFIG_NET_DSA_TAG_DSA
+   [DSA_TAG_PROTO_DSA] = _netdev_ops,
+#endif
+#ifdef CONFIG_NET_DSA_TAG_EDSA
+   [DSA_TAG_PROTO_EDSA] = _netdev_ops,
+#endif
+#ifdef CONFIG_NET_DSA_TAG_TRAILER
+   [DSA_TAG_PROTO_TRAILER] = _netdev_ops,
+#endif
+#ifdef CONFIG_NET_DSA_TAG_BRCM
+   [DSA_TAG_PROTO_BRCM] = _netdev_ops,
+#endif
+   [DSA_TAG_PROTO_NONE] = _ops,
+};
 
 /* switch driver registration ***/
 static DEFINE_MUTEX(dsa_switch_drivers_mutex);
@@ -225,6 +252,20 @@ static int dsa_cpu_dsa_setups(struct dsa_switch *ds, 
struct device *dev)
return 0;
 }
 
+const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol)
+{
+   const struct dsa_device_ops *ops;
+
+   if (tag_protocol >= DSA_TAG_LAST)
+   return ERR_PTR(-EINVAL);
+   ops = dsa_device_ops[tag_protocol];
+
+   if (!ops)
+   return ERR_PTR(-ENOPROTOOPT);
+
+   return ops;
+}
+
 static int dsa_switch_setup_one(struct dsa_switch *ds, struct device *parent)
 {
struct dsa_switch_driver *drv = ds->drv;
@@ -277,35 +318,13 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, 
struct device *parent)
 * switch.
 */
if (dst->cpu_switch == index) {
-   switch (drv->tag_protocol) {
-#ifdef CONFIG_NET_DSA_TAG_DSA
-   case DSA_TAG_PROTO_DSA:
-   dst->rcv = dsa_netdev_ops.rcv;
-   break;
-#endif
-#ifdef CONFIG_NET_DSA_TAG_EDSA
-   case DSA_TAG_PROTO_EDSA:
-   dst->rcv = edsa_netdev_ops.rcv;
-   break;
-#endif
-#ifdef CONFIG_NET_DSA_TAG_TRAILER
-   case DSA_TAG_PROTO_TRAILER:
-   dst->rcv = trailer_netdev_ops.rcv;
-   break;
-#endif
-#ifdef CONFIG_NET_DSA_TAG_BRCM
-   case DSA_TAG_PROTO_BRCM:
-   dst->rcv = brcm_netdev_ops.rcv;
-   break;
-#endif
-   case DSA_TAG_PROTO_NONE:
-   break;
-   default:
-   ret = -ENOPROTOOPT;
+   dst->tag_ops = dsa_resolve_tag_protocol(drv->tag_protocol);
+   if (IS_ERR(dst->tag_ops)) {
+   ret = PTR_ERR(dst->tag_ops);
goto out;
}
 
-   dst->tag_protocol = drv->tag_protocol;
+   dst->rcv = dst->tag_ops->rcv;
}
 
memcpy(ds->rtable, cd->rtable, sizeof(ds->rtable));
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index dbea5d9e7f75..72f7b8989cfb 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -53,6 +53,7 @@ extern char dsa_driver_version[];
 int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct