| Message ID | fd45bf686b31e99097b639e0aa4b797b98548989.1710516075.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] ovn-ic: Avoid igmp/mld traffic flooding. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
On 3/15/24 16:23, Lorenzo Bianconi wrote: > Avoid recirculating IGMP/MLD packets more than one time from stage > ls_out_pre_lb in the egress pipeline to ovn table 37 in order to avoid > packet looping for ovn-ic deployment. > > Acked-by: Mohammad Heib <mheib@redhat.com> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- Thanks, Lorenzo and Mohammad! Applied to main and backported down to 23.06. Regards, Dumitru
On 3/15/24 16:23, Lorenzo Bianconi wrote: > Avoid recirculating IGMP/MLD packets more than one time from stage > ls_out_pre_lb in the egress pipeline to ovn table 37 in order to avoid > packet looping for ovn-ic deployment. > > Acked-by: Mohammad Heib <mheib@redhat.com> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > controller/pinctrl.c | 2 ++ > include/ovn/logical-fields.h | 3 +++ > lib/logical-fields.c | 4 ++++ > northd/northd.c | 11 +++++++---- > 4 files changed, 16 insertions(+), 4 deletions(-) > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > index 98b29de9f..ba4775e20 100644 > --- a/controller/pinctrl.c > +++ b/controller/pinctrl.c > @@ -660,6 +660,8 @@ pinctrl_forward_pkt(struct rconn *swconn, int64_t dp_key, > put_load(dp_key, MFF_LOG_DATAPATH, 0, 64, &ofpacts); > put_load(in_port_key, MFF_LOG_INPORT, 0, 32, &ofpacts); > put_load(out_port_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts); > + /* Avoid re-injecting packet already consumed. */ > + put_load(1, MFF_LOG_FLAGS, MLF_IGMP_IGMP_SPOOF_INJECT_BIT, 1, &ofpacts); > > struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(&ofpacts); > resubmit->in_port = OFPP_CONTROLLER; > diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h > index ce79b501c..84c287e0a 100644 > --- a/include/ovn/logical-fields.h > +++ b/include/ovn/logical-fields.h > @@ -82,6 +82,7 @@ enum mff_log_flags_bits { > MLF_LOCALNET_BIT = 15, > MLF_RX_FROM_TUNNEL_BIT = 16, > MLF_ICMP_SNAT_BIT = 17, > + MLF_IGMP_IGMP_SPOOF_INJECT_BIT = 18, SPOOF? I changed this to MLF_IGMP_IGMP_SNOOP_INJECT_BIT and pushed the patch to main and backported it to stable branches. Thanks, Lorenzo and Mohammad! Regards, Dumitru > }; > > /* MFF_LOG_FLAGS_REG flag assignments */ > @@ -137,6 +138,8 @@ enum mff_log_flags { > MLF_RX_FROM_TUNNEL = (1 << MLF_RX_FROM_TUNNEL_BIT), > > MLF_ICMP_SNAT = (1 << MLF_ICMP_SNAT_BIT), > + > + MLF_IGMP_IGMP_SPOOF = (1 << MLF_IGMP_IGMP_SPOOF_INJECT_BIT), > }; > > /* OVN logical fields > diff --git a/lib/logical-fields.c b/lib/logical-fields.c > index 20219a67a..7e60c7c86 100644 > --- a/lib/logical-fields.c > +++ b/lib/logical-fields.c > @@ -139,6 +139,10 @@ ovn_init_symtab(struct shash *symtab) > flags_str); > snprintf(flags_str, sizeof flags_str, "flags[%d]", MLF_RX_FROM_TUNNEL_BIT); > expr_symtab_add_subfield(symtab, "flags.tunnel_rx", NULL, flags_str); > + snprintf(flags_str, sizeof flags_str, "flags[%d]", > + MLF_IGMP_IGMP_SPOOF_INJECT_BIT); > + expr_symtab_add_subfield(symtab, "flags.igmp_loopback", NULL, > + flags_str); > > /* Connection tracking state. */ > expr_symtab_add_field_scoped(symtab, "ct_mark", MFF_CT_MARK, NULL, false, > diff --git a/northd/northd.c b/northd/northd.c > index 1839b7d8b..2c860082d 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -6089,7 +6089,8 @@ build_interconn_mcast_snoop_flows(struct ovn_datapath *od, > continue; > } > /* Punt IGMP traffic to controller. */ > - char *match = xasprintf("inport == %s && igmp", op->json_key); > + char *match = xasprintf("inport == %s && igmp && " > + "flags.igmp_loopback == 0", op->json_key); > ovn_lflow_metered(lflows, od, S_SWITCH_OUT_PRE_LB, 120, match, > "clone { igmp; }; next;", > copp_meter_get(COPP_IGMP, od->nbs->copp, > @@ -6098,7 +6099,8 @@ build_interconn_mcast_snoop_flows(struct ovn_datapath *od, > free(match); > > /* Punt MLD traffic to controller. */ > - match = xasprintf("inport == %s && (mldv1 || mldv2)", op->json_key); > + match = xasprintf("inport == %s && (mldv1 || mldv2) && " > + "flags.igmp_loopback == 0", op->json_key); > ovn_lflow_metered(lflows, od, S_SWITCH_OUT_PRE_LB, 120, match, > "clone { igmp; }; next;", > copp_meter_get(COPP_IGMP, od->nbs->copp, > @@ -9285,14 +9287,15 @@ build_lswitch_destination_lookup_bmcast(struct ovn_datapath *od, > ds_put_cstr(actions, "igmp;"); > /* Punt IGMP traffic to controller. */ > ovn_lflow_metered(lflows, od, S_SWITCH_IN_L2_LKUP, 100, > - "igmp", ds_cstr(actions), > + "flags.igmp_loopback == 0 && igmp", ds_cstr(actions), > copp_meter_get(COPP_IGMP, od->nbs->copp, > meter_groups), > lflow_ref); > > /* Punt MLD traffic to controller. */ > ovn_lflow_metered(lflows, od, S_SWITCH_IN_L2_LKUP, 100, > - "mldv1 || mldv2", ds_cstr(actions), > + "flags.igmp_loopback == 0 && (mldv1 || mldv2)", > + ds_cstr(actions), > copp_meter_get(COPP_IGMP, od->nbs->copp, > meter_groups), > lflow_ref);
diff --git a/controller/pinctrl.c b/controller/pinctrl.c index 98b29de9f..ba4775e20 100644 --- a/controller/pinctrl.c +++ b/controller/pinctrl.c @@ -660,6 +660,8 @@ pinctrl_forward_pkt(struct rconn *swconn, int64_t dp_key, put_load(dp_key, MFF_LOG_DATAPATH, 0, 64, &ofpacts); put_load(in_port_key, MFF_LOG_INPORT, 0, 32, &ofpacts); put_load(out_port_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts); + /* Avoid re-injecting packet already consumed. */ + put_load(1, MFF_LOG_FLAGS, MLF_IGMP_IGMP_SPOOF_INJECT_BIT, 1, &ofpacts); struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(&ofpacts); resubmit->in_port = OFPP_CONTROLLER; diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h index ce79b501c..84c287e0a 100644 --- a/include/ovn/logical-fields.h +++ b/include/ovn/logical-fields.h @@ -82,6 +82,7 @@ enum mff_log_flags_bits { MLF_LOCALNET_BIT = 15, MLF_RX_FROM_TUNNEL_BIT = 16, MLF_ICMP_SNAT_BIT = 17, + MLF_IGMP_IGMP_SPOOF_INJECT_BIT = 18, }; /* MFF_LOG_FLAGS_REG flag assignments */ @@ -137,6 +138,8 @@ enum mff_log_flags { MLF_RX_FROM_TUNNEL = (1 << MLF_RX_FROM_TUNNEL_BIT), MLF_ICMP_SNAT = (1 << MLF_ICMP_SNAT_BIT), + + MLF_IGMP_IGMP_SPOOF = (1 << MLF_IGMP_IGMP_SPOOF_INJECT_BIT), }; /* OVN logical fields diff --git a/lib/logical-fields.c b/lib/logical-fields.c index 20219a67a..7e60c7c86 100644 --- a/lib/logical-fields.c +++ b/lib/logical-fields.c @@ -139,6 +139,10 @@ ovn_init_symtab(struct shash *symtab) flags_str); snprintf(flags_str, sizeof flags_str, "flags[%d]", MLF_RX_FROM_TUNNEL_BIT); expr_symtab_add_subfield(symtab, "flags.tunnel_rx", NULL, flags_str); + snprintf(flags_str, sizeof flags_str, "flags[%d]", + MLF_IGMP_IGMP_SPOOF_INJECT_BIT); + expr_symtab_add_subfield(symtab, "flags.igmp_loopback", NULL, + flags_str); /* Connection tracking state. */ expr_symtab_add_field_scoped(symtab, "ct_mark", MFF_CT_MARK, NULL, false, diff --git a/northd/northd.c b/northd/northd.c index 1839b7d8b..2c860082d 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -6089,7 +6089,8 @@ build_interconn_mcast_snoop_flows(struct ovn_datapath *od, continue; } /* Punt IGMP traffic to controller. */ - char *match = xasprintf("inport == %s && igmp", op->json_key); + char *match = xasprintf("inport == %s && igmp && " + "flags.igmp_loopback == 0", op->json_key); ovn_lflow_metered(lflows, od, S_SWITCH_OUT_PRE_LB, 120, match, "clone { igmp; }; next;", copp_meter_get(COPP_IGMP, od->nbs->copp, @@ -6098,7 +6099,8 @@ build_interconn_mcast_snoop_flows(struct ovn_datapath *od, free(match); /* Punt MLD traffic to controller. */ - match = xasprintf("inport == %s && (mldv1 || mldv2)", op->json_key); + match = xasprintf("inport == %s && (mldv1 || mldv2) && " + "flags.igmp_loopback == 0", op->json_key); ovn_lflow_metered(lflows, od, S_SWITCH_OUT_PRE_LB, 120, match, "clone { igmp; }; next;", copp_meter_get(COPP_IGMP, od->nbs->copp, @@ -9285,14 +9287,15 @@ build_lswitch_destination_lookup_bmcast(struct ovn_datapath *od, ds_put_cstr(actions, "igmp;"); /* Punt IGMP traffic to controller. */ ovn_lflow_metered(lflows, od, S_SWITCH_IN_L2_LKUP, 100, - "igmp", ds_cstr(actions), + "flags.igmp_loopback == 0 && igmp", ds_cstr(actions), copp_meter_get(COPP_IGMP, od->nbs->copp, meter_groups), lflow_ref); /* Punt MLD traffic to controller. */ ovn_lflow_metered(lflows, od, S_SWITCH_IN_L2_LKUP, 100, - "mldv1 || mldv2", ds_cstr(actions), + "flags.igmp_loopback == 0 && (mldv1 || mldv2)", + ds_cstr(actions), copp_meter_get(COPP_IGMP, od->nbs->copp, meter_groups), lflow_ref);