[net-next,v2,2/5] net: microchip: sparx5: Adding more tc flower keys for the IS2 VCAP
Commit Message
This adds the following TC flower filter keys to Sparx5 for IS2:
- ipv4_addr (sip and dip)
- ipv6_addr (sip and dip)
- control (IPv4 fragments)
- portnum (tcp and udp port numbers)
- basic (L3 and L4 protocol)
- vlan (outer vlan tag info)
- tcp (tcp flags)
- ip (tos field)
as well as an 128 bit keyfield interface on the VCAP API to set the IPv6
addresses.
IS2 supports the classified VLAN information which amounts to the outer
VLAN info in case of multiple tags.
Here are some examples of the tc flower filter operations that are now
supported for the IS2 VCAP:
- IPv4 Addresses
tc filter add dev eth12 ingress chain 8000000 prio 12 handle 12 \
protocol ip flower skip_sw dst_ip 1.0.1.1 src_ip 2.0.2.2 \
action trap
- IPv6 Addresses
tc filter add dev eth12 ingress chain 8000000 prio 13 handle 13 \
protocol ipv6 flower skip_sw dst_ip 1::1:1 src_ip 2::2:2 \
action trap
- IPv4 fragments
tc filter add dev eth12 ingress chain 8000000 prio 14 handle 14 \
protocol ip flower skip_sw dst_ip 3.0.3.3 src_ip 2.0.2.2 \
ip_flags frag/nofirstfrag action trap
- TCP and UDP portnumbers
tc filter add dev eth12 ingress chain 8000000 prio 21 handle 21 \
protocol ip flower skip_sw dst_ip 8.8.8.8 src_ip 2.0.2.2 \
ip_proto tcp dst_port 100 src_port 12000 action trap
tc filter add dev eth12 ingress chain 8000000 prio 23 handle 23 \
protocol ipv6 flower skip_sw dst_ip 5::5:5 src_ip 2::2:2 \
ip_proto tcp dst_port 300 src_port 13000 action trap
- Layer 3 and Layer 4 protocol info
tc filter add dev eth12 ingress chain 8000000 prio 28 handle 28 \
protocol ipv4 flower skip_sw dst_ip 9.0.9.9 src_ip 2.0.2.2 \
ip_proto icmp action trap
- VLAN tag info (outer tag)
tc filter add dev eth12 ingress chain 8000000 prio 29 handle 29 \
protocol 802.1q flower skip_sw vlan_id 600 vlan_prio 6 \
vlan_ethtype ipv4 action trap
tc filter add dev eth12 ingress chain 8000000 prio 31 handle 31 \
protocol 802.1q flower skip_sw vlan_id 600 vlan_prio 5 \
vlan_ethtype ipv6 action trap
- TCP flags
tc filter add dev eth12 ingress chain 8000000 prio 15 handle 15 \
protocol ip flower skip_sw dst_ip 4.0.4.4 src_ip 2.0.2.2 \
ip_proto tcp tcp_flags 0x2a/0x3f action trap
- IP info (IPv4 TOS field)
tc filter add dev eth12 ingress chain 8000000 prio 16 handle 16 \
protocol ip flower skip_sw ip_tos 0x35 dst_ip 5.0.5.5 \
src_ip 2.0.2.2 action trap
The "protocol all" selection is not supported yet.
Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
---
.../microchip/sparx5/sparx5_tc_flower.c | 384 +++++++++++++++++-
.../net/ethernet/microchip/vcap/vcap_api.c | 11 +
.../ethernet/microchip/vcap/vcap_api_client.h | 2 +
3 files changed, 396 insertions(+), 1 deletion(-)
Comments
Hi Steen,
On 2022-10-28 16:45, Steen Hegelund wrote:
> - IPv4 Addresses
> tc filter add dev eth12 ingress chain 8000000 prio 12 handle 12 \
> protocol ip flower skip_sw dst_ip 1.0.1.1 src_ip 2.0.2.2 \
> action trap
I'm not able to get this working on PCB135. I tested the VLAN tags and
did not work either (did not test the rest). The example from the
previous patch series doesn't work either after applying this series.
tc qdisc add dev eth3 clsact
tc filter add dev eth3 ingress chain 8000000 prio 10 handle 10 \
protocol all flower skip_sw \
dst_mac 0a:0b:0c:0d:0e:0f \
src_mac 2:0:0:0:0:1 \
action trap
This example was provided in your last patch series and worked earlier.
My setup is PC-eth0 -> PCB135-eth3 and I use the following EasyFrames
command to send packets:
ef tx eth0 rep 50 eth smac 02:00:00:00:00:01 dmac 0a:0b:0c:0d:0e:0f
IPv4:
tc qdisc add dev eth3 clsact
tc filter add dev eth3 ingress chain 8000000 prio 12 handle 12 \
protocol ip flower skip_sw dst_ip 1.0.1.1 src_ip 2.0.2.2 \
action trap
ef tx eth0 rep 50 eth smac 02:00:00:00:00:01 dmac 0a:0b:0c:0d:0e:0f ipv4 dip 1.0.1.1 sip 2.0.2.2
Same setup as above and I can't get this to work either.
I'm using tcpdump to watch the interface to see if the packets are being
trapped or not. Changing the packets' dmac to broadcast lets me see the
packets so I don't have any issue with the setup.
BR,
Casper
Hi Casper,
First of all thanks for the testing effort (as usual). This is most welcome.
On Mon, 2022-10-31 at 11:44 +0100, Casper Andersson wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Hi Steen,
>
> On 2022-10-28 16:45, Steen Hegelund wrote:
> > - IPv4 Addresses
> > tc filter add dev eth12 ingress chain 8000000 prio 12 handle 12 \
> > protocol ip flower skip_sw dst_ip 1.0.1.1 src_ip 2.0.2.2 \
> > action trap
>
> I'm not able to get this working on PCB135. I tested the VLAN tags and
> did not work either (did not test the rest). The example from the
> previous patch series doesn't work either after applying this series.
Yes I did not really explain this part (and I will update the series with an explanation).
1) The rule example in the previous series will no longer work as expected as the changes to the
port keyset configuration now requires a non-ip frame to generate the MAC_ETYPE keyset.
So to test the MAC_ETYPE case your rule must be non-ip and not use "protocol all" which is not
supported yet.
Here is an example using the "protocol 0xbeef":
tc qdisc add dev eth3 clsact
tc filter add dev eth3 ingress chain 8000000 prio 10 handle 10 \
protocol 0xbeef flower skip_sw \
dst_mac 0a:0b:0c:0d:0e:0f \
src_mac 2:0:0:0:0:1 \
action trap
And send a frame like this (using EasyFrame):
ef tx eth_fiber1 rep 10 eth dmac 0a:0b:0c:0d:0e:0f smac 2::1 et 0xbeef data repeat 50 0x61
I am not sure what went wrong when you tested the ipv4 rule, but if I create the rule that you
quoted above the rule is activated when I send frames like this:
ef tx eth_fiber1 rep 10 eth dmac 0a:0b:0c:0d:0e:0f smac 2::2 ipv4 dip 1.0.1.1 sip 2.0.2.2 data
repeat 50 0x61
Note that the smac is changed to avoid hitting the first rule.
2) As for the VLAN based rules, the VLAN information used by IS2 is the classified VID and PCP, so
you need to create a bridge and add the VID to the bridge and the ports to see this in action.
IS0 uses the VLAN tags in the frames directly: this is one of the differences between IS0 and IS2.
This is how I set up a bridge on my PCB134 when I do the testing:
ip link add name br5 type bridge
ip link set dev br5 up
ip link set eth12 master br5
ip link set eth13 master br5
ip link set eth14 master br5
ip link set eth15 master br5
sysctl -w net.ipv6.conf.eth12.disable_ipv6=1
sysctl -w net.ipv6.conf.eth13.disable_ipv6=1
sysctl -w net.ipv6.conf.eth14.disable_ipv6=1
sysctl -w net.ipv6.conf.eth15.disable_ipv6=1
sysctl -w net.ipv6.conf.br5.disable_ipv6=1
ip link set dev br5 type bridge vlan_filtering 1
bridge vlan add dev eth12 vid 600
bridge vlan add dev eth13 vid 600
bridge vlan add dev eth14 vid 600
bridge vlan add dev eth15 vid 600
bridge vlan add dev br5 vid 600 self
This should now allow you to use the classified VLAN information in IS2 on these four ports.
>
> This example was provided in your last patch series and worked earlier.
>
> My setup is PC-eth0 -> PCB135-eth3 and I use the following EasyFrames
> command to send packets:
>
> ef tx eth0 rep 50 eth smac 02:00:00:00:00:01 dmac 0a:0b:0c:0d:0e:0f
>
> IPv4:
> tc qdisc add dev eth3 clsact
> tc filter add dev eth3 ingress chain 8000000 prio 12 handle 12 \
> protocol ip flower skip_sw dst_ip 1.0.1.1 src_ip 2.0.2.2 \
> action trap
>
> ef tx eth0 rep 50 eth smac 02:00:00:00:00:01 dmac 0a:0b:0c:0d:0e:0f ipv4 dip 1.0.1.1 sip 2.0.2.2
>
> Same setup as above and I can't get this to work either.
Maybe you are hitting the first rule here, so changing the smac to avoid that, should help.
>
> I'm using tcpdump to watch the interface to see if the packets are being
> trapped or not. Changing the packets' dmac to broadcast lets me see the
> packets so I don't have any issue with the setup.
>
> BR,
> Casper
>
Best Regards
Steen
Hi Steen,
On 2022-10-31 13:14, Steen Hegelund wrote:
> Hi Casper,
>
> First of all thanks for the testing effort (as usual). This is most welcome.
>
> On Mon, 2022-10-31 at 11:44 +0100, Casper Andersson wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Hi Steen,
> >
> > On 2022-10-28 16:45, Steen Hegelund wrote:
> > > - IPv4 Addresses
> > > tc filter add dev eth12 ingress chain 8000000 prio 12 handle 12 \
> > > protocol ip flower skip_sw dst_ip 1.0.1.1 src_ip 2.0.2.2 \
> > > action trap
> >
> > I'm not able to get this working on PCB135. I tested the VLAN tags and
> > did not work either (did not test the rest). The example from the
> > previous patch series doesn't work either after applying this series.
>
>
> Yes I did not really explain this part (and I will update the series with an explanation).
>
> 1) The rule example in the previous series will no longer work as expected as the changes to the
> port keyset configuration now requires a non-ip frame to generate the MAC_ETYPE keyset.
>
> So to test the MAC_ETYPE case your rule must be non-ip and not use "protocol all" which is not
> supported yet.
>
> Here is an example using the "protocol 0xbeef":
>
> tc qdisc add dev eth3 clsact
> tc filter add dev eth3 ingress chain 8000000 prio 10 handle 10 \
> protocol 0xbeef flower skip_sw \
> dst_mac 0a:0b:0c:0d:0e:0f \
> src_mac 2:0:0:0:0:1 \
> action trap
>
> And send a frame like this (using EasyFrame):
>
> ef tx eth_fiber1 rep 10 eth dmac 0a:0b:0c:0d:0e:0f smac 2::1 et 0xbeef data repeat 50 0x61
Thanks, this works. I saw now that you even mentioned that "protocol
all" doesn't work at the very end of this commit message.
> I am not sure what went wrong when you tested the ipv4 rule, but if I create the rule that you
> quoted above the rule is activated when I send frames like this:
>
> ef tx eth_fiber1 rep 10 eth dmac 0a:0b:0c:0d:0e:0f smac 2::2 ipv4 dip 1.0.1.1 sip 2.0.2.2 data
> repeat 50 0x61
Looks like adding the "data" at the end of it makes a difference when
creating the packets. Without it the ip.proto field becomes 17 (UDP).
With "data" it becomes 0 (IPv6 Hop-by-Hop Option). Ef will defaults to
17 if no data is specified, otherwise it ends up 0. And the reason
UDP doesn't get trapped I assume is because this rule falls under the
IPV4_OTHER keyset (as opposed to IPV4_TCP_UDP).
Doing just this was enough:
ef tx eth0 rep 10 eth dmac 0a:0b:0c:0d:0e:0f smac 2::2 ipv4 dip 1.0.1.1 sip 2.0.2.2 data
This also solved it for VLANs. I have successfully tested ipv4, ipv6,
protocol info (ICMP), and vlan tag info from the examples you provided.
Tested on Microchip PCB135 switch.
Tested-by: Casper Andersson <casper.casan@gmail.com>
BR,
Casper
>
> Note that the smac is changed to avoid hitting the first rule.
>
> 2) As for the VLAN based rules, the VLAN information used by IS2 is the classified VID and PCP, so
> you need to create a bridge and add the VID to the bridge and the ports to see this in action.
>
> IS0 uses the VLAN tags in the frames directly: this is one of the differences between IS0 and IS2.
>
> This is how I set up a bridge on my PCB134 when I do the testing:
>
> ip link add name br5 type bridge
> ip link set dev br5 up
> ip link set eth12 master br5
> ip link set eth13 master br5
> ip link set eth14 master br5
> ip link set eth15 master br5
> sysctl -w net.ipv6.conf.eth12.disable_ipv6=1
> sysctl -w net.ipv6.conf.eth13.disable_ipv6=1
> sysctl -w net.ipv6.conf.eth14.disable_ipv6=1
> sysctl -w net.ipv6.conf.eth15.disable_ipv6=1
> sysctl -w net.ipv6.conf.br5.disable_ipv6=1
> ip link set dev br5 type bridge vlan_filtering 1
> bridge vlan add dev eth12 vid 600
> bridge vlan add dev eth13 vid 600
> bridge vlan add dev eth14 vid 600
> bridge vlan add dev eth15 vid 600
> bridge vlan add dev br5 vid 600 self
>
> This should now allow you to use the classified VLAN information in IS2 on these four ports.
>
> >
> > This example was provided in your last patch series and worked earlier.
> >
> > My setup is PC-eth0 -> PCB135-eth3 and I use the following EasyFrames
> > command to send packets:
> >
> > ef tx eth0 rep 50 eth smac 02:00:00:00:00:01 dmac 0a:0b:0c:0d:0e:0f
> >
> > IPv4:
> > tc qdisc add dev eth3 clsact
> > tc filter add dev eth3 ingress chain 8000000 prio 12 handle 12 \
> > protocol ip flower skip_sw dst_ip 1.0.1.1 src_ip 2.0.2.2 \
> > action trap
> >
> > ef tx eth0 rep 50 eth smac 02:00:00:00:00:01 dmac 0a:0b:0c:0d:0e:0f ipv4 dip 1.0.1.1 sip 2.0.2.2
> >
> > Same setup as above and I can't get this to work either.
>
> Maybe you are hitting the first rule here, so changing the smac to avoid that, should help.
>
> >
> > I'm using tcpdump to watch the interface to see if the packets are being
> > trapped or not. Changing the packets' dmac to broadcast lets me see the
> > packets so I don't have any issue with the setup.
> >
> > BR,
> > Casper
> >
>
> Best Regards
> Steen
BR,
Casper
Hi Casper,
On Mon, 2022-10-31 at 15:52 +0100, Casper Andersson wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> Hi Steen,
>
> On 2022-10-31 13:14, Steen Hegelund wrote:
> > Hi Casper,
> >
> > First of all thanks for the testing effort (as usual). This is most welcome.
> >
> > On Mon, 2022-10-31 at 11:44 +0100, Casper Andersson wrote:
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > >
> > > Hi Steen,
> > >
> > > On 2022-10-28 16:45, Steen Hegelund wrote:
> > > > - IPv4 Addresses
> > > > tc filter add dev eth12 ingress chain 8000000 prio 12 handle 12 \
> > > > protocol ip flower skip_sw dst_ip 1.0.1.1 src_ip 2.0.2.2 \
> > > > action trap
> > >
> > > I'm not able to get this working on PCB135. I tested the VLAN tags and
> > > did not work either (did not test the rest). The example from the
> > > previous patch series doesn't work either after applying this series.
> >
> >
> > Yes I did not really explain this part (and I will update the series with an explanation).
> >
> > 1) The rule example in the previous series will no longer work as expected as the changes to the
> > port keyset configuration now requires a non-ip frame to generate the MAC_ETYPE keyset.
> >
> > So to test the MAC_ETYPE case your rule must be non-ip and not use "protocol all" which is not
> > supported yet.
> >
> > Here is an example using the "protocol 0xbeef":
> >
> > tc qdisc add dev eth3 clsact
> > tc filter add dev eth3 ingress chain 8000000 prio 10 handle 10 \
> > protocol 0xbeef flower skip_sw \
> > dst_mac 0a:0b:0c:0d:0e:0f \
> > src_mac 2:0:0:0:0:1 \
> > action trap
> >
> > And send a frame like this (using EasyFrame):
> >
> > ef tx eth_fiber1 rep 10 eth dmac 0a:0b:0c:0d:0e:0f smac 2::1 et 0xbeef data repeat 50 0x61
>
> Thanks, this works. I saw now that you even mentioned that "protocol
> all" doesn't work at the very end of this commit message.
>
> > I am not sure what went wrong when you tested the ipv4 rule, but if I create the rule that you
> > quoted above the rule is activated when I send frames like this:
> >
> > ef tx eth_fiber1 rep 10 eth dmac 0a:0b:0c:0d:0e:0f smac 2::2 ipv4 dip 1.0.1.1 sip 2.0.2.2 data
> > repeat 50 0x61
>
> Looks like adding the "data" at the end of it makes a difference when
> creating the packets. Without it the ip.proto field becomes 17 (UDP).
> With "data" it becomes 0 (IPv6 Hop-by-Hop Option). Ef will defaults to
> 17 if no data is specified, otherwise it ends up 0. And the reason
> UDP doesn't get trapped I assume is because this rule falls under the
> IPV4_OTHER keyset (as opposed to IPV4_TCP_UDP).
Yes the EasyFrame tool just uses defaults if you do not specify any data for the frame, so I usually
try to remember to do that to tweak the test a bit.
>
> Doing just this was enough:
> ef tx eth0 rep 10 eth dmac 0a:0b:0c:0d:0e:0f smac 2::2 ipv4 dip 1.0.1.1 sip 2.0.2.2 data
>
> This also solved it for VLANs. I have successfully tested ipv4, ipv6,
> protocol info (ICMP), and vlan tag info from the examples you provided.
>
> Tested on Microchip PCB135 switch.
>
> Tested-by: Casper Andersson <casper.casan@gmail.com>
>
> BR,
> Casper
>
> >
> > Note that the smac is changed to avoid hitting the first rule.
> >
> > 2) As for the VLAN based rules, the VLAN information used by IS2 is the classified VID and PCP,
> > so
> > you need to create a bridge and add the VID to the bridge and the ports to see this in action.
> >
> > IS0 uses the VLAN tags in the frames directly: this is one of the differences between IS0 and
> > IS2.
> >
> > This is how I set up a bridge on my PCB134 when I do the testing:
> >
> > ip link add name br5 type bridge
> > ip link set dev br5 up
> > ip link set eth12 master br5
> > ip link set eth13 master br5
> > ip link set eth14 master br5
> > ip link set eth15 master br5
> > sysctl -w net.ipv6.conf.eth12.disable_ipv6=1
> > sysctl -w net.ipv6.conf.eth13.disable_ipv6=1
> > sysctl -w net.ipv6.conf.eth14.disable_ipv6=1
> > sysctl -w net.ipv6.conf.eth15.disable_ipv6=1
> > sysctl -w net.ipv6.conf.br5.disable_ipv6=1
> > ip link set dev br5 type bridge vlan_filtering 1
> > bridge vlan add dev eth12 vid 600
> > bridge vlan add dev eth13 vid 600
> > bridge vlan add dev eth14 vid 600
> > bridge vlan add dev eth15 vid 600
> > bridge vlan add dev br5 vid 600 self
> >
> > This should now allow you to use the classified VLAN information in IS2 on these four ports.
> >
> > >
> > > This example was provided in your last patch series and worked earlier.
> > >
> > > My setup is PC-eth0 -> PCB135-eth3 and I use the following EasyFrames
> > > command to send packets:
> > >
> > > ef tx eth0 rep 50 eth smac 02:00:00:00:00:01 dmac 0a:0b:0c:0d:0e:0f
> > >
> > > IPv4:
> > > tc qdisc add dev eth3 clsact
> > > tc filter add dev eth3 ingress chain 8000000 prio 12 handle 12 \
> > > protocol ip flower skip_sw dst_ip 1.0.1.1 src_ip 2.0.2.2 \
> > > action trap
> > >
> > > ef tx eth0 rep 50 eth smac 02:00:00:00:00:01 dmac 0a:0b:0c:0d:0e:0f ipv4 dip 1.0.1.1 sip
> > > 2.0.2.2
> > >
> > > Same setup as above and I can't get this to work either.
> >
> > Maybe you are hitting the first rule here, so changing the smac to avoid that, should help.
> >
> > >
> > > I'm using tcpdump to watch the interface to see if the packets are being
> > > trapped or not. Changing the packets' dmac to broadcast lets me see the
> > > packets so I don't have any issue with the setup.
> > >
> > > BR,
> > > Casper
> > >
> >
> > Best Regards
> > Steen
>
> BR,
> Casper
Thanks again for the testing. I will send an updated series with a bit more explanation in the
commit header.
BR
Steen
On Mon, 31 Oct 2022 13:14:33 +0100 Steen Hegelund wrote:
> > I'm not able to get this working on PCB135. I tested the VLAN tags and
> > did not work either (did not test the rest). The example from the
> > previous patch series doesn't work either after applying this series.
Previous series in this context means previous revision or something
that was already merged?
> tc filter add dev eth3 ingress chain 8000000 prio 10 handle 10 \
How are you using chains?
I thought you need to offload FLOW_ACTION_GOTO to get to a chain,
and I get no hits on this driver.
Hi Jacub,
On Mon, 2022-10-31 at 18:41 -0700, Jakub Kicinski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Mon, 31 Oct 2022 13:14:33 +0100 Steen Hegelund wrote:
> > > I'm not able to get this working on PCB135. I tested the VLAN tags and
> > > did not work either (did not test the rest). The example from the
> > > previous patch series doesn't work either after applying this series.
>
> Previous series in this context means previous revision or something
> that was already merged?
Casper refers to this series (the first of the VCAP related series) that was merged on Oct 24th:
https://lore.kernel.org/all/20221020130904.1215072-1-steen.hegelund@microchip.com/
>
> > tc filter add dev eth3 ingress chain 8000000 prio 10 handle 10 \
>
> How are you using chains?
The chain ids are referring to the VCAP instances and their lookups. There are some more details
about this in the series I referred to above.
The short version is that this allows you to select where in the frame processing flow your rule
will be inserted (using ingress or egress and the chain id).
>
> I thought you need to offload FLOW_ACTION_GOTO to get to a chain,
> and I get no hits on this driver.
I have not yet added the goto action, but one use of that is to chain a filter from one VCAP
instance/lookup to another.
The goto action will be added in a soon-to-come series. I just wanted to avoid a series getting too
large, but on the other hand each of them should provide functionality that you can use in practice.
BR
Steen
On Tue, 1 Nov 2022 08:31:16 +0100 Steen Hegelund wrote:
> > Previous series in this context means previous revision or something
> > that was already merged?
>
> Casper refers to this series (the first of the VCAP related series) that was merged on Oct 24th:
>
> https://lore.kernel.org/all/20221020130904.1215072-1-steen.hegelund@microchip.com/
Alright, looks like this is only in net-next so no risk of breaking
existing users.
That said you should reject filters you can't support with an extack
message set. Also see below.
> > > tc filter add dev eth3 ingress chain 8000000 prio 10 handle 10 \
> >
> > How are you using chains?
>
> The chain ids are referring to the VCAP instances and their lookups. There are some more details
> about this in the series I referred to above.
>
> The short version is that this allows you to select where in the frame processing flow your rule
> will be inserted (using ingress or egress and the chain id).
>
> > I thought you need to offload FLOW_ACTION_GOTO to get to a chain,
> > and I get no hits on this driver.
>
> I have not yet added the goto action, but one use of that is to chain a filter from one VCAP
> instance/lookup to another.
>
> The goto action will be added in a soon-to-come series. I just wanted to avoid a series getting too
> large, but on the other hand each of them should provide functionality that you can use in practice.
The behavior of the offload must be the same as the SW implementation.
It sounds like in your case it very much isn't, as adding rules to
a magic chain in SW, without the goto will result in the rules being
unused.
Hi Jacub,
Thanks for the comments.
On Tue, 2022-11-01 at 08:49 -0700, Jakub Kicinski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Tue, 1 Nov 2022 08:31:16 +0100 Steen Hegelund wrote:
> > > Previous series in this context means previous revision or something
> > > that was already merged?
> >
> > Casper refers to this series (the first of the VCAP related series) that was merged on Oct 24th:
> >
> > https://lore.kernel.org/all/20221020130904.1215072-1-steen.hegelund@microchip.com/
>
> Alright, looks like this is only in net-next so no risk of breaking
> existing users.
Yes, this is a new feature.
>
> That said you should reject filters you can't support with an extack
> message set. Also see below.
That should also be the case.
I just checked that using an unsupported key, action or chain will result in an extack error
message.
>
> > > > tc filter add dev eth3 ingress chain 8000000 prio 10 handle 10 \
> > >
> > > How are you using chains?
> >
> > The chain ids are referring to the VCAP instances and their lookups. There are some more
> > details
> > about this in the series I referred to above.
> >
> > The short version is that this allows you to select where in the frame processing flow your rule
> > will be inserted (using ingress or egress and the chain id).
> >
> > > I thought you need to offload FLOW_ACTION_GOTO to get to a chain,
> > > and I get no hits on this driver.
> >
> > I have not yet added the goto action, but one use of that is to chain a filter from one VCAP
> > instance/lookup to another.
> >
> > The goto action will be added in a soon-to-come series. I just wanted to avoid a series getting
> > too
> > large, but on the other hand each of them should provide functionality that you can use in
> > practice.
>
> The behavior of the offload must be the same as the SW implementation.
> It sounds like in your case it very much isn't, as adding rules to
> a magic chain in SW, without the goto will result in the rules being
> unused.
I will add the goto support to my implementation so that it will be consistent with the SW
implementation, adding a check to ensure that there is a goto action when HW offloading.
BR
Steen
Hi Jacub,
On Tue, 2022-11-01 at 08:49 -0700, Jakub Kicinski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On Tue, 1 Nov 2022 08:31:16 +0100 Steen Hegelund wrote:
> > > Previous series in this context means previous revision or something
> > > that was already merged?
> >
> > Casper refers to this series (the first of the VCAP related series) that was
> > merged on Oct 24th:
> >
> > https://lore.kernel.org/all/20221020130904.1215072-1-steen.hegelund@microchip.com/
>
> Alright, looks like this is only in net-next so no risk of breaking
> existing users.
>
> That said you should reject filters you can't support with an extack
> message set. Also see below.
>
> > > > tc filter add dev eth3 ingress chain 8000000 prio 10 handle 10 \
> > >
> > > How are you using chains?
> >
> > The chain ids are referring to the VCAP instances and their lookups. There
> > are some more details
> > about this in the series I referred to above.
> >
> > The short version is that this allows you to select where in the frame
> > processing flow your rule
> > will be inserted (using ingress or egress and the chain id).
> >
> > > I thought you need to offload FLOW_ACTION_GOTO to get to a chain,
> > > and I get no hits on this driver.
> >
> > I have not yet added the goto action, but one use of that is to chain a
> > filter from one VCAP
> > instance/lookup to another.
> >
> > The goto action will be added in a soon-to-come series. I just wanted to
> > avoid a series getting too
> > large, but on the other hand each of them should provide functionality that
> > you can use in practice.
>
> The behavior of the offload must be the same as the SW implementation.
> It sounds like in your case it very much isn't, as adding rules to
> a magic chain in SW, without the goto will result in the rules being
> unused.
I have sent a version 4 of the series, but I realized after sending it, that I
was probably not understanding the implications of what you were saying
entirely.
As far as I understand it now, I need to have a matchall rule that does a goto
from chain 0 (as this is where all traffic processing starts) to my first IS2
VCAP chain and this rule activates the IS2 VCAP lookup.
Each of the rules in this VCAP chain need to point to the next chain etc.
If the matchall rule is deleted the IS2 VCAP lookups should be disabled as there
is no longer any way to reach the VCAP chains.
Does that sound OK?
BR
Steen
On Wed, 2 Nov 2022 14:11:37 +0100 Steen Hegelund wrote:
> I have sent a version 4 of the series, but I realized after sending it, that I
> was probably not understanding the implications of what you were saying
> entirely.
>
> As far as I understand it now, I need to have a matchall rule that does a goto
> from chain 0 (as this is where all traffic processing starts) to my first IS2
> VCAP chain and this rule activates the IS2 VCAP lookup.
>
> Each of the rules in this VCAP chain need to point to the next chain etc.
>
> If the matchall rule is deleted the IS2 VCAP lookups should be disabled as there
> is no longer any way to reach the VCAP chains.
>
> Does that sound OK?
It does as far as I understand.
I haven't grasped what the purpose of using multiple chains is in
case of your design. IIRC correctly other drivers use it for instance
to partition TCAMs with each chain having a different set of fields it
can match on. But I don't see templates used in sparx5.
In general in TC offloads you can reject any configuration you can't
(or choose not to) support, and make up your own constraints (e.g. only
specific priority or chain values are supported).
But for a "target" ruleset, i.e. ruleset comprised fully of rules you
do offload - the behavior of executing that ruleset in software and in
the device must be the same.
Dunno if that helps :)
Hi Jacub,
On Wed, 2022-11-02 at 18:28 -0700, Jakub Kicinski wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
>
> On Wed, 2 Nov 2022 14:11:37 +0100 Steen Hegelund wrote:
> > I have sent a version 4 of the series, but I realized after sending it, that
> > I
> > was probably not understanding the implications of what you were saying
> > entirely.
> >
> > As far as I understand it now, I need to have a matchall rule that does a
> > goto
> > from chain 0 (as this is where all traffic processing starts) to my first
> > IS2
> > VCAP chain and this rule activates the IS2 VCAP lookup.
> >
> > Each of the rules in this VCAP chain need to point to the next chain etc.
> >
> > If the matchall rule is deleted the IS2 VCAP lookups should be disabled as
> > there
> > is no longer any way to reach the VCAP chains.
> >
> > Does that sound OK?
>
> It does as far as I understand.
>
> I haven't grasped what the purpose of using multiple chains is in
> case of your design. IIRC correctly other drivers use it for instance
> to partition TCAMs with each chain having a different set of fields it
> can match on. But I don't see templates used in sparx5.
Yes, so far I have only added the IS2 VCAP, but there are 3 more that I am
planning to to add, and they have very different capabilities in terms of keys
and actions, so I think it makes good sense to keep them in separate chains.
>
> In general in TC offloads you can reject any configuration you can't
> (or choose not to) support, and make up your own constraints (e.g. only
> specific priority or chain values are supported).
Understood.
>
> But for a "target" ruleset, i.e. ruleset comprised fully of rules you
> do offload - the behavior of executing that ruleset in software and in
> the device must be the same.
>
> Dunno if that helps :)
It does, thanks!
I been fireing up a QEMU instance so I have been able to test my understanding,
and it looks like I now have the same experience when I test the same rule there
and in the hardware of Sparx5.
BR
Steen
@@ -16,9 +16,32 @@ struct sparx5_tc_flower_parse_usage {
struct flow_cls_offload *fco;
struct flow_rule *frule;
struct vcap_rule *vrule;
+ u16 l3_proto;
+ u8 l4_proto;
unsigned int used_keys;
};
+/* These protocols have dedicated keysets in IS2 and a TC dissector
+ * ETH_P_ARP does not have a TC dissector
+ */
+static u16 sparx5_tc_known_etypes[] = {
+ ETH_P_ALL,
+ ETH_P_IP,
+ ETH_P_IPV6,
+};
+
+static bool sparx5_tc_is_known_etype(u16 etype)
+{
+ int idx;
+
+ /* For now this only knows about IS2 traffic classification */
+ for (idx = 0; idx < ARRAY_SIZE(sparx5_tc_known_etypes); ++idx)
+ if (sparx5_tc_known_etypes[idx] == etype)
+ return true;
+
+ return false;
+}
+
static int sparx5_tc_flower_handler_ethaddr_usage(struct sparx5_tc_flower_parse_usage *st)
{
enum vcap_key_field smac_key = VCAP_KF_L2_SMAC;
@@ -54,9 +77,368 @@ static int sparx5_tc_flower_handler_ethaddr_usage(struct sparx5_tc_flower_parse_
return err;
}
+static int
+sparx5_tc_flower_handler_ipv4_usage(struct sparx5_tc_flower_parse_usage *st)
+{
+ int err = 0;
+
+ if (st->l3_proto == ETH_P_IP) {
+ struct flow_match_ipv4_addrs mt;
+
+ flow_rule_match_ipv4_addrs(st->frule, &mt);
+ if (mt.mask->src) {
+ err = vcap_rule_add_key_u32(st->vrule,
+ VCAP_KF_L3_IP4_SIP,
+ be32_to_cpu(mt.key->src),
+ be32_to_cpu(mt.mask->src));
+ if (err)
+ goto out;
+ }
+ if (mt.mask->dst) {
+ err = vcap_rule_add_key_u32(st->vrule,
+ VCAP_KF_L3_IP4_DIP,
+ be32_to_cpu(mt.key->dst),
+ be32_to_cpu(mt.mask->dst));
+ if (err)
+ goto out;
+ }
+ }
+
+ st->used_keys |= BIT(FLOW_DISSECTOR_KEY_IPV4_ADDRS);
+
+ return err;
+
+out:
+ NL_SET_ERR_MSG_MOD(st->fco->common.extack, "ipv4_addr parse error");
+ return err;
+}
+
+static int
+sparx5_tc_flower_handler_ipv6_usage(struct sparx5_tc_flower_parse_usage *st)
+{
+ int err = 0;
+
+ if (st->l3_proto == ETH_P_IPV6) {
+ struct flow_match_ipv6_addrs mt;
+ struct vcap_u128_key sip;
+ struct vcap_u128_key dip;
+
+ flow_rule_match_ipv6_addrs(st->frule, &mt);
+ /* Check if address masks are non-zero */
+ if (!ipv6_addr_any(&mt.mask->src)) {
+ vcap_netbytes_copy(sip.value, mt.key->src.s6_addr, 16);
+ vcap_netbytes_copy(sip.mask, mt.mask->src.s6_addr, 16);
+ err = vcap_rule_add_key_u128(st->vrule,
+ VCAP_KF_L3_IP6_SIP, &sip);
+ if (err)
+ goto out;
+ }
+ if (!ipv6_addr_any(&mt.mask->dst)) {
+ vcap_netbytes_copy(dip.value, mt.key->dst.s6_addr, 16);
+ vcap_netbytes_copy(dip.mask, mt.mask->dst.s6_addr, 16);
+ err = vcap_rule_add_key_u128(st->vrule,
+ VCAP_KF_L3_IP6_DIP, &dip);
+ if (err)
+ goto out;
+ }
+ }
+ st->used_keys |= BIT(FLOW_DISSECTOR_KEY_IPV6_ADDRS);
+ return err;
+out:
+ NL_SET_ERR_MSG_MOD(st->fco->common.extack, "ipv6_addr parse error");
+ return err;
+}
+
+static int
+sparx5_tc_flower_handler_control_usage(struct sparx5_tc_flower_parse_usage *st)
+{
+ struct flow_match_control mt;
+ u32 value, mask;
+ int err = 0;
+
+ flow_rule_match_control(st->frule, &mt);
+
+ if (mt.mask->flags) {
+ if (mt.mask->flags & FLOW_DIS_FIRST_FRAG) {
+ if (mt.key->flags & FLOW_DIS_FIRST_FRAG) {
+ value = 1; /* initial fragment */
+ mask = 0x3;
+ } else {
+ if (mt.mask->flags & FLOW_DIS_IS_FRAGMENT) {
+ value = 3; /* follow up fragment */
+ mask = 0x3;
+ } else {
+ value = 0; /* no fragment */
+ mask = 0x3;
+ }
+ }
+ } else {
+ if (mt.mask->flags & FLOW_DIS_IS_FRAGMENT) {
+ value = 3; /* follow up fragment */
+ mask = 0x3;
+ } else {
+ value = 0; /* no fragment */
+ mask = 0x3;
+ }
+ }
+
+ err = vcap_rule_add_key_u32(st->vrule,
+ VCAP_KF_L3_FRAGMENT_TYPE,
+ value, mask);
+ if (err)
+ goto out;
+ }
+
+ st->used_keys |= BIT(FLOW_DISSECTOR_KEY_CONTROL);
+
+ return err;
+
+out:
+ NL_SET_ERR_MSG_MOD(st->fco->common.extack, "ip_frag parse error");
+ return err;
+}
+
+static int
+sparx5_tc_flower_handler_portnum_usage(struct sparx5_tc_flower_parse_usage *st)
+{
+ struct flow_match_ports mt;
+ u16 value, mask;
+ int err = 0;
+
+ flow_rule_match_ports(st->frule, &mt);
+
+ if (mt.mask->src) {
+ value = be16_to_cpu(mt.key->src);
+ mask = be16_to_cpu(mt.mask->src);
+ err = vcap_rule_add_key_u32(st->vrule, VCAP_KF_L4_SPORT, value,
+ mask);
+ if (err)
+ goto out;
+ }
+
+ if (mt.mask->dst) {
+ value = be16_to_cpu(mt.key->dst);
+ mask = be16_to_cpu(mt.mask->dst);
+ err = vcap_rule_add_key_u32(st->vrule, VCAP_KF_L4_DPORT, value,
+ mask);
+ if (err)
+ goto out;
+ }
+
+ st->used_keys |= BIT(FLOW_DISSECTOR_KEY_PORTS);
+
+ return err;
+
+out:
+ NL_SET_ERR_MSG_MOD(st->fco->common.extack, "port parse error");
+ return err;
+}
+
+static int
+sparx5_tc_flower_handler_basic_usage(struct sparx5_tc_flower_parse_usage *st)
+{
+ struct flow_match_basic mt;
+ int err = 0;
+
+ flow_rule_match_basic(st->frule, &mt);
+
+ if (mt.mask->n_proto) {
+ st->l3_proto = be16_to_cpu(mt.key->n_proto);
+ if (!sparx5_tc_is_known_etype(st->l3_proto)) {
+ err = vcap_rule_add_key_u32(st->vrule, VCAP_KF_ETYPE,
+ st->l3_proto, ~0);
+ if (err)
+ goto out;
+ } else if (st->l3_proto == ETH_P_IP) {
+ err = vcap_rule_add_key_bit(st->vrule, VCAP_KF_IP4_IS,
+ VCAP_BIT_1);
+ if (err)
+ goto out;
+ } else if (st->l3_proto == ETH_P_IPV6) {
+ err = vcap_rule_add_key_bit(st->vrule, VCAP_KF_IP4_IS,
+ VCAP_BIT_0);
+ if (err)
+ goto out;
+ }
+ }
+
+ if (mt.mask->ip_proto) {
+ st->l4_proto = mt.key->ip_proto;
+ if (st->l4_proto == IPPROTO_TCP) {
+ err = vcap_rule_add_key_bit(st->vrule,
+ VCAP_KF_TCP_IS,
+ VCAP_BIT_1);
+ if (err)
+ goto out;
+ } else if (st->l4_proto == IPPROTO_UDP) {
+ err = vcap_rule_add_key_bit(st->vrule,
+ VCAP_KF_TCP_IS,
+ VCAP_BIT_0);
+ if (err)
+ goto out;
+ } else {
+ err = vcap_rule_add_key_u32(st->vrule,
+ VCAP_KF_L3_IP_PROTO,
+ st->l4_proto, ~0);
+ if (err)
+ goto out;
+ }
+ }
+
+ st->used_keys |= BIT(FLOW_DISSECTOR_KEY_BASIC);
+
+ return err;
+
+out:
+ NL_SET_ERR_MSG_MOD(st->fco->common.extack, "ip_proto parse error");
+ return err;
+}
+
+static int
+sparx5_tc_flower_handler_vlan_usage(struct sparx5_tc_flower_parse_usage *st)
+{
+ enum vcap_key_field vid_key = VCAP_KF_8021Q_VID_CLS;
+ enum vcap_key_field pcp_key = VCAP_KF_8021Q_PCP_CLS;
+ struct flow_match_vlan mt;
+ int err;
+
+ flow_rule_match_vlan(st->frule, &mt);
+
+ if (mt.mask->vlan_id) {
+ err = vcap_rule_add_key_u32(st->vrule, vid_key,
+ mt.key->vlan_id,
+ mt.mask->vlan_id);
+ if (err)
+ goto out;
+ }
+
+ if (mt.mask->vlan_priority) {
+ err = vcap_rule_add_key_u32(st->vrule, pcp_key,
+ mt.key->vlan_priority,
+ mt.mask->vlan_priority);
+ if (err)
+ goto out;
+ }
+
+ st->used_keys |= BIT(FLOW_DISSECTOR_KEY_VLAN);
+
+ return err;
+out:
+ NL_SET_ERR_MSG_MOD(st->fco->common.extack, "vlan parse error");
+ return err;
+}
+
+static int
+sparx5_tc_flower_handler_tcp_usage(struct sparx5_tc_flower_parse_usage *st)
+{
+ struct flow_match_tcp mt;
+ u16 tcp_flags_mask;
+ u16 tcp_flags_key;
+ enum vcap_bit val;
+ int err = 0;
+
+ flow_rule_match_tcp(st->frule, &mt);
+ tcp_flags_key = be16_to_cpu(mt.key->flags);
+ tcp_flags_mask = be16_to_cpu(mt.mask->flags);
+
+ if (tcp_flags_mask & TCPHDR_FIN) {
+ val = VCAP_BIT_0;
+ if (tcp_flags_key & TCPHDR_FIN)
+ val = VCAP_BIT_1;
+ err = vcap_rule_add_key_bit(st->vrule, VCAP_KF_L4_FIN, val);
+ if (err)
+ goto out;
+ }
+
+ if (tcp_flags_mask & TCPHDR_SYN) {
+ val = VCAP_BIT_0;
+ if (tcp_flags_key & TCPHDR_SYN)
+ val = VCAP_BIT_1;
+ err = vcap_rule_add_key_bit(st->vrule, VCAP_KF_L4_SYN, val);
+ if (err)
+ goto out;
+ }
+
+ if (tcp_flags_mask & TCPHDR_RST) {
+ val = VCAP_BIT_0;
+ if (tcp_flags_key & TCPHDR_RST)
+ val = VCAP_BIT_1;
+ err = vcap_rule_add_key_bit(st->vrule, VCAP_KF_L4_RST, val);
+ if (err)
+ goto out;
+ }
+
+ if (tcp_flags_mask & TCPHDR_PSH) {
+ val = VCAP_BIT_0;
+ if (tcp_flags_key & TCPHDR_PSH)
+ val = VCAP_BIT_1;
+ err = vcap_rule_add_key_bit(st->vrule, VCAP_KF_L4_PSH, val);
+ if (err)
+ goto out;
+ }
+
+ if (tcp_flags_mask & TCPHDR_ACK) {
+ val = VCAP_BIT_0;
+ if (tcp_flags_key & TCPHDR_ACK)
+ val = VCAP_BIT_1;
+ err = vcap_rule_add_key_bit(st->vrule, VCAP_KF_L4_ACK, val);
+ if (err)
+ goto out;
+ }
+
+ if (tcp_flags_mask & TCPHDR_URG) {
+ val = VCAP_BIT_0;
+ if (tcp_flags_key & TCPHDR_URG)
+ val = VCAP_BIT_1;
+ err = vcap_rule_add_key_bit(st->vrule, VCAP_KF_L4_URG, val);
+ if (err)
+ goto out;
+ }
+
+ st->used_keys |= BIT(FLOW_DISSECTOR_KEY_TCP);
+
+ return err;
+
+out:
+ NL_SET_ERR_MSG_MOD(st->fco->common.extack, "tcp_flags parse error");
+ return err;
+}
+
+static int
+sparx5_tc_flower_handler_ip_usage(struct sparx5_tc_flower_parse_usage *st)
+{
+ struct flow_match_ip mt;
+ int err = 0;
+
+ flow_rule_match_ip(st->frule, &mt);
+
+ if (mt.mask->tos) {
+ err = vcap_rule_add_key_u32(st->vrule, VCAP_KF_L3_TOS,
+ mt.key->tos,
+ mt.mask->tos);
+ if (err)
+ goto out;
+ }
+
+ st->used_keys |= BIT(FLOW_DISSECTOR_KEY_IP);
+
+ return err;
+
+out:
+ NL_SET_ERR_MSG_MOD(st->fco->common.extack, "ip_tos parse error");
+ return err;
+}
+
static int (*sparx5_tc_flower_usage_handlers[])(struct sparx5_tc_flower_parse_usage *st) = {
- /* More dissector handlers will be added here later */
[FLOW_DISSECTOR_KEY_ETH_ADDRS] = sparx5_tc_flower_handler_ethaddr_usage,
+ [FLOW_DISSECTOR_KEY_IPV4_ADDRS] = sparx5_tc_flower_handler_ipv4_usage,
+ [FLOW_DISSECTOR_KEY_IPV6_ADDRS] = sparx5_tc_flower_handler_ipv6_usage,
+ [FLOW_DISSECTOR_KEY_CONTROL] = sparx5_tc_flower_handler_control_usage,
+ [FLOW_DISSECTOR_KEY_PORTS] = sparx5_tc_flower_handler_portnum_usage,
+ [FLOW_DISSECTOR_KEY_BASIC] = sparx5_tc_flower_handler_basic_usage,
+ [FLOW_DISSECTOR_KEY_VLAN] = sparx5_tc_flower_handler_vlan_usage,
+ [FLOW_DISSECTOR_KEY_TCP] = sparx5_tc_flower_handler_tcp_usage,
+ [FLOW_DISSECTOR_KEY_IP] = sparx5_tc_flower_handler_ip_usage,
};
static int sparx5_tc_use_dissectors(struct flow_cls_offload *fco,
@@ -1073,6 +1073,17 @@ int vcap_rule_add_key_u72(struct vcap_rule *rule, enum vcap_key_field key,
}
EXPORT_SYMBOL_GPL(vcap_rule_add_key_u72);
+/* Add a 128 bit key with value and mask to the rule */
+int vcap_rule_add_key_u128(struct vcap_rule *rule, enum vcap_key_field key,
+ struct vcap_u128_key *fieldval)
+{
+ struct vcap_client_keyfield_data data;
+
+ memcpy(&data.u128, fieldval, sizeof(data.u128));
+ return vcap_rule_add_key(rule, key, VCAP_FIELD_U128, &data);
+}
+EXPORT_SYMBOL_GPL(vcap_rule_add_key_u128);
+
static void vcap_copy_from_client_actionfield(struct vcap_rule *rule,
struct vcap_client_actionfield *field,
struct vcap_client_actionfield_data *data)
@@ -176,6 +176,8 @@ int vcap_rule_add_key_u48(struct vcap_rule *rule, enum vcap_key_field key,
struct vcap_u48_key *fieldval);
int vcap_rule_add_key_u72(struct vcap_rule *rule, enum vcap_key_field key,
struct vcap_u72_key *fieldval);
+int vcap_rule_add_key_u128(struct vcap_rule *rule, enum vcap_key_field key,
+ struct vcap_u128_key *fieldval);
int vcap_rule_add_action_bit(struct vcap_rule *rule,
enum vcap_action_field action, enum vcap_bit val);
int vcap_rule_add_action_u32(struct vcap_rule *rule,