diff mbox series

[ovs-dev] ofproto-dpif: remove checking setting nd_ext field

Message ID 20200906074550.52743-1-fankaixi.li@bytedance.com
State Superseded
Headers show
Series [ovs-dev] ofproto-dpif: remove checking setting nd_ext field | expand

Commit Message

Kaixi Fan Sept. 6, 2020, 7:45 a.m. UTC 
From: "fankaixi.li" <fankaixi.li@bytedance.com>

In order to support openflow rule which setting nd_ext fields in openflow tables,
we should remove setting nd_ext fields when constructing rule. The ofproto would
translate it into userspace actions when handling upcalls.

Signed-off-by: fankaixi.li <fankaixi.li@bytedance.com>
---
 ofproto/ofproto-dpif.c | 8 --------
 1 file changed, 8 deletions(-)

Comments

Kaixi Fan Sept. 7, 2020, 6:27 a.m. UTC | #1 
I have test it with ovs 2.14.90 on kernel 4.19.117.

My test flows are as flows. Flow 1 match ipv6 neighbor discovery
solicitation packet, modify icmpv6 type and code and redirect it to table
81. Flow 2 modify ipv6 nd target ipv6 address and destination link-address
option.
# flow 1
ovs-ofctl -Oopenflow13 add-flow br0 "cookie=0x4000000, table=0,
priority=50,icmp6,icmp_type=135,icmp_code=0,nd_target=2001:db8:0:2:0:0:0:1,nd_sll=fa:16:3e:55:ad:df,actions=set_field:136->icmpv6_type,set_field:0->icmpv6_code,set_field:2->nd_options_type,goto_table:81"
# flow 2
ovs-ofctl -Oopenflow13 add-flow br0
"cookie=0x12220d57,table=81,priority=80,icmp6,icmp_type=136,icmp_code=0,nd_target=2001:db8:0:2:0:0:0:1,actions=
move:NXM_OF_ETH_SRC[]->NXM_OF_ETH_DST[],set_field:00:23:15:d3:22:01->eth_src,move:NXM_NX_IPV6_SRC[]->NXM_NX_IPV6_DST[],set_field:2001:db8:0:2:0:0:0:1->ipv6_src,set_field:00:23:15:d3:22:01->nd_tll,set_field:0xe00->nd_reserved,set_field:2->nd_options_type,load:0->NXM_OF_IN_PORT[],output:2"

The kernel datapath flows:
#ovs-appctl dpctl/dump-flows
recirc_id(0),in_port(2),eth(src=fa:16:3e:55:ad:df,dst=ff:ff:ff:ff:ff:ff),eth_type(0x86dd),ipv6(src=fe80::f816:3eff:fe80:6715,dst=ff02::1,proto=58,frag=no),icmpv6(type=135,code=0),
packets:77, bytes:6622, used:0.033s,
actions:userspace(pid=2620280871,slow_path(action))

The ipv6 nd advertise packet captured at port:
root@n227-025-153:~# tcpdump -eni vethp1_host -nnvvv
tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
listening on vethp1_host, link-type EN10MB (Ethernet), capture size 262144
bytes
14:24:51.743409 00:23:15:d3:22:01 > fa:16:3e:55:ad:df, ethertype IPv6
(0x86dd), length 86: (hlim 255, next-header ICMPv6 (58) payload length: 32)
2001:db8:0:2::1 > fe80::f816:3eff:fe80:6715: [icmp6 sum ok] ICMP6, neighbor
advertisement, length 32, tgt is 2001:db8:0:2::1, Flags [none]
          destination link-address option (2), length 8 (1):
00:23:15:d3:22:01
            0x0000:  0023 15d3 2201

于2020年09月06日星期日 15:46 <fankaixi.li@bytedance.com> 写道:

From: "fankaixi.li" In order to support openflow rule which setting nd_ext
fields in openflow tables, we should remove setting nd_ext fields when
constructing rule. The ofproto would translate it into userspace actions
when handling upcalls. Signed-off-by: fankaixi.li ---
ofproto/ofproto-dpif.c | 8 -------- 1 file changed, 8 deletions(-) diff
--git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index
4f0638f23..f4c37f43d 100644 --- a/ofproto/ofproto-dpif.c +++
b/ofproto/ofproto-dpif.c @@ -4637,14 +4637,6 @@ check_actions(const struct
ofproto_dpif *ofproto, "ct original direction tuple"); return
OFPERR_NXBAC_CT_DATAPATH_SUPPORT; } - } else if (!support->nd_ext &&
ofpact->type == OFPACT_SET_FIELD) { - const struct mf_field *dst =
ofpact_get_mf_dst(ofpact); - - if (dst->id == MFF_ND_RESERVED || dst->id ==
MFF_ND_OPTIONS_TYPE) { - report_unsupported_act("set field", - "setting
IPv6 ND Extensions fields"); - return OFPERR_OFPBAC_BAD_SET_ARGUMENT; - } }
} -- 2.24.3 (Apple Git-128)
Kaixi Fan Sept. 8, 2020, 1:51 a.m. UTC | #2 
Hi,  I have modified this patch. Now only userspace code would be affected
by this patch. I have test it and please help to check if it could ok.
Thanks.

于2020年09月07日星期一 14:27 <fankaixi.li@bytedance.com> 写道:

I have test it with ovs 2.14.90 on kernel 4.19.117.

My test flows are as flows. Flow 1 match ipv6 neighbor discovery
solicitation packet, modify icmpv6 type and code and redirect it to table
81. Flow 2 modify ipv6 nd target ipv6 address and destination link-address
option.
# flow 1
ovs-ofctl -Oopenflow13 add-flow br0 "cookie=0x4000000, table=0,
priority=50,icmp6,icmp_type=135,icmp_code=0,nd_target=2001:db8:0:2:0:0:0:1,nd_sll=fa:16:3e:55:ad:df,actions=set_field:136->icmpv6_type,set_field:0->icmpv6_code,set_field:2->nd_options_type,goto_table:81"
# flow 2
ovs-ofctl -Oopenflow13 add-flow br0
"cookie=0x12220d57,table=81,priority=80,icmp6,icmp_type=136,icmp_code=0,nd_target=2001:db8:0:2:0:0:0:1,actions=
move:NXM_OF_ETH_SRC[]->NXM_OF_ETH_DST[],set_field:00:23:15:d3:22:01->eth_src,move:NXM_NX_IPV6_SRC[]->NXM_NX_IPV6_DST[],set_field:2001:db8:0:2:0:0:0:1->ipv6_src,set_field:00:23:15:d3:22:01->nd_tll,set_field:0xe00->nd_reserved,set_field:2->nd_options_type,load:0->NXM_OF_IN_PORT[],output:2"

The kernel datapath flows:
#ovs-appctl dpctl/dump-flows
recirc_id(0),in_port(2),eth(src=fa:16:3e:55:ad:df,dst=ff:ff:ff:ff:ff:ff),eth_type(0x86dd),ipv6(src=fe80::f816:3eff:fe80:6715,dst=ff02::1,proto=58,frag=no),icmpv6(type=135,code=0),
packets:77, bytes:6622, used:0.033s,
actions:userspace(pid=2620280871,slow_path(action))

The ipv6 nd advertise packet captured at port:
root@n227-025-153:~# tcpdump -eni vethp1_host -nnvvv
tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
listening on vethp1_host, link-type EN10MB (Ethernet), capture size 262144
bytes
14:24:51.743409 00:23:15:d3:22:01 > fa:16:3e:55:ad:df, ethertype IPv6
(0x86dd), length 86: (hlim 255, next-header ICMPv6 (58) payload length: 32)
2001:db8:0:2::1 > fe80::f816:3eff:fe80:6715: [icmp6 sum ok] ICMP6, neighbor
advertisement, length 32, tgt is 2001:db8:0:2::1, Flags [none]
          destination link-address option (2), length 8 (1):
00:23:15:d3:22:01
            0x0000:  0023 15d3 2201

于2020年09月06日星期日 15:46 <fankaixi.li@bytedance.com> 写道:

From: "fankaixi.li" In order to support openflow rule which setting nd_ext
fields in openflow tables, we should remove setting nd_ext fields when
constructing rule. The ofproto would translate it into userspace actions
when handling upcalls. Signed-off-by: fankaixi.li ---
ofproto/ofproto-dpif.c | 8 -------- 1 file changed, 8 deletions(-) diff
--git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index
4f0638f23..f4c37f43d 100644 --- a/ofproto/ofproto-dpif.c +++
b/ofproto/ofproto-dpif.c @@ -4637,14 +4637,6 @@ check_actions(const struct
ofproto_dpif *ofproto, "ct original direction tuple"); return
OFPERR_NXBAC_CT_DATAPATH_SUPPORT; } - } else if (!support->nd_ext &&
ofpact->type == OFPACT_SET_FIELD) { - const struct mf_field *dst =
ofpact_get_mf_dst(ofpact); - - if (dst->id == MFF_ND_RESERVED || dst->id ==
MFF_ND_OPTIONS_TYPE) { - report_unsupported_act("set field", - "setting
IPv6 ND Extensions fields"); - return OFPERR_OFPBAC_BAD_SET_ARGUMENT; - } }
} -- 2.24.3 (Apple Git-128)
Aaron Conole Sept. 9, 2020, 3:28 p.m. UTC | #3 
范开喜 <fankaixi.li@bytedance.com> writes:

> Hi,  I have modified this patch. Now only userspace code would be affected by this patch. I have test it and please help to
> check if it could ok. Thanks.

I don't see a proper patch file.  Did you send one?

>  于2020年09月07日星期一 14:27 <fankaixi.li@bytedance.com> 写道:
>
>  I have test it with ovs 2.14.90 on kernel 4.19.117.
>
>  My test flows are as flows. Flow 1 match ipv6 neighbor discovery solicitation packet, modify icmpv6 type and code
>  and redirect it to table 81. Flow 2 modify ipv6 nd target ipv6 address and destination link-address option. 
>  # flow 1
>  ovs-ofctl -Oopenflow13 add-flow br0 "cookie=0x4000000, table=0,
>  priority=50,icmp6,icmp_type=135,icmp_code=0,nd_target=2001:db8:0:2:0:0:0:1,nd_sll=fa:16:3e:55:ad:df,actions=set_field:136->icmpv6_type,set_field:0->icmpv6_code,set_field:2->nd_options_type,goto_table:81"
>  
>  # flow 2
>  ovs-ofctl -Oopenflow13 add-flow br0
>  "cookie=0x12220d57,table=81,priority=80,icmp6,icmp_type=136,icmp_code=0,nd_target=2001:db8:0:2:0:0:0:1,actions=
>  move:NXM_OF_ETH_SRC[]->NXM_OF_ETH_DST[],set_field:00:23:15:d3:22:01->eth_src,move:NXM_NX_IPV6_SRC
>  []->NXM_NX_IPV6_DST
>  [],set_field:2001:db8:0:2:0:0:0:1->ipv6_src,set_field:00:23:15:d3:22:01->nd_tll,set_field:0xe00->nd_reserved,set_field:2->nd_options_type,load:0->NXM_OF_IN_PORT[],output:2"
>  
>
>  The kernel datapath flows:
>  #ovs-appctl dpctl/dump-flows
>  recirc_id(0),in_port(2),eth(src=fa:16:3e:55:ad:df,dst=ff:ff:ff:ff:ff:ff),eth_type(0x86dd),ipv6
>  (src=fe80::f816:3eff:fe80:6715,dst=ff02::1,proto=58,frag=no),icmpv6(type=135,code=0), packets:77,
>  bytes:6622, used:0.033s, actions:userspace(pid=2620280871,slow_path(action))
>
>  The ipv6 nd advertise packet captured at port:
>  root@n227-025-153:~# tcpdump -eni vethp1_host -nnvvv
>  tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
>  listening on vethp1_host, link-type EN10MB (Ethernet), capture size 262144 bytes
>  14:24:51.743409 00:23:15:d3:22:01 > fa:16:3e:55:ad:df, ethertype IPv6 (0x86dd), length 86: (hlim 255,
>  next-header ICMPv6 (58) payload length: 32) 2001:db8:0:2::1 > fe80::f816:3eff:fe80:6715: [icmp6 sum ok]
>  ICMP6, neighbor advertisement, length 32, tgt is 2001:db8:0:2::1, Flags [none]
>            destination link-address option (2), length 8 (1): 00:23:15:d3:22:01
>              0x0000:  0023 15d3 2201
>
>  于2020年09月06日星期日 15:46 <fankaixi.li@bytedance.com> 写道:
>
>  From: "fankaixi.li" In order to support openflow rule which setting nd_ext fields in openflow tables, we
>  should remove setting nd_ext fields when constructing rule. The ofproto would translate it into userspace
>  actions when handling upcalls. Signed-off-by: fankaixi.li --- ofproto/ofproto-dpif.c | 8 -------- 1 file
>  changed, 8 deletions(-) diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index
>  4f0638f23..f4c37f43d 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -4637,14
>  +4637,6 @@ check_actions(const struct ofproto_dpif *ofproto, "ct original direction tuple"); return
>  OFPERR_NXBAC_CT_DATAPATH_SUPPORT; } - } else if (!support->nd_ext && ofpact->type ==
>  OFPACT_SET_FIELD) { - const struct mf_field *dst = ofpact_get_mf_dst(ofpact); - - if (dst->id ==
>  MFF_ND_RESERVED || dst->id == MFF_ND_OPTIONS_TYPE) { - report_unsupported_act("set field", -
>  "setting IPv6 ND Extensions fields"); - return OFPERR_OFPBAC_BAD_SET_ARGUMENT; - } } } -- 2.24.3
>  (Apple Git-128)
diff mbox series

Patch

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 4f0638f23..f4c37f43d 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4637,14 +4637,6 @@  check_actions(const struct ofproto_dpif *ofproto,
                                        "ct original direction tuple");
                 return OFPERR_NXBAC_CT_DATAPATH_SUPPORT;
             }
-        } else if (!support->nd_ext && ofpact->type == OFPACT_SET_FIELD) {
-            const struct mf_field *dst = ofpact_get_mf_dst(ofpact);
-
-            if (dst->id == MFF_ND_RESERVED || dst->id == MFF_ND_OPTIONS_TYPE) {
-                report_unsupported_act("set field",
-                                       "setting IPv6 ND Extensions fields");
-                return OFPERR_OFPBAC_BAD_SET_ARGUMENT;
-            }
         }
     }