Hi, Could you use the e-mail address zoltan.balogh.eth@gmail, please?
Thanks, Zoltan On 4 April 2018 at 01:52, Jan Scheurich <jan.scheur...@ericsson.com> wrote: > Hi, > > I took this over from Zoltan and investigated the failing unit test a bit > further. > > The essential difference between master and Zoltan's (rebased) patches is > that the dynamic "ARP cache" flow entry > > table=66,priority=100,reg0=0xa000002,reg15=0x2,metadata=0x1 > actions=set_field:f0:00:00:01:02:04->eth_dst > > (which OVN installs in response to the ARP reply it receives from OVS in > PACKET_IN after the first injected IP packet) triggers different behavior in > the subsequent revalidation: > > In the case of master the existing datapath flow entry > > recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=f0:00:00:01:02:03,dst=00:00:00:01:02:03),eth_type(0x0800),ipv4(src=192.168.1.2/255.255.255.254,dst=10.0.0.2/248.0.0.0,ttl=64,frag=no), > > actions:ct_clear,set(eth(src=00:00:00:01:02:04,dst=00:00:00:00:00:00)),set(ipv4(src=192.168.1.2/255.255.255.254,dst=8.0.0.0/248.0.0.0,ttl=63)),userspace(pid=0,controller(reason=1,dont_send=0,continuation=0,recirc_id=1,rule_cookie=0x62066318,controller_id=0,max_len=65535)) > > is correctly deleted during revalidation due to the presence of the new rule > in table 66, while with Zoltan's patch that same datapath flow entry remains > untouched. It seems as if with the patch the revalidation of the datapath > flow does not reach the new rule in table 66. > > Consequently the next IP packet injected by netdev-dummy/receive then matches > the datapath flow and is sent up to OVN again, where it triggers the same ARP > query procedure as before. > > However, if I add a "sleep 11" before injecting the second IP packet to let > the datapath flow entry time out, the test succeeds, proving that the OF > pipeline behaves correctly for a real packet but not when revalidating the > datapath flow entry with the userspace(controller(...)) action. > > I still do not understand how the (passive) tunnel ARP snooping patch can > possibly change the translation behavior for an IP packet in such a way that > it affects the revalidation result. > > Any idea is welcome. > > Regards, Jan > > > BTW: The “pseudo” tunnel neighbor cache entry we get in this test on master > for the tenant IP address 10.0.0.2 > > IP MAC Bridge > ====================================================== > 10.0.0.2 f0:00:00:01:02:04 br-int > 192.168.0.2 5e:97:6a:82:7d:41 br-phys > > is a good example why we need Zoltan's patch. Any IP address in an ARP reply > is blindly inserted into the tunnel neighbor cache. Overlapping IP addresses > among tenants can cause frequent overwriting of cache entries, in the worst > case leading to continuous configuration changes and revalidation. > > >> -----Original Message----- >> From: Zoltán Balogh >> Sent: Friday, 02 February, 2018 15:42 >> To: Justin Pettit <jpet...@ovn.org>; Ben Pfaff <b...@ovn.org> >> Cc: g...@ovn.org; d...@openvswitch.org; Jan Scheurich >> <jan.scheur...@ericsson.com> >> Subject: RE: [ovs-dev] [PATCH v3 3/3] xlate: call tnl_neigh_snoop() from >> terminate_native_tunnel() >> >> Hi Justin, >> >> I rebased the patches to recent master. Please find them attached. >> >> Best regards, >> Zoltan >> >> > -----Original Message----- >> > From: Justin Pettit [mailto:jpet...@ovn.org] >> > Sent: Friday, February 02, 2018 12:00 AM >> > To: Ben Pfaff <mailto:b...@ovn.org> >> > Cc: Zoltán Balogh <mailto:zoltan.bal...@ericsson.com>; >> > mailto:g...@ovn.org; mailto:d...@openvswitch.org; Jan Scheurich >> > <mailto:jan.scheur...@ericsson.com> >> > Subject: Re: [ovs-dev] [PATCH v3 3/3] xlate: call tnl_neigh_snoop() from >> > terminate_native_tunnel() >> > >> > I wasn't able to get this patch to apply to the tip of master. Zoltan, >> > can you rebase this patch and repost? >> > >> > The main thing my patch series does is make it so that packets that have a >> > controller action aren't processed >> > entirely in userspace. If, for example, the patches expect packets to be >> > in userspace without an explicit slow-path >> > request when generating the datapath flow, then that would be a problem. >> > >> > --Justin >> > >> > >> > > On Feb 1, 2018, at 2:17 PM, Ben Pfaff <mailto:b...@ovn.org> wrote: >> > > >> > > Justin, I think this is mainly a question about your patches, can you >> > > take a look? >> > > >> > > On Fri, Jan 26, 2018 at 01:08:35PM +0000, Zoltán Balogh wrote: >> > >> Hi, >> > >> >> > >> I've been investigating the failing unit test. I can confirm, it does >> > >> fail >> > >> with my series on master. However, when I created the series and sent >> > >> it to >> > >> the mailing list it did not. >> > >> >> > >> I've rebased my series to this commit before I sent it to the mailing >> > >> list. >> > >> commit f59cb331c481d08f9a851c07cf31e9d826650485 >> > >> Author: Yi Yang <mailto:yi.y.y...@intel.com> >> > >> Date: Sat Jan 6 13:47:51 2018 +0800 >> > >> >> > >> If I apply the series to the commit below, the test does pass. >> > >> >> > >> So, I started to search for any commits between the one from Yi Yang >> > >> and the >> > >> Last one on master which could make my series fail on the OVN unit test >> > >> "/32 router IP address". >> > >> >> > >> I found, that reverting the series of two commits from Justin Pettit >> > >> below, >> > >> makes my series pass the unit test again. Even rebased to master. >> > >> >> > >> commit 74c4530dca939109f3fb79776b60b8722e149738 >> > >> Author: Justin Pettit <mailto:jpet...@ovn.org> >> > >> Date: Wed Oct 18 23:16:22 2017 -0700 >> > >> >> > >> ofproto-dpif: Don't slow-path controller actions with pause. >> > >> >> > >> A previous patch removed slow-pathing for controller actions with >> > >> the >> > >> exception of ones that specified "pause". This commit removes that >> > >> restriction so that no controller actions are slow-pathed. >> > >> >> > >> Signed-off-by: Justin Pettit <mailto:jpet...@ovn.org> >> > >> Acked-by: Ben Pfaff <mailto:b...@ovn.org> >> > >> >> > >> commit d39ec23de38464ee35b3098b9f6c5f06d5191015 >> > >> Author: Justin Pettit <mailto:jpet...@ovn.org> >> > >> Date: Wed Jul 5 15:17:52 2017 -0700 >> > >> >> > >> ofproto-dpif: Don't slow-path controller actions. >> > >> >> > >> Controller actions have become more commonly used for purposes >> > >> other >> > >> than just making forwarding decisions (e.g., packet logging). A >> > >> packet >> > >> that needs to be copied to the controller and forwarded would >> > >> always be >> > >> sent to ovs-vswitchd to be handled, which could negatively affect >> > >> performance and cause heavier CPU utilization in ovs-vswitchd. >> > >> >> > >> This commit changes the behavior so that OpenFlow controller >> > >> actions >> > >> become userspace datapath actions while continuing to let packet >> > >> forwarding and manipulation continue to be handled by the datapath >> > >> directly. >> > >> >> > >> This patch still slow-paths controller actions with the "pause" >> > >> flag >> > >> set. A future patch will stop slow-pathing these pause actions as >> > >> well. >> > >> >> > >> Signed-off-by: Justin Pettit <mailto:jpet...@ovn.org> >> > >> Acked-by: Ben Pfaff <mailto:b...@ovn.org> >> > >> >> > >> The main difference, my series does compared to master, is not putting >> > >> the >> > >> ARP entry of {10.0.0.2, f0:00:00:01:02:04, br-int} into tunnel neighbor >> > >> cache. All the OF rules are almost the same, except loading different >> > >> numbers into NXM_NX_REGx and using different metadata. >> > >> >> > >> How should this be related to Justin's series? >> > >> >> > >> If I modify the unit test and populate the missing ARP entry then the >> > >> test >> > >> does pass. >> > >> >> > >> Should this entry be present in the ARP neighbor cache in order to make >> > >> the >> > >> test pass? If yes, then why? >> > >> >> > >> Best regards, >> > >> Zoltan >> > >> >> > >>> -----Original Message----- >> > >>> From: Ben Pfaff [mailto:b...@ovn.org] >> > >>> Sent: Tuesday, January 23, 2018 8:02 PM >> > >>> To: Zoltán Balogh <mailto:zoltan.bal...@ericsson.com> >> > >>> Cc: mailto:d...@openvswitch.org; Jan Scheurich >> > >>> <mailto:jan.scheur...@ericsson.com> >> > >>> Subject: Re: [ovs-dev] [PATCH v3 3/3] xlate: call tnl_neigh_snoop() >> > >>> from terminate_native_tunnel() >> > >>> >> > >>> On Tue, Jan 09, 2018 at 07:54:33PM +0100, Zoltan Balogh wrote: >> > >>>> Currenlty, OVS snoops any ARP or ND packets in any bridge and >> > >>>> populates >> > >>>> the tunnel neighbor cache with the retreived data. For instance, when >> > >>>> ARP reply originated by a tenant is received in an overlay bridge, the >> > >>>> ARP message is snooped and tunnel neighbor cache is filled with tenant >> > >>>> data, however only tunnel neighbor data should be stored there. >> > >>>> >> > >>>> This patch moves tunnel neighbor snooping from do_xlate_actions() to >> > >>>> terminate_native_tunnel() where native tunnel is terminated, in order >> > >>>> to >> > >>>> keep ARP neighbor cache clean, i.e. only packets comming from a native >> > >>>> tunnel are snooped. >> > >>>> >> > >>>> By applying the patch, only ARP and Neighbor Advertisement messages >> > >>>> addressing a tunnel endpoint (LOCAL port of underlay bridge) are >> > >>>> snooped. In order to achieve this, IP addresses of the bridge are >> > >>>> retrieved and then stored in xbridge by calling xlate_xbridge_set(). >> > >>>> These data is then matched against the data extracted from an ARP or >> > >>>> Neighbor Advertisement message in is_neighbor_reply_correct() which is >> > >>>> invoked from terminate_native_tunnel(). >> > >>>> >> > >>>> Signed-off-by: Zoltan Balogh <mailto:zoltan.bal...@ericsson.com> >> > >>> >> > >>> Thanks a lot! >> > >>> >> > >>> It appears to me that this patch makes the following test consistently >> > >>> fail. When I revert the test, it passes for me again. Can you take a >> > >>> look? >> > >>> >> > >>> Thanks, >> > >>> >> > >>> Ben. >> > >>> >> > >>> # -*- compilation -*- >> > >>> 2370. ovn.at:8136: testing ovn -- /32 router IP address ... >> > >>> creating ovn-sb database >> > >>> creating ovn-nb database >> > >>> starting ovn-northd >> > >>> starting backup ovn-northd >> > >>> adding simulator 'main' >> > >>> adding simulator 'hv1' >> > >>> adding simulator 'hv2' >> > >>> ../../tests/ovn.at:8198: ovn_populate_arp__ >> > >>> stdout: >> > >>> OK >> > >>> OK >> > >>> >> > >>> checking packets in hv2/vif1-tx.pcap against expected: >> > >>> expected 1 packets, only received 0 >> > >>> ../../tests/ovn.at:8230: sort $rcv_text >> > >>> --- expout 2018-01-23 10:59:47.231797273 -0800 >> > >>> +++ >> > >>> /home/blp/nicira/ovs/_build/tests/testsuite.dir/at-groups/2370/stdout >> > >>> 2018-01-23 10:59:47.231797273 - >> > 0800 >> > >>> @@ -1 +0,0 @@ >> > >>> -f0000001020400000001020408004500001c000000003f110100c0a801020a0000020035111100080000 >> > >>> 2370. ovn.at:8136: 2370. ovn -- /32 router IP address (ovn.at:8136): >> > >>> FAILED (ovn.at:8230) >> > >>> >> > >>> >> > >>>> --- >> > >>>> include/sparse/netinet/in.h | 10 +++ >> > >>>> ofproto/ofproto-dpif-xlate.c | 147 >> > >>>> ++++++++++++++++++++++++++++++++++++++++-- >> > >>>> tests/tunnel-push-pop-ipv6.at | 68 ++++++++++++++++++- >> > >>>> tests/tunnel-push-pop.at | 67 ++++++++++++++++++- >> > >>>> 4 files changed, 282 insertions(+), 10 deletions(-) >> > >>>> >> > >>>> diff --git a/include/sparse/netinet/in.h b/include/sparse/netinet/in.h >> > >>>> index 6abdb2331..eea41bd7f 100644 >> > >>>> --- a/include/sparse/netinet/in.h >> > >>>> +++ b/include/sparse/netinet/in.h >> > >>>> @@ -123,6 +123,16 @@ struct sockaddr_in6 { >> > >>>> (X)->s6_addr[10] == 0xff && \ >> > >>>> (X)->s6_addr[11] == 0xff) >> > >>>> >> > >>>> +#define IN6_IS_ADDR_MC_LINKLOCAL(a) \ >> > >>>> + (((const uint8_t *) (a))[0] == 0xff && \ >> > >>>> + (((const uint8_t *) (a))[1] & 0xf) == 0x2) >> > >>>> + >> > >>>> +# define IN6_ARE_ADDR_EQUAL(a,b) >> > >>>> \ >> > >>>> + ((((const uint32_t *) (a))[0] == ((const uint32_t *) (b))[0]) && >> > >>>> \ >> > >>>> + (((const uint32_t *) (a))[1] == ((const uint32_t *) (b))[1]) && >> > >>>> \ >> > >>>> + (((const uint32_t *) (a))[2] == ((const uint32_t *) (b))[2]) && >> > >>>> \ >> > >>>> + (((const uint32_t *) (a))[3] == ((const uint32_t *) (b))[3])) >> > >>>> + >> > >>>> #define INET_ADDRSTRLEN 16 >> > >>>> #define INET6_ADDRSTRLEN 46 >> > >>>> >> > >>>> diff --git a/ofproto/ofproto-dpif-xlate.c >> > >>>> b/ofproto/ofproto-dpif-xlate.c >> > >>>> index 2328ec1da..16bc0cd96 100644 >> > >>>> --- a/ofproto/ofproto-dpif-xlate.c >> > >>>> +++ b/ofproto/ofproto-dpif-xlate.c >> > >>>> @@ -90,6 +90,16 @@ VLOG_DEFINE_THIS_MODULE(ofproto_dpif_xlate); >> > >>>> * recursive or not. */ >> > >>>> #define MAX_RESUBMITS (MAX_DEPTH * MAX_DEPTH) >> > >>>> >> > >>>> +/* The structure holds an array of IP addresses assigned to a bridge >> > >>>> and the >> > >>>> + * number of elements in the array. These data are mutable and are >> > >>>> evaluated >> > >>>> + * when ARP or Neighbor Advertisement packets received on a native >> > >>>> tunnel >> > >>>> + * port are xlated. So 'ref_cnt' and RCU are used for >> > >>>> synchronization. */ >> > >>>> +struct xbridge_addr { >> > >>>> + struct in6_addr *addr; /* Array of IP addresses of >> > >>>> xbridge. */ >> > >>>> + int n_addr; /* Number of IP addresses. */ >> > >>>> + struct ovs_refcount ref_cnt; >> > >>>> +}; >> > >>>> + >> > >>>> struct xbridge { >> > >>>> struct hmap_node hmap_node; /* Node in global 'xbridges' map. */ >> > >>>> struct ofproto_dpif *ofproto; /* Key in global 'xbridges' map. */ >> > >>>> @@ -113,6 +123,8 @@ struct xbridge { >> > >>>> >> > >>>> /* Datapath feature support. */ >> > >>>> struct dpif_backer_support support; >> > >>>> + >> > >>>> + struct xbridge_addr *addr; >> > >>>> }; >> > >>>> >> > >>>> struct xbundle { >> > >>>> @@ -576,7 +588,8 @@ static void xlate_xbridge_set(struct xbridge *, >> > >>>> struct dpif *, >> > >>>> const struct dpif_ipfix *, >> > >>>> const struct netflow *, >> > >>>> bool forward_bpdu, bool has_in_band, >> > >>>> - const struct dpif_backer_support *); >> > >>>> + const struct dpif_backer_support *, >> > >>>> + const struct xbridge_addr *); >> > >>>> static void xlate_xbundle_set(struct xbundle *xbundle, >> > >>>> enum port_vlan_mode vlan_mode, >> > >>>> uint16_t qinq_ethtype, int vlan, >> > >>>> @@ -826,6 +839,56 @@ xlate_xport_init(struct xlate_cfg *xcfg, struct >> > >>>> xport *xport) >> > >>>> hash_ofp_port(xport->ofp_port)); >> > >>>> } >> > >>>> >> > >>>> +static struct xbridge_addr * >> > >>>> +xbridge_addr_create(struct xbridge *xbridge) >> > >>>> +{ >> > >>>> + struct xbridge_addr *xbridge_addr = xbridge->addr; >> > >>>> + struct in6_addr *addr = NULL, *mask = NULL; >> > >>>> + struct netdev *dev; >> > >>>> + int err, n_addr = 0; >> > >>>> + >> > >>>> + err = netdev_open(xbridge->name, NULL, &dev); >> > >>>> + if (!err) { >> > >>>> + err = netdev_get_addr_list(dev, &addr, &mask, &n_addr); >> > >>>> + if (!err) { >> > >>>> + if (!xbridge->addr || >> > >>>> + n_addr != xbridge->addr->n_addr || >> > >>>> + (xbridge->addr->addr && memcmp(addr, >> > >>>> xbridge->addr->addr, >> > >>>> + sizeof(*addr) * >> > >>>> n_addr))) { >> > >>>> + xbridge_addr = xzalloc(sizeof *xbridge_addr); >> > >>>> + xbridge_addr->addr = addr; >> > >>>> + xbridge_addr->n_addr = n_addr; >> > >>>> + ovs_refcount_init(&xbridge_addr->ref_cnt); >> > >>>> + } else { >> > >>>> + free(addr); >> > >>>> + } >> > >>>> + free(mask); >> > >>>> + } >> > >>>> + netdev_close(dev); >> > >>>> + } >> > >>>> + >> > >>>> + return xbridge_addr; >> > >>>> +} >> > >>>> + >> > >>>> +static struct xbridge_addr * >> > >>>> +xbridge_addr_ref(const struct xbridge_addr *addr_) >> > >>>> +{ >> > >>>> + struct xbridge_addr *addr = CONST_CAST(struct xbridge_addr *, >> > >>>> addr_); >> > >>>> + if (addr) { >> > >>>> + ovs_refcount_ref(&addr->ref_cnt); >> > >>>> + } >> > >>>> + return addr; >> > >>>> +} >> > >>>> + >> > >>>> +static void >> > >>>> +xbridge_addr_unref(struct xbridge_addr *addr) >> > >>>> +{ >> > >>>> + if (addr && ovs_refcount_unref_relaxed(&addr->ref_cnt) == 1) { >> > >>>> + free(addr->addr); >> > >>>> + free(addr); >> > >>>> + } >> > >>>> +} >> > >>>> + >> > >>>> static void >> > >>>> xlate_xbridge_set(struct xbridge *xbridge, >> > >>>> struct dpif *dpif, >> > >>>> @@ -836,7 +899,8 @@ xlate_xbridge_set(struct xbridge *xbridge, >> > >>>> const struct dpif_ipfix *ipfix, >> > >>>> const struct netflow *netflow, >> > >>>> bool forward_bpdu, bool has_in_band, >> > >>>> - const struct dpif_backer_support *support) >> > >>>> + const struct dpif_backer_support *support, >> > >>>> + const struct xbridge_addr *addr) >> > >>>> { >> > >>>> if (xbridge->ml != ml) { >> > >>>> mac_learning_unref(xbridge->ml); >> > >>>> @@ -878,6 +942,11 @@ xlate_xbridge_set(struct xbridge *xbridge, >> > >>>> xbridge->netflow = netflow_ref(netflow); >> > >>>> } >> > >>>> >> > >>>> + if (xbridge->addr != addr) { >> > >>>> + xbridge_addr_unref(xbridge->addr); >> > >>>> + xbridge->addr = xbridge_addr_ref(addr); >> > >>>> + } >> > >>>> + >> > >>>> xbridge->dpif = dpif; >> > >>>> xbridge->forward_bpdu = forward_bpdu; >> > >>>> xbridge->has_in_band = has_in_band; >> > >>>> @@ -971,7 +1040,7 @@ xlate_xbridge_copy(struct xbridge *xbridge) >> > >>>> xbridge->rstp, xbridge->ms, xbridge->mbridge, >> > >>>> xbridge->sflow, xbridge->ipfix, >> > >>>> xbridge->netflow, >> > >>>> xbridge->forward_bpdu, xbridge->has_in_band, >> > >>>> - &xbridge->support); >> > >>>> + &xbridge->support, xbridge->addr); >> > >>>> LIST_FOR_EACH (xbundle, list_node, &xbridge->xbundles) { >> > >>>> xlate_xbundle_copy(new_xbridge, xbundle); >> > >>>> } >> > >>>> @@ -1126,6 +1195,7 @@ xlate_ofproto_set(struct ofproto_dpif *ofproto, >> > >>>> const char *name, >> > >>>> const struct dpif_backer_support *support) >> > >>>> { >> > >>>> struct xbridge *xbridge; >> > >>>> + struct xbridge_addr *xbridge_addr, *old_addr; >> > >>>> >> > >>>> ovs_assert(new_xcfg); >> > >>>> >> > >>>> @@ -1140,8 +1210,16 @@ xlate_ofproto_set(struct ofproto_dpif >> > >>>> *ofproto, const char *name, >> > >>>> free(xbridge->name); >> > >>>> xbridge->name = xstrdup(name); >> > >>>> >> > >>>> + xbridge_addr = xbridge_addr_create(xbridge); >> > >>>> + old_addr = xbridge->addr; >> > >>>> + >> > >>>> xlate_xbridge_set(xbridge, dpif, ml, stp, rstp, ms, mbridge, >> > >>>> sflow, ipfix, >> > >>>> - netflow, forward_bpdu, has_in_band, support); >> > >>>> + netflow, forward_bpdu, has_in_band, support, >> > >>>> + xbridge_addr); >> > >>>> + >> > >>>> + if (xbridge_addr != old_addr) { >> > >>>> + xbridge_addr_unref(xbridge_addr); >> > >>>> + } >> > >>>> } >> > >>>> >> > >>>> static void >> > >>>> @@ -1171,6 +1249,7 @@ xlate_xbridge_remove(struct xlate_cfg *xcfg, >> > >>>> struct xbridge *xbridge) >> > >>>> netflow_unref(xbridge->netflow); >> > >>>> stp_unref(xbridge->stp); >> > >>>> rstp_unref(xbridge->rstp); >> > >>>> + xbridge_addr_unref(xbridge->addr); >> > >>>> hmap_destroy(&xbridge->xports); >> > >>>> free(xbridge->name); >> > >>>> free(xbridge); >> > >>>> @@ -3655,6 +3734,54 @@ check_output_prerequisites(struct xlate_ctx >> > >>>> *ctx, >> > >>>> return true; >> > >>>> } >> > >>>> >> > >>>> +/* Function verifies if destination address of received Neighbor >> > >>>> Advertisement >> > >>>> + * message stored in 'flow' is correct. It should be either >> > >>>> FF02::1:FFXX:XXXX >> > >>>> + * where XX:XXXX stands for the last 24 bits of 'ipv6_addr' or it >> > >>>> should match >> > >>>> + * 'ipv6_addr'. */ >> > >>>> +static bool >> > >>>> +is_nd_dst_correct(const struct flow *flow, const struct in6_addr >> > >>>> *ipv6_addr) >> > >>>> +{ >> > >>>> + const uint8_t *flow_ipv6_addr = (uint8_t *) &flow->ipv6_dst; >> > >>>> + const uint8_t *addr = (uint8_t *) ipv6_addr; >> > >>>> + >> > >>>> + return (IN6_IS_ADDR_MC_LINKLOCAL(flow_ipv6_addr) && >> > >>>> + flow_ipv6_addr[11] == 0x01 && >> > >>>> + flow_ipv6_addr[12] == 0xff && >> > >>>> + flow_ipv6_addr[13] == addr[13] && >> > >>>> + flow_ipv6_addr[14] == addr[14] && >> > >>>> + flow_ipv6_addr[15] == addr[15]) || >> > >>>> + IN6_ARE_ADDR_EQUAL(&flow->ipv6_dst, ipv6_addr); >> > >>>> +} >> > >>>> + >> > >>>> +/* Function verifies if the ARP reply or Neighbor Advertisement >> > >>>> represented by >> > >>>> + * 'flow' addresses the 'xbridge' of 'ctx'. Returns true if the ARP >> > >>>> TA or >> > >>>> + * neighbor discovery destination is in the list of configured IP >> > >>>> addresses of >> > >>>> + * the bridge. Otherwise, it returns false. */ >> > >>>> +static bool >> > >>>> +is_neighbor_reply_correct(const struct xlate_ctx *ctx, const struct >> > >>>> flow *flow) >> > >>>> +{ >> > >>>> + bool ret = false; >> > >>>> + int i; >> > >>>> + struct xbridge_addr *xbridge_addr = >> > >>>> xbridge_addr_ref(ctx->xbridge->addr); >> > >>>> + >> > >>>> + /* Verify if 'nw_dst' of ARP or 'ipv6_dst' of ICMPV6 is in the >> > >>>> list. */ >> > >>>> + for (i = 0; xbridge_addr && i < xbridge_addr->n_addr; i++) { >> > >>>> + struct in6_addr *ip_addr = &xbridge_addr->addr[i]; >> > >>>> + if ((IN6_IS_ADDR_V4MAPPED(ip_addr) && >> > >>>> + flow->dl_type == htons(ETH_TYPE_ARP) && >> > >>>> + in6_addr_get_mapped_ipv4(ip_addr) == flow->nw_dst) || >> > >>>> + (!IN6_IS_ADDR_V4MAPPED(ip_addr) && >> > >>>> + is_nd_dst_correct(flow, ip_addr))) { >> > >>>> + /* Found a match. */ >> > >>>> + ret = true; >> > >>>> + break; >> > >>>> + } >> > >>>> + } >> > >>>> + >> > >>>> + xbridge_addr_unref(xbridge_addr); >> > >>>> + return ret; >> > >>>> +} >> > >>>> + >> > >>>> static bool >> > >>>> terminate_native_tunnel(struct xlate_ctx *ctx, ofp_port_t ofp_port, >> > >>>> struct flow *flow, struct flow_wildcards *wc, >> > >>>> @@ -3667,6 +3794,15 @@ terminate_native_tunnel(struct xlate_ctx *ctx, >> > >>>> ofp_port_t ofp_port, >> > >>>> if (ofp_port == OFPP_LOCAL && >> > >>>> ovs_native_tunneling_is_on(ctx->xbridge->ofproto)) { >> > >>>> *tnl_port = tnl_port_map_lookup(flow, wc); >> > >>>> + >> > >>>> + /* If no tunnel port was found and it's about an ARP or >> > >>>> ICMPv6 packet, >> > >>>> + * do tunnel neighbor snooping. */ >> > >>>> + if (*tnl_port == ODPP_NONE && >> > >>>> + (flow->dl_type == htons(ETH_TYPE_ARP) || >> > >>>> + flow->nw_proto == IPPROTO_ICMPV6) && >> > >>>> + is_neighbor_reply_correct(ctx, flow)) { >> > >>>> + tnl_neigh_snoop(flow, wc, ctx->xbridge->name); >> > >>>> + } >> > >>>> } >> > >>>> >> > >>>> return *tnl_port != ODPP_NONE; >> > >>>> @@ -6183,9 +6319,6 @@ do_xlate_actions(const struct ofpact *ofpacts, >> > >>>> size_t ofpacts_len, >> > >>>> struct flow *flow = &ctx->xin->flow; >> > >>>> const struct ofpact *a; >> > >>>> >> > >>>> - if (ovs_native_tunneling_is_on(ctx->xbridge->ofproto)) { >> > >>>> - tnl_neigh_snoop(flow, wc, ctx->xbridge->name); >> > >>>> - } >> > >>>> /* dl_type already in the mask, not set below. */ >> > >>>> >> > >>>> if (!ofpacts_len) { >> > >>>> diff --git a/tests/tunnel-push-pop-ipv6.at >> > >>>> b/tests/tunnel-push-pop-ipv6.at >> > >>>> index 0b9d436db..b0f4a7e91 100644 >> > >>>> --- a/tests/tunnel-push-pop-ipv6.at >> > >>>> +++ b/tests/tunnel-push-pop-ipv6.at >> > >>>> @@ -55,9 +55,73 @@ AT_CHECK([cat p0.pcap.txt | grep >> > >>>> 93aa55aa55000086dd6000000000203aff2001cafe | un >> > >>>> ]) >> > >>>> >> > >>>> dnl Check ARP Snoop >> > >>>> -AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> > >>> >> > 'in_port(1),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:00),eth_type(0x86dd),ipv6(src=2001:cafe::92,dst=2001:cafe:: >> > >>> >> > 94,label=0,proto=58,tclass=0,hlimit=255,frag=no),icmpv6(type=136,code=0),nd(target=2001:cafe::92,sll=00:00:00:00:00: >> > >>> 00,tll=f8:bc:12:44:34:b6)']) >> > >>>> +AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> > >>> >> > 'in_port(1),eth(src=f8:bc:12:44:34:c8,dst=aa:55:aa:55:00:00),eth_type(0x86dd),ipv6(src=2001:cafe::92,dst=2001:cafe:: >> > >>> >> > 88,label=0,proto=58,tclass=0,hlimit=255,frag=no),icmpv6(type=136,code=0),nd(target=2001:cafe::92,sll=00:00:00:00:00: >> > >>> 00,tll=f8:bc:12:44:34:c8)']) >> > >>>> >> > >>>> -AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> > >>> >> > 'in_port(1),eth(src=f8:bc:12:44:34:b7,dst=aa:55:aa:55:00:00),eth_type(0x86dd),ipv6(src=2001:cafe::93,dst=2001:cafe:: >> > >>> >> > 94,label=0,proto=58,tclass=0,hlimit=255,frag=no),icmpv6(type=136,code=0),nd(target=2001:cafe::93,sll=00:00:00:00:00: >> > >>> 00,tll=f8:bc:12:44:34:b7)']) >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> + >> > >>>> +AT_CHECK([ovs-appctl tnl/arp/show | tail -n+3 | sort], [0], [dnl >> > >>>> +2001:cafe::92 f8:bc:12:44:34:c8 br0 >> > >>>> +]) >> > >>>> + >> > >>>> +dnl Receiving Neighbor Advertisement with incorrect 'nw_dst' should >> > >>>> not alter tunnel neighbor cache >> > >>>> +AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> > >>> >> > 'in_port(1),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:00),eth_type(0x86dd),ipv6(src=2001:cafe::92,dst=2001:cafe:: >> > >>> >> > 99,label=0,proto=58,tclass=0,hlimit=255,frag=no),icmpv6(type=136,code=0),nd(target=2001:cafe::92,sll=00:00:00:00:00: >> > >>> 00,tll=f8:bc:12:44:34:b6)']) >> > >>>> + >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> + >> > >>>> +AT_CHECK([ovs-appctl tnl/arp/show | tail -n+3 | sort], [0], [dnl >> > >>>> +2001:cafe::92 f8:bc:12:44:34:c8 br0 >> > >>>> +]) >> > >>>> + >> > >>>> +dnl Receiving Neighbot Advertisement with incorrect VLAN id should >> > >>>> not alter tunnel neighbor cache >> > >>>> +AT_CHECK([ovs-vsctl set port br0 tag=10]) >> > >>>> +AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> > >>> >> > 'in_port(1),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:00),eth_type(0x8100),vlan(vid=99,pcp=7),encap(eth_type(0x86 >> > >>> >> > dd),ipv6(src=2001:cafe::92,dst=2001:cafe::88,label=0,proto=58,tclass=0,hlimit=255,frag=no),icmpv6(type=136,code=0),n >> > >>> d(target=2001:cafe::92,sll=00:00:00:00:00:00,tll=f8:bc:12:44:34:b6))']) >> > >>>> + >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> + >> > >>>> +AT_CHECK([ovs-appctl tnl/arp/show | tail -n+3 | sort], [0], [dnl >> > >>>> +2001:cafe::92 f8:bc:12:44:34:c8 br0 >> > >>>> +]) >> > >>>> + >> > >>>> +dnl Receiving Neighbor Advertisement with correct VLAN id should >> > >>>> alter tunnel neighbor cache >> > >>>> +AT_CHECK([ovs-vsctl set port br0 tag=10]) >> > >>>> +AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> > >>> >> > 'in_port(1),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:00),eth_type(0x8100),vlan(vid=10,pcp=7),encap(eth_type(0x86 >> > >>> >> > dd),ipv6(src=2001:cafe::92,dst=2001:cafe::88,label=0,proto=58,tclass=0,hlimit=255,frag=no),icmpv6(type=136,code=0),n >> > >>> d(target=2001:cafe::92,sll=00:00:00:00:00:00,tll=f8:bc:12:44:34:b6))']) >> > >>>> + >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> + >> > >>>> +AT_CHECK([ovs-appctl tnl/arp/show | tail -n+3 | sort], [0], [dnl >> > >>>> +2001:cafe::92 f8:bc:12:44:34:b6 br0 >> > >>>> +]) >> > >>>> + >> > >>>> +dnl Receiving Neighbor Advertisement in overlay bridge should not >> > >>>> alter tunnel neighbor cache >> > >>>> +AT_CHECK([ovs-vsctl add-port int-br p1 -- set interface p1 >> > >>>> type=dummy ofport_request=200 other- >> > >>> config:hwaddr=aa:55:aa:55:00:99]) >> > >>>> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 >> > >>> >> > 'in_port(200),eth(src=f8:bc:12:44:34:c8,dst=aa:55:aa:55:00:00),eth_type(0x86dd),ipv6(src=2001:cafe::92,dst=2001:cafe >> > >>> >> > ::99,label=0,proto=58,tclass=0,hlimit=255,frag=no),icmpv6(type=136,code=0),nd(target=2001:cafe::92,sll=00:00:00:00:0 >> > >>> 0:00,tll=f8:bc:12:44:34:c8)']) >> > >>>> + >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> + >> > >>>> +AT_CHECK([ovs-appctl tnl/neigh/show | grep br | sort], [0], [dnl >> > >>>> +2001:cafe::92 f8:bc:12:44:34:b6 br0 >> > >>>> +]) >> > >>>> + >> > >>>> +dnl Receive Neighbor Advertisement without VLAN header >> > >>>> +AT_CHECK([ovs-vsctl set port br0 tag=0]) >> > >>>> +AT_CHECK([ovs-appctl tnl/neigh/flush], [0], [OK >> > >>>> +]) >> > >>>> + >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> + >> > >>>> + >> > >>>> +AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> > >>> >> > 'in_port(1),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:00),eth_type(0x86dd),ipv6(src=2001:cafe::92,dst=2001:cafe:: >> > >>> >> > 88,label=0,proto=58,tclass=0,hlimit=255,frag=no),icmpv6(type=136,code=0),nd(target=2001:cafe::92,sll=00:00:00:00:00: >> > >>> 00,tll=f8:bc:12:44:34:b6)']) >> > >>>> + >> > >>>> +AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> > >>> >> > 'in_port(1),eth(src=f8:bc:12:44:34:b7,dst=aa:55:aa:55:00:00),eth_type(0x86dd),ipv6(src=2001:cafe::93,dst=ff02::1:ff0 >> > >>> >> > 0:0088,label=0,proto=58,tclass=0,hlimit=255,frag=no),icmpv6(type=136,code=0),nd(target=2001:cafe::93,sll=00:00:00:00 >> > >>> :00:00,tll=f8:bc:12:44:34:b7)']) >> > >>>> + >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> >> > >>>> AT_CHECK([ovs-appctl tnl/arp/show | tail -n+3 | sort], [0], [dnl >> > >>>> 2001:cafe::92 f8:bc:12:44:34:b6 br0 >> > >>>> diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at >> > >>>> index b168f5f2f..4e259730c 100644 >> > >>>> --- a/tests/tunnel-push-pop.at >> > >>>> +++ b/tests/tunnel-push-pop.at >> > >>>> @@ -70,9 +70,71 @@ >> > >>>> ffffffffffffaa55aa55000008060001080006040001aa55aa550000010102580000000000000101 >> > >>>> ]) >> > >>>> >> > >>>> dnl Check ARP Snoop >> > >>>> +AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> > >>> >> > 'recirc_id(0),in_port(1),eth(src=f8:bc:12:44:34:c8,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1. >> > >>> 2.88,op=2,sha=f8:bc:12:44:34:c8,tha=00:00:00:00:00:00)']) >> > >>>> + >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> + >> > >>>> +AT_CHECK([ovs-appctl tnl/neigh/show | grep br0 | sort], [0], [dnl >> > >>>> +1.1.2.92 f8:bc:12:44:34:c8 br0 >> > >>>> +]) >> > >>>> + >> > >>>> +dnl Receiving ARP reply with incorrect 'tip' should not alter tunnel >> > >>>> neighbor cache >> > >>>> +AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> > >>> >> > 'recirc_id(0),in_port(1),eth(src=f8:bc:12:44:34:b8,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1. >> > >>> 2.90,op=2,sha=f8:bc:12:44:34:b8,tha=00:00:00:00:00:00)']) >> > >>>> + >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> + >> > >>>> +AT_CHECK([ovs-appctl tnl/neigh/show | grep br0 | sort], [0], [dnl >> > >>>> +1.1.2.92 f8:bc:12:44:34:c8 br0 >> > >>>> +]) >> > >>>> + >> > >>>> +dnl Receiving ARP reply with incorrect VLAN id should not alter >> > >>>> tunnel neighbor cache >> > >>>> +AT_CHECK([ovs-vsctl set port br0 tag=10]) >> > >>>> +AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> > >>> >> > 'recirc_id(0),in_port(1),eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8100),vlan(vid=99,pcp=7),encap( >> > >>> eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00))']) >> > >>>> + >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> + >> > >>>> +AT_CHECK([ovs-appctl tnl/neigh/show | grep br0 | sort], [0], [dnl >> > >>>> +1.1.2.92 f8:bc:12:44:34:c8 br0 >> > >>>> +]) >> > >>>> + >> > >>>> +dnl Receiving ARP reply with correct VLAN id should alter tunnel >> > >>>> neighbor cache >> > >>>> +AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> > >>> >> > 'recirc_id(0),in_port(1),eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x8100),vlan(vid=10,pcp=7),encap( >> > >>> eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00))']) >> > >>>> + >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> + >> > >>>> +AT_CHECK([ovs-appctl tnl/neigh/show | grep br0 | sort], [0], [dnl >> > >>>> +1.1.2.92 f8:bc:12:44:34:b6 br0 >> > >>>> +]) >> > >>>> + >> > >>>> +dnl Receiving ARP reply in overlay bridge should not alter tunnel >> > >>>> neighbor cache >> > >>>> +AT_CHECK([ovs-vsctl add-port int-br p1 -- set interface p1 >> > >>>> type=dummy ofport_request=200 other- >> > >>> config:hwaddr=aa:55:aa:55:00:99]) >> > >>>> +AT_CHECK([ovs-appctl netdev-dummy/receive p1 >> > >>> >> > 'recirc_id(0),in_port(200),eth(src=f8:bc:12:44:34:c8,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1. >> > >>> 1.2.88,op=2,sha=f8:bc:12:44:34:c8,tha=00:00:00:00:00:00)']) >> > >>>> + >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> + >> > >>>> +AT_CHECK([ovs-appctl tnl/neigh/show | grep br | sort], [0], [dnl >> > >>>> +1.1.2.92 f8:bc:12:44:34:b6 br0 >> > >>>> +]) >> > >>>> + >> > >>>> +dnl Receive ARP reply without VLAN header >> > >>>> +AT_CHECK([ovs-vsctl set port br0 tag=0]) >> > >>>> +AT_CHECK([ovs-appctl tnl/neigh/flush], [0], [OK >> > >>>> +]) >> > >>>> + >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> + >> > >>>> AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> > >>> >> > 'recirc_id(0),in_port(1),eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1. >> > >>> 2.88,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)']) >> > >>>> AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> > >>> >> > 'recirc_id(0),in_port(1),eth(src=f8:bc:12:44:34:b7,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.93,tip=1.1. >> > >>> 2.88,op=2,sha=f8:bc:12:44:34:b7,tha=00:00:00:00:00:00)']) >> > >>>> >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> + >> > >>>> AT_CHECK([ovs-appctl tnl/neigh/show | tail -n+3 | sort], [0], [dnl >> > >>>> 1.1.2.92 f8:bc:12:44:34:b6 br0 >> > >>>> 1.1.2.93 f8:bc:12:44:34:b7 br0 >> > >>>> @@ -190,9 +252,12 @@ AT_CHECK([ovs-ofctl dump-ports int-br | grep >> > >>>> 'port 7'], [0], [dnl >> > >>>> dnl Check GREL3 only accepts non-fragmented packets? >> > >>>> AT_CHECK([ovs-appctl netdev-dummy/receive p0 >> > >>> >> > >> 'aa55aa550000001b213cab6408004500007e79464000402fba550101025c0101025820000800000001c8fe71d883724fbeb6f4e1494a0800450 >> > >>> >> > >> 00054ba200000400184861e0000011e00000200004227e75400030af3195500000000f265010000000000101112131415161718191a1b1c1d1e1 >> > >>> f202122232425262728292a2b2c2d2e2f3031323334353637']) >> > >>>> >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> +ovs-appctl time/warp 1000 >> > >>>> + >> > >>>> AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port [[37]]' | sort], >> > >>>> [0], [dnl >> > >>>> port 3: rx pkts=3, bytes=294, drop=?, errs=?, frame=?, over=?, >> > >>>> crc=? >> > >>>> - port 7: rx pkts=3, bytes=252, drop=?, errs=?, frame=?, over=?, >> > >>>> crc=? >> > >>>> + port 7: rx pkts=4, bytes=350, drop=?, errs=?, frame=?, over=?, >> > >>>> crc=? >> > >>>> ]) >> > >>>> >> > >>>> dnl Check decapsulation of Geneve packet with options >> > >>>> -- >> > >>>> 2.14.1 >> > >>>> >> > >>>> _______________________________________________ >> > >>>> dev mailing list >> > >>>> mailto:d...@openvswitch.org >> > >>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
