| Message ID | 1449714462-31831-7-git-send-email-diproiettod@vmware.com |
|---|---|
| State | Accepted |
| Headers | show |
With the comment below, Acked-by: Jarno Rajahalme <jarno@ovn.org> > On Dec 9, 2015, at 6:27 PM, Daniele Di Proietto <diproiettod@vmware.com> wrote: > > When converting between ODP attributes and struct flow_wildcards, we > check that all the prerequisites are exact matched on the mask. > > For ND(ICMPv6) attributes, an exact match on tp_src and tp_dst > (which in this context are the icmp type and code) shold look like > htons(0xff), not htons(0xffff). Fix this in two places. > > The consequences were that the datapath wouldn't match on the ND > attributes, and the flow would get revalidated away. > You mean that the datapath mask would not match the generated mask? Also, adding a reference to the big comment in xlate_wc_finish() would be informative. Jarno > Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> > --- > lib/odp-util.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/lib/odp-util.c b/lib/odp-util.c > index 3717aee..fbc0788 100644 > --- a/lib/odp-util.c > +++ b/lib/odp-util.c > @@ -4358,8 +4358,8 @@ odp_flow_key_from_flow__(const struct odp_flow_key_parms *parms, > if (flow->tp_dst == htons(0) > && (flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) > || flow->tp_src == htons(ND_NEIGHBOR_ADVERT)) > - && (!export_mask || (data->tp_src == htons(0xffff) > - && data->tp_dst == htons(0xffff)))) { > + && (!export_mask || (data->tp_src == htons(0xff) > + && data->tp_dst == htons(0xff)))) { > > struct ovs_key_nd *nd_key; > > @@ -4905,8 +4905,8 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], > flow->arp_tha = nd_key->nd_tll; > if (is_mask) { > if (!is_all_zeros(nd_key, sizeof *nd_key) && > - (flow->tp_src != htons(0xffff) || > - flow->tp_dst != htons(0xffff))) { > + (flow->tp_src != htons(0xff) || > + flow->tp_dst != htons(0xff))) { > return ODP_FIT_ERROR; > } else { > expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ND; > -- > 2.1.4 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
On 10/12/2015 15:16, "Jarno Rajahalme" <jarno@ovn.org> wrote: >With the comment below, > >Acked-by: Jarno Rajahalme <jarno@ovn.org> > >> On Dec 9, 2015, at 6:27 PM, Daniele Di Proietto >><diproiettod@vmware.com> wrote: >> >> When converting between ODP attributes and struct flow_wildcards, we >> check that all the prerequisites are exact matched on the mask. >> >> For ND(ICMPv6) attributes, an exact match on tp_src and tp_dst >> (which in this context are the icmp type and code) shold look like >> htons(0xff), not htons(0xffff). Fix this in two places. >> >> The consequences were that the datapath wouldn't match on the ND >> attributes, and the flow would get revalidated away. >> > >You mean that the datapath mask would not match the generated mask? I actually meant that the flow installed in the datapath would have the ND attributes wildcarded. I rephrased it to "The consequences were that the ODP mask wouldn't include the ND attributes and the flow would be deleted by the revalidation." > >Also, adding a reference to the big comment in xlate_wc_finish() would be >informative. Agreed, I've added that in the both places. Pushed to master and branch-2.5, thanks! > > Jarno > >> Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> >> --- >> lib/odp-util.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/lib/odp-util.c b/lib/odp-util.c >> index 3717aee..fbc0788 100644 >> --- a/lib/odp-util.c >> +++ b/lib/odp-util.c >> @@ -4358,8 +4358,8 @@ odp_flow_key_from_flow__(const struct >>odp_flow_key_parms *parms, >> if (flow->tp_dst == htons(0) >> && (flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) >> || flow->tp_src == htons(ND_NEIGHBOR_ADVERT)) >> - && (!export_mask || (data->tp_src == htons(0xffff) >> - && data->tp_dst == >>htons(0xffff)))) { >> + && (!export_mask || (data->tp_src == htons(0xff) >> + && data->tp_dst == htons(0xff)))) >>{ >> >> struct ovs_key_nd *nd_key; >> >> @@ -4905,8 +4905,8 @@ parse_l2_5_onward(const struct nlattr >>*attrs[OVS_KEY_ATTR_MAX + 1], >> flow->arp_tha = nd_key->nd_tll; >> if (is_mask) { >> if (!is_all_zeros(nd_key, sizeof *nd_key) && >> - (flow->tp_src != htons(0xffff) || >> - flow->tp_dst != htons(0xffff))) { >> + (flow->tp_src != htons(0xff) || >> + flow->tp_dst != htons(0xff))) { >> return ODP_FIT_ERROR; >> } else { >> expected_attrs |= UINT64_C(1) << >>OVS_KEY_ATTR_ND; >> -- >> 2.1.4 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> >>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailm >>an_listinfo_dev&d=BQIFAg&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r= >>SmB5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=Xezpsxh69y-Uz_6rO62BbHfP27O >>_O-dXO4nIt8fNWaM&s=t4pQ27B0QAdHA45YBD1orCBHiE_1QXHu7zjhJ3auKPc&e= >
diff --git a/lib/odp-util.c b/lib/odp-util.c index 3717aee..fbc0788 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -4358,8 +4358,8 @@ odp_flow_key_from_flow__(const struct odp_flow_key_parms *parms, if (flow->tp_dst == htons(0) && (flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) || flow->tp_src == htons(ND_NEIGHBOR_ADVERT)) - && (!export_mask || (data->tp_src == htons(0xffff) - && data->tp_dst == htons(0xffff)))) { + && (!export_mask || (data->tp_src == htons(0xff) + && data->tp_dst == htons(0xff)))) { struct ovs_key_nd *nd_key; @@ -4905,8 +4905,8 @@ parse_l2_5_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1], flow->arp_tha = nd_key->nd_tll; if (is_mask) { if (!is_all_zeros(nd_key, sizeof *nd_key) && - (flow->tp_src != htons(0xffff) || - flow->tp_dst != htons(0xffff))) { + (flow->tp_src != htons(0xff) || + flow->tp_dst != htons(0xff))) { return ODP_FIT_ERROR; } else { expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ND;
When converting between ODP attributes and struct flow_wildcards, we check that all the prerequisites are exact matched on the mask. For ND(ICMPv6) attributes, an exact match on tp_src and tp_dst (which in this context are the icmp type and code) shold look like htons(0xff), not htons(0xffff). Fix this in two places. The consequences were that the datapath wouldn't match on the ND attributes, and the flow would get revalidated away. Signed-off-by: Daniele Di Proietto <diproiettod@vmware.com> --- lib/odp-util.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)