Re: [RFC PATCH v2 net-next 10/15] net: dsa: Pass ndo_setup_tc slave callback to drivers
From: Kurt Kanzenbach <hidden>
Date: 2019-09-02 07:52:32
Subsystem:
networking [dsa], networking [general], the rest · Maintainers:
Andrew Lunn, Vladimir Oltean, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
Hi, On Fri, Aug 30, 2019 at 03:46:30AM +0300, Vladimir Oltean wrote:
DSA currently handles shared block filters (for the classifier-action qdisc) in the core due to what I believe are simply pragmatic reasons - hiding the complexity from drivers and offerring a simple API for port mirroring. Extend the dsa_slave_setup_tc function by passing all other qdisc offloads to the driver layer, where the driver may choose what it implements and how. DSA is simply a pass-through in this case.
I'm having the same problem on how to pass the taprio schedule down to the DSA driver. I didn't perform a pass-through to keep it in sync with the already implemented offload. See my approach below.
quoted hunk ↗ jump to hunk
There is an open question related to the drivers potentially needing to do work in process context, but .ndo_setup_tc is called in atomic context. At the moment the drivers are left to handle this on their own. The risk is that once accepting the offload callback right away in the DSA core, then the driver would have no way to signal an error back. So right now the driver has to do as much error checking as possible in the atomic context and only defer (probably) the actual configuring of the offload. Signed-off-by: Vladimir Oltean <olteanv@gmail.com> --- include/net/dsa.h | 3 +++ net/dsa/slave.c | 12 ++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-)diff --git a/include/net/dsa.h b/include/net/dsa.h index 96acb14ec1a8..232b5d36815d 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h@@ -154,6 +154,7 @@ struct dsa_mall_tc_entry { }; }; +struct tc_taprio_qopt_offload;
Is this needed? The rest looks good to me. My approach:
diff --git a/include/net/dsa.h b/include/net/dsa.h
index ba6dfff98196..a60bd55f27f2 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h@@ -20,6 +20,7 @@ #include <linux/platform_data/dsa.h> #include <net/devlink.h> #include <net/switchdev.h> +#include <net/pkt_sched.h> struct tc_action; struct phy_device;
@@ -539,6 +540,13 @@ struct dsa_switch_ops { */ netdev_tx_t (*port_deferred_xmit)(struct dsa_switch *ds, int port, struct sk_buff *skb); + + /* + * Scheduled traffic functionality + */ + int (*port_set_schedule)(struct dsa_switch *ds, int port, + const struct tc_taprio_qopt_offload *taprio); + int (*port_del_schedule)(struct dsa_switch *ds, int port); }; struct dsa_switch_driver {
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 8157be7e162d..6290d55e6011 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c@@ -15,6 +15,7 @@ #include <linux/mdio.h> #include <net/rtnetlink.h> #include <net/pkt_cls.h> +#include <net/pkt_sched.h> #include <net/tc_act/tc_mirred.h> #include <linux/if_bridge.h> #include <linux/netpoll.h>
@@ -953,12 +954,33 @@ static int dsa_slave_setup_tc_block(struct net_device *dev, } } +static int dsa_slave_setup_tc_taprio(struct net_device *dev, + const struct tc_taprio_qopt_offload *taprio) +{ + struct dsa_port *dp = dsa_slave_to_port(dev); + struct dsa_switch *ds = dp->ds; + + if (taprio->enable) { + if (!ds->ops->port_set_schedule) + return -EOPNOTSUPP; + + return ds->ops->port_set_schedule(ds, dp->index, taprio); + } + + if (!ds->ops->port_del_schedule) + return -EOPNOTSUPP; + + return ds->ops->port_del_schedule(ds, dp->index); +} + static int dsa_slave_setup_tc(struct net_device *dev, enum tc_setup_type type, void *type_data) { switch (type) { case TC_SETUP_BLOCK: return dsa_slave_setup_tc_block(dev, type_data); + case TC_SETUP_QDISC_TAPRIO: + return dsa_slave_setup_tc_taprio(dev, type_data); default: return -EOPNOTSUPP; }
Thanks, Kurt
Attachments
- signature.asc [application/pgp-signature] 833 bytes