diff mbox series

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

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

Commit Message

范开喜 Sept. 28, 2020, 1:49 p.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.

When trying to add the following flows to ovs with kernel:
# flow 1
ovs-ofctl -Oopenflow13 add-flow br0 "cookie=0x4000000, table=0,
priority=50,icmp6,icmpv6_type=135,icmpv6_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:1"
# flow 2
ovs-ofctl -Oopenflow13 add-flow br0 "cookie=0x12220d57,table=1,priority=80,icmp6,icmpv6_type=136,icmpv6_code=0,nd_target=2001:db8:0:2:0:0:0:1,actions= move:eth_src[]->eth_dst[],set_field:00:23:15:d3:22:01->eth_src,move:ipv6_src[]->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,output:3"

Ovs would emit a error message:
OFPT_ERROR (OF1.3) (xid=0x6): OFPBAC_BAD_SET_ARGUMENT

The output of log file shows a error message:
2020-09-28T12:22:09.453Z|00036|ofproto_dpif|WARN|Rejecting set field action because datapath does not support setting IPv6 ND Extensions fields (your kernel module may be out of date)
2020-09-28T12:22:09.453Z|00037|connmgr|INFO|br0<->unix#14: sending OFPBAC_BAD_SET_ARGUMENT error reply to OFPT_FLOW_MOD message

This patch would fix this error.

Also following Flavio Leitner's advise, I have prepared a testcase for this patch. 
But after running auto-testcases, I found that the test framework ovs support flows with actions to set nd_reserved field. 
The testsuites.log shows table 0 supporting setting nd_reserved and nd_options_type fields, and it shows that ovs was configured at dummy mode. 
I think there are some differences bewteen these two ovs modes. So I decide not commit the testcase about this fix. 

Signed-off-by: fankaixi.li <fankaixi.li@bytedance.com>
CC: Flavio Leitner <fbl@sysclose.org>
Fixes: d0d571493cf8 ("ofproto-dpif: Allow IPv6 ND Extensions only if supported")
---
 ofproto/ofproto-dpif.c | 8 --------
 1 file changed, 8 deletions(-)

Comments

Aaron Conole Sept. 28, 2020, 3:39 p.m. UTC | #1
fankaixi.li@bytedance.com writes:

> 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.
>
> When trying to add the following flows to ovs with kernel:
> # flow 1
> ovs-ofctl -Oopenflow13 add-flow br0 "cookie=0x4000000, table=0,
> priority=50,icmp6,icmpv6_type=135,icmpv6_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:1"
> # flow 2
> ovs-ofctl -Oopenflow13 add-flow br0 "cookie=0x12220d57,table=1,priority=80,icmp6,icmpv6_type=136,icmpv6_code=0,nd_target=2001:db8:0:2:0:0:0:1,actions= move:eth_src[]->eth_dst[],set_field:00:23:15:d3:22:01->eth_src,move:ipv6_src[]->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,output:3"
>
> Ovs would emit a error message:
> OFPT_ERROR (OF1.3) (xid=0x6): OFPBAC_BAD_SET_ARGUMENT
>
> The output of log file shows a error message:
> 2020-09-28T12:22:09.453Z|00036|ofproto_dpif|WARN|Rejecting set field action because datapath does not support setting IPv6 ND Extensions fields (your kernel module may be out of date)
> 2020-09-28T12:22:09.453Z|00037|connmgr|INFO|br0<->unix#14: sending OFPBAC_BAD_SET_ARGUMENT error reply to OFPT_FLOW_MOD message
>
> This patch would fix this error.
>
> Also following Flavio Leitner's advise, I have prepared a testcase for this patch. 
> But after running auto-testcases, I found that the test framework ovs support flows with actions to set nd_reserved field. 
> The testsuites.log shows table 0 supporting setting nd_reserved and nd_options_type fields, and it shows that ovs was configured at dummy mode. 
> I think there are some differences bewteen these two ovs modes. So I decide not commit the testcase about this fix. 

Please submit the test case so that we can look it over.  Flag it as RFC
and maybe we can figure out how to adjust it so it tests what you're
hoping.

> Signed-off-by: fankaixi.li <fankaixi.li@bytedance.com>
> CC: Flavio Leitner <fbl@sysclose.org>
> Fixes: d0d571493cf8 ("ofproto-dpif: Allow IPv6 ND Extensions only if supported")
> ---
>  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;
> -            }
>          }
>      }
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;
-            }
         }
     }