Thanks, I applied this and the previous patch to master, branch-2.6 and branch-2.5
I'll rebase and send the rest of the series (which is mostly for master) in the next few days. Thanks, Daniele On 19/09/2016 17:14, "Jarno Rajahalme" <ja...@ovn.org> wrote: >Acked-by: Jarno Rajahalme <ja...@ovn.org> > >IMO this and the earlier (2/5) one are bug fixes that should be cherry-picked >to branches 2.6 and 2.5, if applicable. > > Jarno > >> On Aug 30, 2016, at 6:47 PM, Daniele Di Proietto <diproiet...@vmware.com> >> wrote: >> >> tnl_neigh_snoop() is part of the translation. During translation we >> have to unwildcard all the fields we examine to make a decision. >> >> tnl_arp_snoop() and tnl_nd_snoop() failed to unwildcard fileds in case >> of failure. The solution is to do unwildcarding before the field is >> inspected. >> >> Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com> >> --- >> lib/flow.h | 7 +++++++ >> lib/tnl-neigh-cache.c | 22 ++++++++-------------- >> 2 files changed, 15 insertions(+), 14 deletions(-) >> >> diff --git a/lib/flow.h b/lib/flow.h >> index 5b83695..ea24e28 100644 >> --- a/lib/flow.h >> +++ b/lib/flow.h >> @@ -854,6 +854,13 @@ pkt_metadata_from_flow(struct pkt_metadata *md, const >> struct flow *flow) >> md->ct_label = flow->ct_label; >> } >> >> +/* Often, during translation we need to read a value from a flow('FLOW') and >> + * unwildcard the corresponding bits in the wildcards('WC'). This macro >> makes >> + * it easier to do that. */ >> + >> +#define FLOW_WC_GET_AND_MASK_WC(FLOW, WC, FIELD) \ >> + (((WC) ? WC_MASK_FIELD(WC, FIELD) : NULL), ((FLOW)->FIELD)) >> + >> static inline bool is_vlan(const struct flow *flow, >> struct flow_wildcards *wc) >> { >> diff --git a/lib/tnl-neigh-cache.c b/lib/tnl-neigh-cache.c >> index f7d29f6..499efff 100644 >> --- a/lib/tnl-neigh-cache.c >> +++ b/lib/tnl-neigh-cache.c >> @@ -115,7 +115,7 @@ tnl_neigh_delete(struct tnl_neigh_entry *neigh) >> >> static void >> tnl_neigh_set__(const char name[IFNAMSIZ], const struct in6_addr *dst, >> - const struct eth_addr mac) >> + const struct eth_addr mac) >> { >> ovs_mutex_lock(&mutex); >> struct tnl_neigh_entry *neigh = tnl_neigh_lookup__(name, dst); >> @@ -151,26 +151,21 @@ static int >> tnl_arp_snoop(const struct flow *flow, struct flow_wildcards *wc, >> const char name[IFNAMSIZ]) >> { >> - if (flow->dl_type != htons(ETH_TYPE_ARP) || >> - flow->nw_proto != ARP_OP_REPLY || >> - eth_addr_is_zero(flow->arp_sha)) { >> + if (flow->dl_type != htons(ETH_TYPE_ARP) >> + || FLOW_WC_GET_AND_MASK_WC(flow, wc, nw_proto) != ARP_OP_REPLY >> + || eth_addr_is_zero(FLOW_WC_GET_AND_MASK_WC(flow, wc, arp_sha))) { >> return EINVAL; >> } >> >> - /* Exact Match on all ARP flows. */ >> - memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto); >> - memset(&wc->masks.nw_src, 0xff, sizeof wc->masks.nw_src); >> - memset(&wc->masks.arp_sha, 0xff, sizeof wc->masks.arp_sha); >> - >> - tnl_arp_set(name, flow->nw_src, flow->arp_sha); >> + tnl_arp_set(name, FLOW_WC_GET_AND_MASK_WC(flow, wc, nw_src), >> flow->arp_sha); >> return 0; >> } >> >> static int >> tnl_nd_snoop(const struct flow *flow, struct flow_wildcards *wc, >> - const char name[IFNAMSIZ]) >> + const char name[IFNAMSIZ]) >> { >> - if (!is_nd(flow, NULL) || flow->tp_src != htons(ND_NEIGHBOR_ADVERT)) { >> + if (!is_nd(flow, wc) || flow->tp_src != htons(ND_NEIGHBOR_ADVERT)) { >> return EINVAL; >> } >> /* - RFC4861 says Neighbor Advertisements sent in response to unicast >> Neighbor >> @@ -179,14 +174,13 @@ tnl_nd_snoop(const struct flow *flow, struct >> flow_wildcards *wc, >> * TLL address and other Advertisements not including it can be >> ignored. >> * - OVS flow extract can set this field to zero in case of packet >> parsing errors. >> * For details refer miniflow_extract()*/ >> - if (eth_addr_is_zero(flow->arp_tha)) { >> + if (eth_addr_is_zero(FLOW_WC_GET_AND_MASK_WC(flow, wc, arp_tha))) { >> return EINVAL; >> } >> >> memset(&wc->masks.ipv6_src, 0xff, sizeof wc->masks.ipv6_src); >> memset(&wc->masks.ipv6_dst, 0xff, sizeof wc->masks.ipv6_dst); >> memset(&wc->masks.nd_target, 0xff, sizeof wc->masks.nd_target); >> - memset(&wc->masks.arp_tha, 0xff, sizeof wc->masks.arp_tha); >> >> tnl_neigh_set__(name, &flow->nd_target, flow->arp_tha); >> return 0; >> -- >> 2.9.3 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev