subject:"\[ovs\-dev\] \[PATCH ovn 3\/5\] ovn\-northd\: Add CoPP policies for flows that punt packets to ovn\-controller."

Re: [ovs-dev] [PATCH ovn 3/5] ovn-northd: Add CoPP policies for flows that punt packets to ovn-controller.

2021-05-24 Thread Lorenzo Bianconi

> On 29/04/2021 18:04, Lorenzo Bianconi wrote:
> > From: Dumitru Ceara 
> > 
> > Change the ovn-northd implementation to set the new 'controller_meter'
> > field for flows that need to punt packets to ovn-controller.
> > 
> > Protocol packets for which CoPP is enforced when sending packets to
> > ovn-controller (if configured):
> > - ARP
> > - ND_NS
> > - ND_NA
> > - ND_RA
> > - DNS
> > - IGMP
> > - packets that require ARP resolution before forwarding
> > - packets that require ND_NS before forwarding
> > - packets that need to be replied to with ICMP Errors
> > - packets that need to be replied to with TCP RST
> > - packets that need to be replied to with DHCP_OPTS
> > - BFD
> > 
> Add reject?
> 
> Do you need to add event-elb?

ack, I will fix it in v2

> 
> > Co-authored-by: Lorenzo Bianconi 
> > Signed-off-by: Lorenzo Bianconi 
> > Signed-off-by: Dumitru Ceara 
> > ---
> >  lib/copp.c|   1 +
> >  lib/copp.h|   1 +
> >  northd/ovn-northd.c   | 451 --
> >  ovn-nb.xml|   3 +
> >  tests/atlocal.in  |   3 +
> >  tests/system-ovn.at   | 119 ++
> >  utilities/ovn-nbctl.8.xml |   1 +
> >  7 files changed, 417 insertions(+), 162 deletions(-)
> > 
> > diff --git a/lib/copp.c b/lib/copp.c
> > index ac53a1094..7713046e5 100644
> > --- a/lib/copp.c
> > +++ b/lib/copp.c
> > @@ -37,6 +37,7 @@ static char *copp_proto_names[COPP_PROTO_MAX] = {
> >  [COPP_ND_NS_RESOLVE] = "nd-ns-resolve",
> >  [COPP_ND_RA_OPTS]= "nd-ra-opts",
> >  [COPP_TCP_RESET] = "tcp-reset",
> > +[COPP_REJECT]= "reject",
> >  [COPP_BFD]   = "bfd",
> >  };
> >  
[...]
> 
> Why do you add the same flow twice but one with a meter instead of
> modifying the flow above?

it is a bug :) thx for spotting it. I will fix in v2

> 
> >  const char *dns_action = "eth.dst <-> eth.src; ip4.src <-> 
> > ip4.dst; "
> >"udp.dst = udp.src; udp.src = 53; outport = inport; "
> >"flags.loopback = 1; output;";
> > @@ -7244,7 +7280,8 @@ build_lswitch_external_port(struct ovn_port *op,
> >  static void
> >  build_lswitch_destination_lookup_bmcast(struct ovn_datapath *od,
> >  struct hmap *lflows,
> > -struct ds *actions)
> > +struct ds *actions,
> > +struct shash *meter_groups)
> >  {
> >  if (od->nbs) {
> >  
> > @@ -7265,12 +7302,16 @@ build_lswitch_destination_lookup_bmcast(struct 
> > ovn_datapath *od,
> >  }
> >  ds_put_cstr(actions, "igmp;");
> >  /* Punt IGMP traffic to controller. */
> > -ovn_lflow_add_unique(lflows, od, S_SWITCH_IN_L2_LKUP, 100,
> > - "ip4 && ip.proto == 2", ds_cstr(actions));
> > +ovn_lflow_add_unique__(lflows, od, S_SWITCH_IN_L2_LKUP, 100,
> > +   "ip4 && ip.proto == 2", 
> > ds_cstr(actions),
> > +   copp_meter_get(COPP_IGMP, od->nbs->copp,
> > +  meter_groups));
> >  
> >  /* Punt MLD traffic to controller. */
> > -ovn_lflow_add_unique(lflows, od, S_SWITCH_IN_L2_LKUP, 100,
> > - "mldv1 || mldv2", ds_cstr(actions));
> > +ovn_lflow_add_unique__(lflows, od, S_SWITCH_IN_L2_LKUP, 100,
> > +   "mldv1 || mldv2", ds_cstr(actions),
> > +   copp_meter_get(COPP_IGMP, od->nbs->copp,
> > +  meter_groups));
> >  
> >  /* Flood all IP multicast traffic destined to 224.0.0.X to all
> >   * ports - RFC 4541, section 2.1.2, item 2.
> > @@ -8650,23 +8691,29 @@ add_router_lb_flow(struct hmap *lflows, struct 
> > ovn_datapath *od,
> > struct ds *match, struct ds *actions, int priority,
> > enum lb_snat_type snat_type, struct ovn_lb_vip *lb_vip,
> > const char *proto, struct nbrec_load_balancer *lb,
> > -   struct shash *meter_groups, struct sset *nat_entries)
> > +   struct shash *meter_groups, struct sset *nat_entries,
> > +   bool reject)
> >  {
> >  build_empty_lb_event_flow(od, lflows, lb_vip, lb, S_ROUTER_IN_DNAT,
> >meter_groups);
> >  
> > +const char *meter = NULL;
> > +if (reject) {
> > +meter = copp_meter_get(COPP_REJECT, od->nbr->copp, meter_groups);
> > +}
> >  /* A match and actions for new connections. */
> >  char *new_match = xasprintf("ct.new && %s", ds_cstr(match));
> >  if (snat_type == FORCE_SNAT || snat_type == SKIP_SNAT) {
> >  char *new_actions = xasprintf("flags.%s_snat_for_lb = 1; %s",
> >

Re: [ovs-dev] [PATCH ovn 3/5] ovn-northd: Add CoPP policies for flows that punt packets to ovn-controller.

2021-05-21 Thread Mark Gray

On 29/04/2021 18:04, Lorenzo Bianconi wrote:
> From: Dumitru Ceara 
> 
> Change the ovn-northd implementation to set the new 'controller_meter'
> field for flows that need to punt packets to ovn-controller.
> 
> Protocol packets for which CoPP is enforced when sending packets to
> ovn-controller (if configured):
> - ARP
> - ND_NS
> - ND_NA
> - ND_RA
> - DNS
> - IGMP
> - packets that require ARP resolution before forwarding
> - packets that require ND_NS before forwarding
> - packets that need to be replied to with ICMP Errors
> - packets that need to be replied to with TCP RST
> - packets that need to be replied to with DHCP_OPTS
> - BFD
> 
Add reject?

Do you need to add event-elb?

> Co-authored-by: Lorenzo Bianconi 
> Signed-off-by: Lorenzo Bianconi 
> Signed-off-by: Dumitru Ceara 
> ---
>  lib/copp.c|   1 +
>  lib/copp.h|   1 +
>  northd/ovn-northd.c   | 451 --
>  ovn-nb.xml|   3 +
>  tests/atlocal.in  |   3 +
>  tests/system-ovn.at   | 119 ++
>  utilities/ovn-nbctl.8.xml |   1 +
>  7 files changed, 417 insertions(+), 162 deletions(-)
> 
> diff --git a/lib/copp.c b/lib/copp.c
> index ac53a1094..7713046e5 100644
> --- a/lib/copp.c
> +++ b/lib/copp.c
> @@ -37,6 +37,7 @@ static char *copp_proto_names[COPP_PROTO_MAX] = {
>  [COPP_ND_NS_RESOLVE] = "nd-ns-resolve",
>  [COPP_ND_RA_OPTS]= "nd-ra-opts",
>  [COPP_TCP_RESET] = "tcp-reset",
> +[COPP_REJECT]= "reject",
>  [COPP_BFD]   = "bfd",
>  };
>  
> diff --git a/lib/copp.h b/lib/copp.h
> index 82581e7e4..826fe987e 100644
> --- a/lib/copp.h
> +++ b/lib/copp.h
> @@ -36,6 +36,7 @@ enum copp_proto {
>  COPP_ND_RA_OPTS,
>  COPP_TCP_RESET,
>  COPP_BFD,
> +COPP_REJECT,

If it is easy to do, you should merge the reject meter addition into the
previous patch as I think that is where it actually belongs.

>  COPP_PROTO_MAX,
>  COPP_PROTO_INVALID = COPP_PROTO_MAX,
>  };
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 2e9c7de22..93b431f4c 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -3328,11 +3328,11 @@ ovn_lb_svc_create(struct northd_context *ctx, struct 
> ovn_northd_lb *lb,
>  }
>  }
>  
> -static
> -void build_lb_vip_actions(struct ovn_lb_vip *lb_vip,
> -  struct ovn_northd_lb_vip *lb_vip_nb,
> -  struct ds *action, char *selection_fields,
> -  bool ls_dp)
> +static bool
> +build_lb_vip_actions(struct ovn_lb_vip *lb_vip,
> + struct ovn_northd_lb_vip *lb_vip_nb,
> + struct ds *action, char *selection_fields,
> + bool ls_dp)
>  {
>  bool skip_hash_fields = false, reject = false;
>  
> @@ -3384,6 +3384,7 @@ void build_lb_vip_actions(struct ovn_lb_vip *lb_vip,
>  ds_chomp(action, ')');
>  ds_put_format(action, "; hash_fields=\"%s\");", selection_fields);
>  }
> +return reject;
>  }
>  
>  static void
> @@ -4174,9 +4175,14 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct 
> ovn_datapath *od,
>  ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, false, \
>   NULL, STAGE_HINT, OVS_SOURCE_LOCATOR)
>  
> -#define ovn_lflow_add_unique(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS) 
> \
> +#define ovn_lflow_add_unique__(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, \
> +   ACTIONS, CTRL_METER) \
>  ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, false, \
> - NULL, NULL, OVS_SOURCE_LOCATOR)
> + CTRL_METER, NULL, OVS_SOURCE_LOCATOR)
> +
> +#define ovn_lflow_add_unique(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS) 
> \
> +ovn_lflow_add_unique__(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, \
> +   NULL)
>  
>  static struct ovn_lflow *
>  ovn_lflow_find(struct hmap *lflows, struct ovn_datapath *od,
> @@ -5429,9 +5435,12 @@ build_reject_acl_rules(struct ovn_datapath *od, struct 
> hmap *lflows,
>"reject { "
>"/* eth.dst <-> eth.src; ip.dst <-> ip.src; is implicit. 
> */ "
>"outport <-> inport; %s };", next_action);
> -ovn_lflow_add_with_hint(lflows, od, stage,
> -acl->priority + OVN_ACL_PRI_OFFSET,
> -ds_cstr(&match), ds_cstr(&actions), stage_hint);
> +ovn_lflow_add_with_hint__(lflows, od, stage,
> +  acl->priority + OVN_ACL_PRI_OFFSET,
> +  ds_cstr(&match), ds_cstr(&actions),
> +  copp_meter_get(COPP_REJECT, od->nbs->copp,
> + meter_groups),
> +  stage_hint);
>  
>  free(next_action);
>  ds_destroy(&match);
> @@ -5938,7 +5947,7 @@ build_lb(struct ovn_datapath *od, struct hm

[ovs-dev] [PATCH ovn 3/5] ovn-northd: Add CoPP policies for flows that punt packets to ovn-controller.

2021-04-29 Thread Lorenzo Bianconi

From: Dumitru Ceara 

Change the ovn-northd implementation to set the new 'controller_meter'
field for flows that need to punt packets to ovn-controller.

Protocol packets for which CoPP is enforced when sending packets to
ovn-controller (if configured):
- ARP
- ND_NS
- ND_NA
- ND_RA
- DNS
- IGMP
- packets that require ARP resolution before forwarding
- packets that require ND_NS before forwarding
- packets that need to be replied to with ICMP Errors
- packets that need to be replied to with TCP RST
- packets that need to be replied to with DHCP_OPTS
- BFD

Co-authored-by: Lorenzo Bianconi 
Signed-off-by: Lorenzo Bianconi 
Signed-off-by: Dumitru Ceara 
---
 lib/copp.c|   1 +
 lib/copp.h|   1 +
 northd/ovn-northd.c   | 451 --
 ovn-nb.xml|   3 +
 tests/atlocal.in  |   3 +
 tests/system-ovn.at   | 119 ++
 utilities/ovn-nbctl.8.xml |   1 +
 7 files changed, 417 insertions(+), 162 deletions(-)

diff --git a/lib/copp.c b/lib/copp.c
index ac53a1094..7713046e5 100644
--- a/lib/copp.c
+++ b/lib/copp.c
@@ -37,6 +37,7 @@ static char *copp_proto_names[COPP_PROTO_MAX] = {
 [COPP_ND_NS_RESOLVE] = "nd-ns-resolve",
 [COPP_ND_RA_OPTS]= "nd-ra-opts",
 [COPP_TCP_RESET] = "tcp-reset",
+[COPP_REJECT]= "reject",
 [COPP_BFD]   = "bfd",
 };
 
diff --git a/lib/copp.h b/lib/copp.h
index 82581e7e4..826fe987e 100644
--- a/lib/copp.h
+++ b/lib/copp.h
@@ -36,6 +36,7 @@ enum copp_proto {
 COPP_ND_RA_OPTS,
 COPP_TCP_RESET,
 COPP_BFD,
+COPP_REJECT,
 COPP_PROTO_MAX,
 COPP_PROTO_INVALID = COPP_PROTO_MAX,
 };
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 2e9c7de22..93b431f4c 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -3328,11 +3328,11 @@ ovn_lb_svc_create(struct northd_context *ctx, struct 
ovn_northd_lb *lb,
 }
 }
 
-static
-void build_lb_vip_actions(struct ovn_lb_vip *lb_vip,
-  struct ovn_northd_lb_vip *lb_vip_nb,
-  struct ds *action, char *selection_fields,
-  bool ls_dp)
+static bool
+build_lb_vip_actions(struct ovn_lb_vip *lb_vip,
+ struct ovn_northd_lb_vip *lb_vip_nb,
+ struct ds *action, char *selection_fields,
+ bool ls_dp)
 {
 bool skip_hash_fields = false, reject = false;
 
@@ -3384,6 +3384,7 @@ void build_lb_vip_actions(struct ovn_lb_vip *lb_vip,
 ds_chomp(action, ')');
 ds_put_format(action, "; hash_fields=\"%s\");", selection_fields);
 }
+return reject;
 }
 
 static void
@@ -4174,9 +4175,14 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct 
ovn_datapath *od,
 ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, false, \
  NULL, STAGE_HINT, OVS_SOURCE_LOCATOR)
 
-#define ovn_lflow_add_unique(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS) \
+#define ovn_lflow_add_unique__(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, \
+   ACTIONS, CTRL_METER) \
 ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, false, \
- NULL, NULL, OVS_SOURCE_LOCATOR)
+ CTRL_METER, NULL, OVS_SOURCE_LOCATOR)
+
+#define ovn_lflow_add_unique(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS) \
+ovn_lflow_add_unique__(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, \
+   NULL)
 
 static struct ovn_lflow *
 ovn_lflow_find(struct hmap *lflows, struct ovn_datapath *od,
@@ -5429,9 +5435,12 @@ build_reject_acl_rules(struct ovn_datapath *od, struct 
hmap *lflows,
   "reject { "
   "/* eth.dst <-> eth.src; ip.dst <-> ip.src; is implicit. */ "
   "outport <-> inport; %s };", next_action);
-ovn_lflow_add_with_hint(lflows, od, stage,
-acl->priority + OVN_ACL_PRI_OFFSET,
-ds_cstr(&match), ds_cstr(&actions), stage_hint);
+ovn_lflow_add_with_hint__(lflows, od, stage,
+  acl->priority + OVN_ACL_PRI_OFFSET,
+  ds_cstr(&match), ds_cstr(&actions),
+  copp_meter_get(COPP_REJECT, od->nbs->copp,
+ meter_groups),
+  stage_hint);
 
 free(next_action);
 ds_destroy(&match);
@@ -5938,7 +5947,7 @@ build_lb(struct ovn_datapath *od, struct hmap *lflows)
 
 static void
 build_lb_rules(struct ovn_datapath *od, struct hmap *lflows,
-   struct ovn_northd_lb *lb)
+   struct ovn_northd_lb *lb, struct shash *meter_groups)
 {
 for (size_t i = 0; i < lb->n_vips; i++) {
 struct ovn_lb_vip *lb_vip = &lb->vips[i];
@@ -5979,21 +5988,25 @@ build_lb_rules(struct ovn_datapath *od, struct hmap 
*lflows,
 }
 
 /* New connections in Ingress table. */
-build_lb_vip_actions(

Re: [ovs-dev] [PATCH ovn 3/5] ovn-northd: Add CoPP policies for flows that punt packets to ovn-controller.

Re: [ovs-dev] [PATCH ovn 3/5] ovn-northd: Add CoPP policies for flows that punt packets to ovn-controller.

[ovs-dev] [PATCH ovn 3/5] ovn-northd: Add CoPP policies for flows that punt packets to ovn-controller.

3 matches

Site Navigation

Mail list logo

Footer information