Thanks for the extra commentary. It sounds like you see some spots that we missed, but I don't understand that yet--can you explain further, or send a patch?
Thanks, Ben. On Tue, Jan 23, 2018 at 12:33:08PM +0800, Huanle Han wrote: > I have done a similar fix few months ago. The commit tries to avoid an > inconsistent result caused by using different xcfg pointers. > https://github.com/openvswitch/ovs/pull/210/commits/ec75e8d9bb0e16690447b40b39d2fd0fb0f7aae2 > > I think a few more place fixes needed for this patch: > update_mcast_snooping_table() > xlate_normal_mcast_send_group() > xlate_normal_mcast_send_mrouters() > xlate_normal_mcast_send_fports() > > > Thank you for the analysis and the fix! I applied this to master and > > backported as far as branch-2.8. > > > > On Fri, Jan 19, 2018 at 08:12:30AM +0000, Lilijun (Jerry) wrote: > >> Hi all, > >> > >> In my test, the new datapath flow which has the same in_port and actions > >> output port was found using ovs-appctl dpctl/dump-flows. > >> Then the mac address will move from one port to another and back it again > >> in the physical switch. This problem result in the VM's traffic become > >> abnormal. > >> > >> My test key steps: > >> 1) There are three VM using ovs bridge and intel 82599 nics as uplink > >> port, deployed in different hosts connecting to the same physical switch. > >> They can be named using VM-A, VM-B and VM-C, Host-A, Host-B, Host-C. > >> 2) VM-A send many unicast packets to VM-B, and VM-B also send unicast > >> packets to VM-A. > >> 3) VM-C ping VM-A continuously, and do ovs port add/delete testing in > >> Host-C ovs bridge. > >> 4) In some abormal scence, the physical switch clear all the mac-entry > >> on each ports. Then Host-C ovs bridge's uplink port will receive two > >> direction packets(VM-A to VM-B, and VM-B to VM-A). > >> > >> The expected result is that this two direction packets should be droppd in > >> the uplink port. Because the dst port of this packets is the uplink port > >> which is also the src port by looking ovs bridge's mac-entry table learned > >> by ovs NORMAL rules. > >> But the truth is some packets being sent back to uplink port and physical > >> switch. And then VM-A's mac was moved to the physical switch port of > >> Host-C from the port of Host-A, as a reulst, VM-C ping VM-A failed at this > >> time. > >> When this problem occurs, the abnormal ovs datapath's flow "in_port(2) > >> actions:2" was found by executing the command "ovs-appctl > >> dpctl/dump-flows". > >> > >> Currently, xlate_normal() uses xbundle pointer compare to verify the > >> packet's dst port whether is same with its input port. This implemention > >> may be wrong while calling xlate_txn_start/xlate_txn_commit in type_run() > >> at the same time, > >> because xcfg/xbridge/xbundle object was reallocated and copied just before > >> we lookup the dst mac_port and mac_xbundle. Then mac_xbundle and > >> in_xbundle are same related with the uplink port but not same object > >> pointer. > >> > >> And we can fix this bug by adding ofbundle check conditions shown in my > >> patch. > >> > >> Signed-off-by: Lilijun <jerry.lilijun at huawei.com> > >> > >> > >> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c > >> index d94e9dc..b7a8b55 100644 > >> --- a/ofproto/ofproto-dpif-xlate.c > >> +++ b/ofproto/ofproto-dpif-xlate.c > >> @@ -2604,7 +2604,9 @@ xlate_normal_mcast_send_rports(struct xlate_ctx *ctx, > >> xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp); > >> LIST_FOR_EACH(rport, node, &ms->rport_list) { > >> mcast_xbundle = xbundle_lookup(xcfg, rport->port); > >> - if (mcast_xbundle && mcast_xbundle != in_xbundle) { > >> + if (mcast_xbundle > >> + && mcast_xbundle != in_xbundle > >> + && mcast_xbundle->ofbundle != in_xbundle->ofbundle) { > >> xlate_report(ctx, OFT_DETAIL, > >> "forwarding report to mcast flagged port"); > >> output_normal(ctx, mcast_xbundle, xvlan); > >> @@ -2626,6 +2628,7 @@ xlate_normal_flood(struct xlate_ctx *ctx, struct > >> xbundle *in_xbundle, > >> LIST_FOR_EACH (xbundle, list_node, &ctx->xbridge->xbundles) { > >> if (xbundle != in_xbundle > >> + && xbundle->ofbundle != in_xbundle->ofbundle > >> && xbundle_includes_vlan(xbundle, xvlan) > >> && xbundle->floodable > >> && !xbundle_mirror_out(ctx->xbridge, xbundle)) { > >> @@ -2833,7 +2836,9 @@ xlate_normal(struct xlate_ctx *ctx) > >> if (mac_port) { > >> struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, > >> &xcfgp); > >> struct xbundle *mac_xbundle = xbundle_lookup(xcfg, mac_port); > >> - if (mac_xbundle && mac_xbundle != in_xbundle) { > >> + if (mac_xbundle > >> + && mac_xbundle != in_xbundle > >> + && mac_xbundle->ofbundle != in_xbundle->ofbundle) { > >> xlate_report(ctx, OFT_DETAIL, "forwarding to learned > >> port"); > >> output_normal(ctx, mac_xbundle, &xvlan); > >> } else if (!mac_xbundle) { > >> > >> > >> _______________________________________________ > >> dev mailing list > >> dev at openvswitch.org > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
