| Message ID | 1441996587-615-1-git-send-email-blp@nicira.com |
|---|---|
| State | Accepted |
| Headers | show |
> On Sep 11, 2015, at 11:36 AM, Ben Pfaff <blp@nicira.com> wrote: > > Until now, the priority-100 flow for broadcast and multicast packets caused > such packets to be delivered to disabled logical ports. This commit makes > ovn-northd add a priority-150 flow for each disabled logical port to > override that behavior. > > Found by inspection. > > Signed-off-by: Ben Pfaff <blp@nicira.com> > --- > ovn/northd/ovn-northd.c | 22 ++++++++++++++-------- > 1 file changed, 14 insertions(+), 8 deletions(-) > > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > index 253ee59..a6572df 100644 > --- a/ovn/northd/ovn-northd.c > +++ b/ovn/northd/ovn-northd.c > @@ -863,20 +863,26 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, > "output;"); > } > > - /* Egress table 1: Egress port security (priority 50). */ > + /* Egress table 1: Egress port security (priority 50). > + * > + * Also, priority 150 rules for disabled logical ports so that they don't > + * even receive multicast or broadcast packets. */ It seems like it might be clearer in the title to indicate both priorities being set. Then, in the comment describe both types of flows. Not a big deal, though. Acked-by: Justin Pettit <jpettit@nicira.com> --Justin
On Fri, Sep 11, 2015 at 12:49:13PM -0700, Justin Pettit wrote: > > > On Sep 11, 2015, at 11:36 AM, Ben Pfaff <blp@nicira.com> wrote: > > > > Until now, the priority-100 flow for broadcast and multicast packets caused > > such packets to be delivered to disabled logical ports. This commit makes > > ovn-northd add a priority-150 flow for each disabled logical port to > > override that behavior. > > > > Found by inspection. > > > > Signed-off-by: Ben Pfaff <blp@nicira.com> > > --- > > ovn/northd/ovn-northd.c | 22 ++++++++++++++-------- > > 1 file changed, 14 insertions(+), 8 deletions(-) > > > > diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c > > index 253ee59..a6572df 100644 > > --- a/ovn/northd/ovn-northd.c > > +++ b/ovn/northd/ovn-northd.c > > @@ -863,20 +863,26 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, > > "output;"); > > } > > > > - /* Egress table 1: Egress port security (priority 50). */ > > + /* Egress table 1: Egress port security (priority 50). > > + * > > + * Also, priority 150 rules for disabled logical ports so that they don't > > + * even receive multicast or broadcast packets. */ > > It seems like it might be clearer in the title to indicate both > priorities being set. Then, in the comment describe both types of > flows. Not a big deal, though. OK, I updated the comment to: /* Egress table 1: Egress port security (priorities 50 and 150). * * Priority 50 rules implement port security for enabled logical port. * * Priority 150 rules drop packets to disabled logical ports, so that they * don't even receive multicast or broadcast packets. */ > Acked-by: Justin Pettit <jpettit@nicira.com> Thanks, I'll apply this in a minute.
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index 253ee59..a6572df 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -863,20 +863,26 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths, "output;"); } - /* Egress table 1: Egress port security (priority 50). */ + /* Egress table 1: Egress port security (priority 50). + * + * Also, priority 150 rules for disabled logical ports so that they don't + * even receive multicast or broadcast packets. */ HMAP_FOR_EACH (op, key_node, ports) { struct ds match; ds_init(&match); ds_put_cstr(&match, "outport == "); json_string_escape(op->key, &match); - build_port_security("eth.dst", - op->nb->port_security, op->nb->n_port_security, - &match); - - ovn_lflow_add(&lflows, op->od, P_OUT, S_OUT_PORT_SEC, 50, - ds_cstr(&match), - lport_is_enabled(op->nb) ? "output;" : "drop;"); + if (lport_is_enabled(op->nb)) { + build_port_security("eth.dst", + op->nb->port_security, op->nb->n_port_security, + &match); + ovn_lflow_add(&lflows, op->od, P_OUT, S_OUT_PORT_SEC, 50, + ds_cstr(&match), "output;"); + } else { + ovn_lflow_add(&lflows, op->od, P_OUT, S_OUT_PORT_SEC, 150, + ds_cstr(&match), "drop;"); + } ds_destroy(&match); }
Until now, the priority-100 flow for broadcast and multicast packets caused such packets to be delivered to disabled logical ports. This commit makes ovn-northd add a priority-150 flow for each disabled logical port to override that behavior. Found by inspection. Signed-off-by: Ben Pfaff <blp@nicira.com> --- ovn/northd/ovn-northd.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-)