Thanks, I applied this to master and branch-2.9.
Would you mind reviewing or testing the rest of the series, especially
patches 4 and 5? Thanks!
On Tue, Jan 30, 2018 at 11:28:27PM +0800, Huanle Han wrote:
> 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 <b...@ovn.org> wrote:
> > From: Huanle Han <hanxue...@gmail.com>
> >
> > 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 <hanxue...@gmail.com>
> > Signed-off-by: Ben Pfaff <b...@ovn.org>
> > ---
> > 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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
