| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20190524114050.rznhisqcgdm5c2e6@soft-dev16>
Date: Fri, 24 May 2019 13:40:52 +0200
From: Joergen Andreasen <joergen.andreasen@...rochip.com>
To: Jakub Kicinski <jakub.kicinski@...ronome.com>
CC: <netdev@...r.kernel.org>,
Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
"David S. Miller" <davem@...emloft.net>,
<linux-kernel@...r.kernel.org>
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@...rochip.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
Powered by blists - more mailing lists