| Message ID | 20210113154139.1803705-1-olteanv@gmail.com |
|---|---|
| Headers | show |
| Series | Port-based priority on DSA switches using tc-matchall | expand |
On Wed, Jan 13, 2021 at 05:41:39PM +0200, Vladimir Oltean wrote: > From: Vladimir Oltean <vladimir.oltean@nxp.com> > > Even though we should really share the implementation with the ocelot > switchdev driver, that one needs a little bit of rework first, since its > struct ocelot_port_tc only supports one tc matchall action at a time, > which at the moment is used for port policers. Whereas DSA keeps a list > of port-based actions in struct dsa_slave_priv::mall_tc_list, so it is > much more easily extensible. It is too tempting to add the implementation > for the port priority directly in Felix at the moment, which is what we > do. > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > --- > drivers/net/dsa/ocelot/felix.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c > index 768a74dc462a..5cc42c3aaf0d 100644 > --- a/drivers/net/dsa/ocelot/felix.c > +++ b/drivers/net/dsa/ocelot/felix.c > @@ -739,6 +739,20 @@ static void felix_port_policer_del(struct dsa_switch *ds, int port) > ocelot_port_policer_del(ocelot, port); > } > > +static int felix_port_priority_set(struct dsa_switch *ds, int port, > + struct dsa_mall_skbedit_tc_entry *skbedit) > +{ > + struct ocelot *ocelot = ds->priv; > + > + ocelot_rmw_gix(ocelot, > + ANA_PORT_QOS_CFG_QOS_DEFAULT_VAL(skbedit->priority), No range check? Seems like -ERANGE or similar would help avoid surprises when somebody asks for an unsupported priority and it gets masked to something much lower. Andrew
On 1/13/21 3:36 PM, Andrew Lunn wrote: > On Wed, Jan 13, 2021 at 05:41:39PM +0200, Vladimir Oltean wrote: >> From: Vladimir Oltean <vladimir.oltean@nxp.com> >> >> Even though we should really share the implementation with the ocelot >> switchdev driver, that one needs a little bit of rework first, since its >> struct ocelot_port_tc only supports one tc matchall action at a time, >> which at the moment is used for port policers. Whereas DSA keeps a list >> of port-based actions in struct dsa_slave_priv::mall_tc_list, so it is >> much more easily extensible. It is too tempting to add the implementation >> for the port priority directly in Felix at the moment, which is what we >> do. >> >> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> >> --- >> drivers/net/dsa/ocelot/felix.c | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c >> index 768a74dc462a..5cc42c3aaf0d 100644 >> --- a/drivers/net/dsa/ocelot/felix.c >> +++ b/drivers/net/dsa/ocelot/felix.c >> @@ -739,6 +739,20 @@ static void felix_port_policer_del(struct dsa_switch *ds, int port) >> ocelot_port_policer_del(ocelot, port); >> } >> >> +static int felix_port_priority_set(struct dsa_switch *ds, int port, >> + struct dsa_mall_skbedit_tc_entry *skbedit) >> +{ >> + struct ocelot *ocelot = ds->priv; >> + >> + ocelot_rmw_gix(ocelot, >> + ANA_PORT_QOS_CFG_QOS_DEFAULT_VAL(skbedit->priority), > > No range check? Seems like -ERANGE or similar would help avoid > surprises when somebody asks for an unsupported priority and it gets > masked to something much lower. You are passing the whole dsa_mall_skbedit_tc_entry structure here, only to look up priority, would it make sense for now to pass skbedit->priority as a parameter which would be matching the function name and what it is dealing with?
On Wed, Jan 13, 2021 at 03:37:49PM -0800, Florian Fainelli wrote: > You are passing the whole dsa_mall_skbedit_tc_entry structure here, > only to look up priority, would it make sense for now to pass > skbedit->priority as a parameter which would be matching the function > name and what it is dealing with? Actually I am passing a pointer to it, which should be more or less equal in size to an integer. But I can pass just the priority, sure.
From: Vladimir Oltean <vladimir.oltean@nxp.com> This is a proposal for configuring the port-based default priority on switch ports using tc-matchall and skbedit priority. Comments welcome. Vladimir Oltean (2): net: dsa: allow setting port-based QoS priority using tc matchall skbedit net: dsa: felix: offload port priority drivers/net/dsa/ocelot/felix.c | 15 +++++++ include/net/dsa.h | 8 ++++ net/dsa/slave.c | 72 ++++++++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+)