That 'ctx->xcfg' can't be null in my previous patch. This patch looks good to me.
On Thu, Jan 25, 2018 at 3:40 AM, Ben Pfaff <[email protected]> wrote: > From: Huanle Han <[email protected]> > > Some functions, such as xlate_normal_mcast_send_mrouters, test xbundle > pointers equality to avoid sending packet back to in bundle. However, > xbundle pointers port from different xcfgp for same port are inequal. > This may lead to the packet loopback. > > This commit stores xcfgp on ctx at first and always uses the same xcfgp > during one packet process period. > > Signed-off-by: Huanle Han <[email protected]> > Signed-off-by: Ben Pfaff <[email protected]> > --- > ofproto/ofproto-dpif-xlate.c | 43 ++++++++++++++----------------------------- > 1 file changed, 14 insertions(+), 29 deletions(-) > > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c > index 40c04cc4fb4a..f767224941cf 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -182,6 +182,7 @@ struct xlate_ctx { > struct xlate_in *xin; > struct xlate_out *xout; > > + struct xlate_cfg *xcfg; > const struct xbridge *xbridge; > > /* Flow at the last commit. */ > @@ -514,7 +515,6 @@ static struct xlate_cfg *new_xcfg = NULL; > > typedef void xlate_actions_handler(const struct ofpact *, size_t ofpacts_len, > struct xlate_ctx *, bool); > - > static bool may_receive(const struct xport *, struct xlate_ctx *); > static void do_xlate_actions(const struct ofpact *, size_t ofpacts_len, > struct xlate_ctx *, bool); > @@ -1965,8 +1965,7 @@ mirror_packet(struct xlate_ctx *ctx, struct xbundle > *xbundle, > > /* Send the packet to the mirror. */ > if (out) { > - struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp); > - struct xbundle *out_xbundle = xbundle_lookup(xcfg, out); > + struct xbundle *out_xbundle = xbundle_lookup(ctx->xcfg, out); > if (out_xbundle) { > output_normal(ctx, out_xbundle, &xvlan); > } > @@ -2234,7 +2233,6 @@ output_normal(struct xlate_ctx *ctx, const struct > xbundle *out_xbundle, > xport = CONTAINER_OF(ovs_list_front(&out_xbundle->xports), struct > xport, > bundle_node); > } else { > - struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp); > struct flow_wildcards *wc = ctx->wc; > struct ofport_dpif *ofport; > > @@ -2256,7 +2254,7 @@ output_normal(struct xlate_ctx *ctx, const struct > xbundle *out_xbundle, > > ofport = bond_choose_output_slave(out_xbundle->bond, > &ctx->xin->flow, wc, vid); > - xport = xport_lookup(xcfg, ofport); > + xport = xport_lookup(ctx->xcfg, ofport); > > if (!xport) { > /* No slaves enabled, so drop packet. */ > @@ -2525,7 +2523,6 @@ update_mcast_snooping_table(const struct xlate_ctx *ctx, > const struct dp_packet *packet) > { > struct mcast_snooping *ms = ctx->xbridge->ms; > - struct xlate_cfg *xcfg; > struct xbundle *mcast_xbundle; > struct mcast_port_bundle *fport; > > @@ -2537,9 +2534,8 @@ update_mcast_snooping_table(const struct xlate_ctx *ctx, > /* Don't learn from flood ports */ > mcast_xbundle = NULL; > ovs_rwlock_wrlock(&ms->rwlock); > - xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp); > LIST_FOR_EACH(fport, node, &ms->fport_list) { > - mcast_xbundle = xbundle_lookup(xcfg, fport->port); > + mcast_xbundle = xbundle_lookup(ctx->xcfg, fport->port); > if (mcast_xbundle == in_xbundle) { > break; > } > @@ -2566,13 +2562,11 @@ xlate_normal_mcast_send_group(struct xlate_ctx *ctx, > const struct xvlan *xvlan) > OVS_REQ_RDLOCK(ms->rwlock) > { > - struct xlate_cfg *xcfg; > struct mcast_group_bundle *b; > struct xbundle *mcast_xbundle; > > - xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp); > LIST_FOR_EACH(b, bundle_node, &grp->bundle_lru) { > - mcast_xbundle = xbundle_lookup(xcfg, b->port); > + mcast_xbundle = xbundle_lookup(ctx->xcfg, b->port); > if (mcast_xbundle && mcast_xbundle != in_xbundle) { > xlate_report(ctx, OFT_DETAIL, "forwarding to mcast group port"); > output_normal(ctx, mcast_xbundle, xvlan); > @@ -2594,13 +2588,11 @@ xlate_normal_mcast_send_mrouters(struct xlate_ctx > *ctx, > const struct xvlan *xvlan) > OVS_REQ_RDLOCK(ms->rwlock) > { > - struct xlate_cfg *xcfg; > struct mcast_mrouter_bundle *mrouter; > struct xbundle *mcast_xbundle; > > - xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp); > LIST_FOR_EACH(mrouter, mrouter_node, &ms->mrouter_lru) { > - mcast_xbundle = xbundle_lookup(xcfg, mrouter->port); > + mcast_xbundle = xbundle_lookup(ctx->xcfg, mrouter->port); > if (mcast_xbundle && mcast_xbundle != in_xbundle > && mrouter->vlan == xvlan->v[0].vid) { > xlate_report(ctx, OFT_DETAIL, "forwarding to mcast router port"); > @@ -2626,13 +2618,11 @@ xlate_normal_mcast_send_fports(struct xlate_ctx *ctx, > const struct xvlan *xvlan) > OVS_REQ_RDLOCK(ms->rwlock) > { > - struct xlate_cfg *xcfg; > struct mcast_port_bundle *fport; > struct xbundle *mcast_xbundle; > > - xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp); > LIST_FOR_EACH(fport, node, &ms->fport_list) { > - mcast_xbundle = xbundle_lookup(xcfg, fport->port); > + mcast_xbundle = xbundle_lookup(ctx->xcfg, fport->port); > if (mcast_xbundle && mcast_xbundle != in_xbundle) { > xlate_report(ctx, OFT_DETAIL, "forwarding to mcast flood port"); > output_normal(ctx, mcast_xbundle, xvlan); > @@ -2654,13 +2644,11 @@ xlate_normal_mcast_send_rports(struct xlate_ctx *ctx, > const struct xvlan *xvlan) > OVS_REQ_RDLOCK(ms->rwlock) > { > - struct xlate_cfg *xcfg; > struct mcast_port_bundle *rport; > struct xbundle *mcast_xbundle; > > - xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp); > LIST_FOR_EACH(rport, node, &ms->rport_list) { > - mcast_xbundle = xbundle_lookup(xcfg, rport->port); > + mcast_xbundle = xbundle_lookup(ctx->xcfg, rport->port); > if (mcast_xbundle > && mcast_xbundle != in_xbundle > && mcast_xbundle->ofbundle != in_xbundle->ofbundle) { > @@ -2891,8 +2879,7 @@ xlate_normal(struct xlate_ctx *ctx) > ovs_rwlock_unlock(&ctx->xbridge->ml->rwlock); > > if (mac_port) { > - struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp); > - struct xbundle *mac_xbundle = xbundle_lookup(xcfg, mac_port); > + struct xbundle *mac_xbundle = xbundle_lookup(ctx->xcfg, > mac_port); > if (mac_xbundle > && mac_xbundle != in_xbundle > && mac_xbundle->ofbundle != in_xbundle->ofbundle) { > @@ -3139,13 +3126,13 @@ process_special(struct xlate_ctx *ctx, const struct > xport *xport) > } > > static int > -tnl_route_lookup_flow(const struct flow *oflow, > +tnl_route_lookup_flow(const struct xlate_ctx *ctx, > + const struct flow *oflow, > struct in6_addr *ip, struct in6_addr *src, > struct xport **out_port) > { > char out_dev[IFNAMSIZ]; > struct xbridge *xbridge; > - struct xlate_cfg *xcfg; > struct in6_addr gw; > struct in6_addr dst; > > @@ -3161,10 +3148,7 @@ tnl_route_lookup_flow(const struct flow *oflow, > *ip = dst; > } > > - xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp); > - ovs_assert(xcfg); > - > - HMAP_FOR_EACH (xbridge, hmap_node, &xcfg->xbridges) { > + HMAP_FOR_EACH (xbridge, hmap_node, &ctx->xcfg->xbridges) { > if (!strncmp(xbridge->name, out_dev, IFNAMSIZ)) { > struct xport *port; > > @@ -3333,7 +3317,7 @@ native_tunnel_output(struct xlate_ctx *ctx, const > struct xport *xport, > memcpy(&old_base_flow, &ctx->base_flow, sizeof old_base_flow); > memcpy(&old_flow, &ctx->xin->flow, sizeof old_flow); > > - err = tnl_route_lookup_flow(flow, &d_ip6, &s_ip6, &out_dev); > + err = tnl_route_lookup_flow(ctx, flow, &d_ip6, &s_ip6, &out_dev); > if (err) { > xlate_report(ctx, OFT_WARN, "native tunnel routing failed"); > return err; > @@ -6841,6 +6825,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out > *xout) > .xout = xout, > .base_flow = *flow, > .orig_tunnel_ipv6_dst = flow_tnl_dst(&flow->tunnel), > + .xcfg = xcfg, > .xbridge = xbridge, > .stack = OFPBUF_STUB_INITIALIZER(stack_stub), > .rule = xin->rule, > -- > 2.10.2 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
