On 1 Apr 2022, at 19:54, Mike Pattrick wrote:
> On Mon, Mar 28, 2022 at 7:11 AM Eelco Chaudron <echau...@redhat.com> wrote:
>>
>> This change implements support for the check_pkt_len
>> action using the TC police action, which supports MTU
>> checking.
>>
>> Signed-off-by: Eelco Chaudron <echau...@redhat.com>
>
> Hello Eelco, I've run into a few problems with this patch.
>
> I've found that the following invocations will cause a core dump:
>
> ovs-ofctl add-flow br0
> "table=0,udp,actions=load:0x1->NXM_NX_REG1[],resubmit(,1)"
> ovs-ofctl add-flow br0
> "table=1,reg1=0x1,actions=check_pkt_larger(20)->NXM_NX_REG0[1],
> check_pkt_larger(40)->NXM_NX_REG1[1],check_pkt_larger(60)->NXM_NX_REG2[1],
> check_pkt_larger(80)->NXM_NX_REG3[1],check_pkt_larger(100)->NXM_NX_REG4[1],
> check_pkt_larger(110)->NXM_NX_REG5[1],check_pkt_larger(120)->NXM_NX_REG6[1],
> check_pkt_larger(140)->NXM_NX_REG7[1],check_pkt_larger(160)->NXM_NX_REG8[1],
> check_pkt_larger(180)->NXM_NX_REG9[1],check_pkt_larger(200)->NXM_NX_REG10[1],
> check_pkt_larger(220)->NXM_NX_REG11[1],resubmit(,2)"
As discussed offline, this is not related to this patchset, and is already
present in the current version.
You were also kind enough to offer to fix this and sent a patch upstream (maybe
you could also include a general test case for this?).
> Secondly, and I think this one is my mistake somehow, the following invocation
>
> ovs-ofctl add-flow br0
> "table=0,udp,actions=load:0x1->NXM_NX_REG1[],resubmit(,1)"
> ovs-ofctl add-flow br0
> "table=1,reg1=0x1,actions=check_pkt_larger(800)->NXM_NX_REG0[1],resubmit(,2)"
> ovs-ofctl add-flow br0 "table=2,reg0=0x1,actions=drop"
> ovs-ofctl add-flow br0 "table=2,reg0=0x0,actions=normal"
>
> Will produce the following flow rule action in dpctl:
>
> actions:check_pkt_len(size=800,gt(drop),le(drop))
I added a quick testcase as follows and it seems to work:
[ebuild:~/...DK_v21.11/ovs_github]$ git diff
diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at
index 4cde53e2e..f7b4a5118 100644
--- a/tests/system-offloads-traffic.at
+++ b/tests/system-offloads-traffic.at
@@ -573,6 +573,23 @@ NS_CHECK_EXEC([at_ns1], [ping -q -c 10 -i 0.1 -w 2 -s 64
10.1.1.2 | FORMAT_PING]
AT_CHECK_ACTIONS([check_pkt_len(size=200,gt(5),le(check_pkt_len(size=100,gt(5),le(4)))),3])
+AT_CHECK([ovs-appctl revalidator/wait], [0])
+AT_CHECK([ovs-ofctl del-flows br0])
+AT_DATA([flows.txt], [dnl
+table=0,in_port=2 actions=output:1
+table=0,in_port=1
actions=load:0x1->NXM_NX_REG1[[]],resubmit(,1),load:0x2->NXM_NX_REG1[[]],resubmit(,1)
+table=1,in_port=1,reg1=0x1
actions=check_pkt_larger(200)->NXM_NX_REG0[[0]],resubmit(,4)
+table=4,in_port=1,reg0=0x1 actions=drop
+table=4,in_port=1,reg0=0x0 actions=normal
+])
+AT_CHECK([ovs-ofctl --protocols=OpenFlow10 add-flows br0 flows.txt])
+sleep 1
+NS_CHECK_EXEC([at_ns1], [ping -q -c 10 -i 0.1 -w 2 -s 64 10.1.1.2 |
FORMAT_PING], [0], [dnl
+10 packets transmitted, 10 received, 0% packet loss, time 0ms
+], [], [ovs-appctl dpctl/dump-flows; ovs-ofctl dump-flows br0])
+AT_CHECK_ACTIONS([check_pkt_len(size=200,gt(drop),le(1,3,4,5))])
+
Note that for the le() action I get 1,3,4,5 as the MAC was not yet learned.
Maybe in your case the src and dst mac were the same so the FDB decided to drop
the packet?
You probably need to figure out why the NORMAL rule decided to insert a DROP.
You can use ofproto trace and see.
<SNIP>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
