Messages in this thread | | | Date | Fri, 24 May 2019 13:40:52 +0200 | From | Joergen Andreasen <> | Subject | Re: [PATCH net-next v2 1/1] net: mscc: ocelot: Implement port policers via tc command |
| |
Hi Jakub,
The 05/23/2019 11:56, Jakub Kicinski wrote: > External E-Mail > > > On Thu, 23 May 2019 12:49:39 +0200, Joergen Andreasen wrote: > > Hardware offload of matchall classifier and police action are now > > supported via the tc command. > > Supported police parameters are: rate and burst. > > > > Example: > > > > Add: > > tc qdisc add dev eth3 handle ffff: ingress > > tc filter add dev eth3 parent ffff: prio 1 handle 2 \ > > matchall skip_sw \ > > action police rate 100Mbit burst 10000 > > > > Show: > > tc -s -d qdisc show dev eth3 > > tc -s -d filter show dev eth3 ingress > > > > Delete: > > tc filter del dev eth3 parent ffff: prio 1 > > tc qdisc del dev eth3 handle ffff: ingress > > > > Signed-off-by: Joergen Andreasen <joergen.andreasen@microchip.com> > > > diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c > > index d715ef4fc92f..3ec7864d9dc8 100644 > > --- a/drivers/net/ethernet/mscc/ocelot.c > > +++ b/drivers/net/ethernet/mscc/ocelot.c > > @@ -943,6 +943,7 @@ static const struct net_device_ops ocelot_port_netdev_ops = { > > .ndo_vlan_rx_kill_vid = ocelot_vlan_rx_kill_vid, > > .ndo_set_features = ocelot_set_features, > > .ndo_get_port_parent_id = ocelot_get_port_parent_id, > > + .ndo_setup_tc = ocelot_setup_tc, > > }; > > > > static void ocelot_get_strings(struct net_device *netdev, u32 sset, u8 *data) > > @@ -1663,8 +1664,9 @@ int ocelot_probe_port(struct ocelot *ocelot, u8 port, > > dev->netdev_ops = &ocelot_port_netdev_ops; > > dev->ethtool_ops = &ocelot_ethtool_ops; > > > > - dev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_RXFCS; > > - dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER; > > + dev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_RXFCS | > > + NETIF_F_HW_TC; > > + dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_TC; > > > > memcpy(dev->dev_addr, ocelot->base_mac, ETH_ALEN); > > dev->dev_addr[ETH_ALEN - 1] += port; > > You need to add a check in set_features to make sure nobody clears the > NETIF_F_TC flag while something is offloaded, otherwise you will miss > the REMOVE callback (it will bounce from the > tc_cls_can_offload_and_chain0() check).
I will add this check in v3
> > > diff --git a/drivers/net/ethernet/mscc/ocelot_tc.c b/drivers/net/ethernet/mscc/ocelot_tc.c > > new file mode 100644 > > index 000000000000..2412e0dbc267 > > --- /dev/null > > +++ b/drivers/net/ethernet/mscc/ocelot_tc.c > > @@ -0,0 +1,164 @@ > > +// SPDX-License-Identifier: (GPL-2.0 OR MIT) > > +/* Microsemi Ocelot Switch TC driver > > + * > > + * Copyright (c) 2019 Microsemi Corporation > > + */ > > + > > +#include "ocelot_tc.h" > > +#include "ocelot_police.h" > > +#include <net/pkt_cls.h> > > + > > +static int ocelot_setup_tc_cls_matchall(struct ocelot_port *port, > > + struct tc_cls_matchall_offload *f, > > + bool ingress) > > +{ > > + struct netlink_ext_ack *extack = f->common.extack; > > + struct ocelot_policer pol = { 0 }; > > + struct flow_action_entry *action; > > + int err; > > + > > + netdev_dbg(port->dev, "%s: port %u command %d cookie %lu\n", > > + __func__, port->chip_port, f->command, f->cookie); > > + > > + if (!ingress) { > > + NL_SET_ERR_MSG_MOD(extack, "Only ingress is supported"); > > + return -EOPNOTSUPP; > > + } > > + > > + switch (f->command) { > > + case TC_CLSMATCHALL_REPLACE: > > + if (!flow_offload_has_one_action(&f->rule->action)) { > > + NL_SET_ERR_MSG_MOD(extack, > > + "Only one action is supported"); > > + return -EOPNOTSUPP; > > + } > > + > > + action = &f->rule->action.entries[0]; > > + > > + if (action->id != FLOW_ACTION_POLICE) { > > + NL_SET_ERR_MSG_MOD(extack, "Unsupported action"); > > + return -EOPNOTSUPP; > > + } > > Please also reject the offload if block is shared, as HW policer state > cannot be shared between ports, the way it is in SW. You have to save > whether the block is shared or not at bind time, see: > > d6787147e15d ("net/sched: remove block pointer from common offload structure")
I will fix this in v3.
> > > + if (port->tc.police_id && port->tc.police_id != f->cookie) { > > + NL_SET_ERR_MSG_MOD(extack, > > + "Only one policer per port is supported\n"); > > + return -EEXIST; > > + } > > + > > + pol.rate = (u32)div_u64(action->police.rate_bytes_ps, 1000) * 8; > > + pol.burst = (u32)div_u64(action->police.rate_bytes_ps * > > + PSCHED_NS2TICKS(action->police.burst), > > + PSCHED_TICKS_PER_SEC); > > + > > + err = ocelot_port_policer_add(port, &pol); > > + if (err) { > > + NL_SET_ERR_MSG_MOD(extack, "Could not add policer\n"); > > + return err; > > + } > > + > > + port->tc.police_id = f->cookie; > > + return 0; > > + case TC_CLSMATCHALL_DESTROY: > > + if (port->tc.police_id != f->cookie) > > + return -ENOENT; > > + > > + err = ocelot_port_policer_del(port); > > + if (err) { > > + NL_SET_ERR_MSG_MOD(extack, > > + "Could not delete policer\n"); > > + return err; > > + } > > + port->tc.police_id = 0; > > + return 0; > > + case TC_CLSMATCHALL_STATS: /* fall through */ > > + default: > > + return -EOPNOTSUPP; > > + } > > +} >
-- Joergen Andreasen, Microchip
| |