Messages in this thread | ![/](/images/icornerl.gif) | | Date | Sun, 16 Apr 2023 19:56:58 +0300 | From | Vladimir Oltean <> | Subject | Re: [PATCH net-next v1 2/2] net: dsa: microchip: Add partial ACL support for ksz9477 switches |
| |
Hi Oleksij,
I only took a superficial look, and hence, here are some superficial comments.
On Tue, Apr 11, 2023 at 07:24:55PM +0200, Oleksij Rempel wrote: > The ACL also implements a count function, generating an interrupt > instead of a forwarding action. It can be used as a watchdog timer or an > event counter.
Is the interrupt handled here? I didn't see cls_flower_stats().
> The ACL consists of three parts: matching rules, action > rules, and processing entries. Multiple match conditions can be either > AND'ed or OR'ed together. > > This patch introduces support for a subset of the available ACL > functionality, specifically layer 2 matching and prioritization of > matched packets. For example: > > tc qdisc add dev lan2 clsact > tc filter add dev lan2 ingress protocol 0x88f7 flower skip_sw hw_tc 7 > > tc qdisc add dev lan1 clsact > tc filter add dev lan1 ingress protocol 0x88f7 flower skip_sw hw_tc 7
Have you considered the "skbedit priority" action as opposed to hw_tc?
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> > --- > drivers/net/dsa/microchip/Makefile | 2 +- > drivers/net/dsa/microchip/ksz9477.c | 7 + > drivers/net/dsa/microchip/ksz9477.h | 7 + > drivers/net/dsa/microchip/ksz9477_acl.c | 950 ++++++++++++++++++++++++ > drivers/net/dsa/microchip/ksz_common.c | 40 + > drivers/net/dsa/microchip/ksz_common.h | 1 + > 6 files changed, 1006 insertions(+), 1 deletion(-) > create mode 100644 drivers/net/dsa/microchip/ksz9477_acl.c > > diff --git a/drivers/net/dsa/microchip/Makefile b/drivers/net/dsa/microchip/Makefile > index 48360cc9fc68..50851519c9a1 100644 > --- a/drivers/net/dsa/microchip/Makefile > +++ b/drivers/net/dsa/microchip/Makefile > @@ -1,7 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0-only > obj-$(CONFIG_NET_DSA_MICROCHIP_KSZ_COMMON) += ksz_switch.o > ksz_switch-objs := ksz_common.o > -ksz_switch-objs += ksz9477.o > +ksz_switch-objs += ksz9477.o ksz9477_acl.o > ksz_switch-objs += ksz8795.o > ksz_switch-objs += lan937x_main.o > > diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c > index bf13d47c26cf..f2c34ab3d3eb 100644 > --- a/drivers/net/dsa/microchip/ksz9477.c > +++ b/drivers/net/dsa/microchip/ksz9477.c > @@ -1151,6 +1151,7 @@ int ksz9477_setup(struct dsa_switch *ds) > { > struct ksz_device *dev = ds->priv; > int ret = 0; > + int i; > > ds->mtu_enforcement_ingress = true; > > @@ -1176,6 +1177,12 @@ int ksz9477_setup(struct dsa_switch *ds) > /* enable global MIB counter freeze function */ > ksz_cfg(dev, REG_SW_MAC_CTRL_6, SW_MIB_COUNTER_FREEZE, true); > > + for (i = 0; i < dev->info->port_cnt; i++) { > + if (i == dev->cpu_port) > + continue;
dsa_switch_for_each_user_port()
> + ksz9477_port_acl_init(dev, i);
Don't ignore the return code.
> + } > + > return 0; > } > > diff --git a/drivers/net/dsa/microchip/ksz9477.h b/drivers/net/dsa/microchip/ksz9477.h > index b6f7e3c46e3f..5201bccda0ed 100644 > --- a/drivers/net/dsa/microchip/ksz9477.h > +++ b/drivers/net/dsa/microchip/ksz9477.h > @@ -59,4 +59,11 @@ int ksz9477_switch_init(struct ksz_device *dev); > void ksz9477_switch_exit(struct ksz_device *dev); > void ksz9477_port_queue_split(struct ksz_device *dev, int port); > > + > +int ksz9477_port_acl_init(struct ksz_device *dev, int port); > +int ksz9477_cls_flower_add(struct dsa_switch *ds, int port, > + struct flow_cls_offload *cls, bool ingress); > +int ksz9477_cls_flower_del(struct dsa_switch *ds, int port, > + struct flow_cls_offload *cls, bool ingress); > + > #endif > diff --git a/drivers/net/dsa/microchip/ksz9477_acl.c b/drivers/net/dsa/microchip/ksz9477_acl.c > new file mode 100644 > index 000000000000..319158ebbdd9 > --- /dev/null > +++ b/drivers/net/dsa/microchip/ksz9477_acl.c > +#define KSZ9477_ACL_ENTRY_SIZE 18 > +#define KSZ9477_ACL_MAX_ENTRIES 16 > +#define KSZ9477_MAC_TC 7
MAC_TC or MAX_TC?
> + if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ETH_ADDRS)) { > + u8 bcast[] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; > + > + flow_rule_match_eth_addrs(rule, &ematch); > + > + if (!is_zero_ether_addr(ematch.key->src)) { > + if (!ether_addr_equal(ematch.mask->src, bcast))
is_broadcast_ether_addr()
> + goto not_full_mask_err; > + > + src_mac = ematch.key->src; > + } > + > + if (!is_zero_ether_addr(ematch.key->dst)) { > + if (!ether_addr_equal(ematch.mask->dst, bcast)) > + goto not_full_mask_err; > + > + dst_mac = ematch.key->dst; > + } > + } > + > + acles = &acl->acles; > + /* ACL supports only one MAC per entry */ > + required_entries = src_mac && dst_mac ? 2 : 1;
| ![\](/images/icornerr.gif) |