| Message ID | 20210615125444.31538-1-vadym.kochan@plvision.eu |
|---|---|
| Headers | show |
| Series | Marvell Prestera add flower and match all support | expand |
From: Vadym Kochan <vadym.kochan@plvision.eu> Date: Tue, 15 Jun 2021 15:54:42 +0300 > From: Vadym Kochan <vkochan@marvell.com> > > Add ACL infrastructure for Prestera Switch ASICs family devices to > offload cls_flower rules to be processed in the HW. Please fix this and resubmit, thank you: Applying: net: marvell: Implement TC flower offload .git/rebase-apply/patch:805: new blank line at EOF. + warning: 1 line adds whitespace errors. Applying: net: marvell: prestera: Add matchall support .git/rebase-apply/patch:538: new blank line at EOF. + warning: 1 line adds whitespace errors. Switched to branch 'master' Your branch is up to date with 'origin/master'. hint: Waiting for your editor to close the file...
On Tue, Jun 15, 2021 at 03:54:43PM +0300, Vadym Kochan wrote: > +static int prestera_port_set_features(struct net_device *dev, > + netdev_features_t features) > +{ > + netdev_features_t oper_features = dev->features; > + int err; > + > + err = prestera_port_handle_feature(dev, features, NETIF_F_HW_TC, > + prestera_port_feature_hw_tc); Why do you even make NETIF_F_HW_TC able to be toggled and not just fixed to "on" in dev->features? If I understand correctly, you could then delete a bunch of refcounting code whose only purpose is to allow that feature to be disabled per port. > + > + if (err) { > + dev->features = oper_features; > + return -EINVAL; > + } > + > + return 0; > +}
Hi Vladimir, On Wed, Jun 16, 2021 at 03:54:53AM +0300, Vladimir Oltean wrote: > On Tue, Jun 15, 2021 at 03:54:43PM +0300, Vadym Kochan wrote: > > +static int prestera_port_set_features(struct net_device *dev, > > + netdev_features_t features) > > +{ > > + netdev_features_t oper_features = dev->features; > > + int err; > > + > > + err = prestera_port_handle_feature(dev, features, NETIF_F_HW_TC, > > + prestera_port_feature_hw_tc); > > Why do you even make NETIF_F_HW_TC able to be toggled and not just fixed > to "on" in dev->features? If I understand correctly, you could then delete > a bunch of refcounting code whose only purpose is to allow that feature > to be disabled per port. > The only case where it can be used is when user want to disable TC offloading and apply set of rules w/o skip_hw. So you think it is OK to not having an ability to disable offloading at all ? > > + > > + if (err) { > > + dev->features = oper_features; > > + return -EINVAL; > > + } > > + > > + return 0; > > +}
On Wed, Jun 16, 2021 at 04:04:24PM +0300, Vadym Kochan wrote: > Hi Vladimir, > > On Wed, Jun 16, 2021 at 03:54:53AM +0300, Vladimir Oltean wrote: > > On Tue, Jun 15, 2021 at 03:54:43PM +0300, Vadym Kochan wrote: > > > +static int prestera_port_set_features(struct net_device *dev, > > > + netdev_features_t features) > > > +{ > > > + netdev_features_t oper_features = dev->features; > > > + int err; > > > + > > > + err = prestera_port_handle_feature(dev, features, NETIF_F_HW_TC, > > > + prestera_port_feature_hw_tc); > > > > Why do you even make NETIF_F_HW_TC able to be toggled and not just fixed > > to "on" in dev->features? If I understand correctly, you could then delete > > a bunch of refcounting code whose only purpose is to allow that feature > > to be disabled per port. > > > > The only case where it can be used is when user want to disable TC > offloading and apply set of rules w/o skip_hw. > > So you think it is OK to not having an ability to disable offloading at > all ? Because adding "skip_hw" is already possible per filter in the first place, I don't think that this feature justifies the added complexity, no.
From: Vadym Kochan <vkochan@marvell.com> Add ACL infrastructure for Prestera Switch ASICs family devices to offload cls_flower rules to be processed in the HW. ACL implementation is based on tc filter api. The flower classifier is supported to configure ACL rules/matches/action. Supported actions: - drop - trap - pass Supported dissector keys: - indev - src_mac - dst_mac - src_ip - dst_ip - ip_proto - src_port - dst_port - vlan_id - vlan_ethtype - icmp type/code - Introduce matchall filter support - Add SPAN API to configure port mirroring. - Add tc mirror action. At this moment, only mirror (egress) action is supported. Example: tc filter ... action mirred egress mirror dev DEV Serhiy Boiko (2): net: marvell: Implement TC flower offload net: marvell: prestera: Add matchall support .../net/ethernet/marvell/prestera/Makefile | 3 +- .../net/ethernet/marvell/prestera/prestera.h | 7 + .../ethernet/marvell/prestera/prestera_acl.c | 400 ++++++++++++++++++ .../ethernet/marvell/prestera/prestera_acl.h | 130 ++++++ .../ethernet/marvell/prestera/prestera_flow.c | 215 ++++++++++ .../ethernet/marvell/prestera/prestera_flow.h | 14 + .../marvell/prestera/prestera_flower.c | 359 ++++++++++++++++ .../marvell/prestera/prestera_flower.h | 18 + .../ethernet/marvell/prestera/prestera_hw.c | 361 ++++++++++++++++ .../ethernet/marvell/prestera/prestera_hw.h | 23 + .../ethernet/marvell/prestera/prestera_main.c | 98 ++++- .../ethernet/marvell/prestera/prestera_span.c | 245 +++++++++++ .../ethernet/marvell/prestera/prestera_span.h | 20 + 13 files changed, 1891 insertions(+), 2 deletions(-) create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_acl.c create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_acl.h create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_flow.c create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_flow.h create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_flower.c create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_flower.h create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_span.c create mode 100644 drivers/net/ethernet/marvell/prestera/prestera_span.h