Re: [ovs-dev] [PATCH ovn v2] northd: bypass connection tracking for stateless flows when there are LB flows present
Thanks, much, Han for applying the patch! Numan, i'd appreciate it if you could
still
have a look at the changes, so that if i can follow up, in case i missed
anything.
thanks,
-venu
From: Numan Siddique
Sent: Friday, December 16, 2022 1:07 PM
To: Han Zhou
Cc: Venugopal Iyer; d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH ovn v2] northd: bypass connection tracking for
stateless flows when there are LB flows present
External email: Use caution opening links or attachments
On Thu, Dec 15, 2022 at 4:38 PM Han Zhou wrote:
>
> On Mon, Dec 12, 2022 at 1:28 AM venu iyer wrote:
> >
> > Currently, even stateless flows are subject to connection tracking when
> there are
> > LB rules (for DNAT). However, if a flow needs to be subjected to LB, then
> it shouldn't
> > be configured as stateless.
> >
> > Stateless flow means we should not track it, and this change exempts
> stateless
> > flows from being tracked regardless of whether LB rules are present or
> not.
> >
> > Signed-off-by: venu iyer
> > Acked-by: Han Zhou
> > ---
> > northd/northd.c | 25 +++-
> > northd/ovn-northd.8.xml | 57
> > ovn-nb.xml | 3 +
> > tests/ovn-northd.at | 76 +--
> > tests/ovn.at| 4 +-
> > tests/system-ovn.at | 296
> > 6 files changed, 383 insertions(+), 78 deletions(-)
> >
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 7c48bb3b4..5d8ef612f 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -140,8 +140,8 @@ enum ovn_stage {
> > PIPELINE_STAGE(SWITCH, IN, L2_UNKNOWN,26, "ls_in_l2_unknown")
> \
> >
> \
> > /* Logical switch egress stages. */
> \
> > -PIPELINE_STAGE(SWITCH, OUT, PRE_LB, 0, "ls_out_pre_lb")
> \
> > -PIPELINE_STAGE(SWITCH, OUT, PRE_ACL, 1, "ls_out_pre_acl")
> \
> > +PIPELINE_STAGE(SWITCH, OUT, PRE_ACL, 0, "ls_out_pre_acl")
> \
> > +PIPELINE_STAGE(SWITCH, OUT, PRE_LB, 1, "ls_out_pre_lb")
> \
> > PIPELINE_STAGE(SWITCH, OUT, PRE_STATEFUL, 2, "ls_out_pre_stateful")
> \
> > PIPELINE_STAGE(SWITCH, OUT, ACL_HINT, 3, "ls_out_acl_hint")
> \
> > PIPELINE_STAGE(SWITCH, OUT, ACL, 4, "ls_out_acl")
> \
> > @@ -215,6 +215,7 @@ enum ovn_stage {
> > #define REGBIT_ACL_LABEL "reg0[13]"
> > #define REGBIT_FROM_RAMP "reg0[14]"
> > #define REGBIT_PORT_SEC_DROP "reg0[15]"
> > +#define REGBIT_ACL_STATELESS "reg0[16]"
> >
> > #define REG_ORIG_DIP_IPV4 "reg1"
> > #define REG_ORIG_DIP_IPV6 "xxreg1"
> > @@ -290,7 +291,7 @@ enum ovn_stage {
> > * | R0 | REGBIT_{CONNTRACK/DHCP/DNS} | |
> |
> > * || REGBIT_{HAIRPIN/HAIRPIN_REPLY} | |
> |
> > * || REGBIT_ACL_HINT_{ALLOW_NEW/ALLOW/DROP/BLOCK} | |
> |
> > - * || REGBIT_ACL_LABEL | X |
> |
> > + * || REGBIT_ACL_{LABEL/STATELESS} | X |
> |
> > * ++--+ X |
> |
> > * | R5 | UNUSED | X |
> LB_L2_AFF_BACKEND_IP6 |
> > * | R1 | ORIG_DIP_IPV4 (>= IN_PRE_STATEFUL) | R |
> |
> > @@ -5693,17 +5694,18 @@ build_stateless_filter(struct ovn_datapath *od,
> > const struct nbrec_acl *acl,
> > struct hmap *lflows)
> > {
> > +const char *action = REGBIT_ACL_STATELESS" = 1; next;";
> > if (!strcmp(acl->direction, "from-lport")) {
> > ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_PRE_ACL,
> > acl->priority + OVN_ACL_PRI_OFFSET,
> > acl->match,
> > -"next;",
> > +action,
> > &acl->header_);
> > } else {
> > ovn_lflow_add_with_hint(lflows, od, S_SWITCH_OUT_PRE_ACL,
> > acl->priority + OVN_ACL_PRI_OFFSET,
> > acl->match,
> > -"next;",
> > +action,
> > &acl->header_);
> > }
> > }
> > @@ -5795,6 +5797,10 @@ build_pre_acls(struct ovn_datapath *od, const
> struct hmap *port_groups,
> >REGBIT_CONNTRACK_DEFRAG" = 1; next;");
> > ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 100, "ip",
> >REGBIT_CONNTRACK_DEFRAG" = 1; next;");
> > +} else if (od->has_lb_vip) {
> > +/* We'll build stateless filters if there are LB rules so that
> > + * the stateless flows are not tracked in pre-lb. */
> > + build_stateless_filters(od, port_groups, lflows);
> > }
> >
Re: [ovs-dev] [PATCH ovn] northd: bypass connection tracking for stateless flows when there are LB flows present
Thanks for looking at this, Numan. I haven't worked with the system tests
before, and am having some
issues getting it to run. So, let me get back to you on this.
regards,
-venu
From: Numan Siddique
Sent: Monday, November 14, 2022 9:17 AM
To: Han Zhou
Cc: Venugopal Iyer; d...@openvswitch.org
Subject: Re: [ovs-dev] [PATCH ovn] northd: bypass connection tracking for
stateless flows when there are LB flows present
External email: Use caution opening links or attachments
On Wed, Nov 9, 2022 at 4:15 PM Numan Siddique wrote:
>
> On Wed, Nov 9, 2022 at 4:11 PM Han Zhou wrote:
> >
> > On Tue, Nov 8, 2022 at 7:51 AM venu.iyer wrote:
> > >
> > > Currently, even stateless flows are subject to connection tracking when
> > there are
> > > LB rules (for DNAT). However, if a flow needs to be subjected to LB, then
> > it shouldn't
> > > be configured as stateless.
> > >
> > > A stateless flow means we should not track it, and this change exempts
> > stateless
> > > flows from being tracked regardless of whether LB rules are present or
> > not.
> > >
> > > Signed-off-by: venu.iyer
> > > Acked-by: Han Zhou
I had a quick look and it looks ok to me.
Can you please add system tests to cover some scenarios for this use
case to avoid future regressions ?
Thanks
Numan
> > > ---
> > > northd/northd.c | 24 +-
> > > northd/ovn-northd.8.xml | 56 ++---
> > > ovn-nb.xml | 3 +++
> > > tests/ovn-northd.at | 48 ---
> > > tests/ovn.at| 4 +--
> > > 5 files changed, 69 insertions(+), 66 deletions(-)
> > >
> > > diff --git a/northd/northd.c b/northd/northd.c
> > > index b7388afc5..da4beede6 100644
> > > --- a/northd/northd.c
> > > +++ b/northd/northd.c
> > > @@ -137,8 +137,8 @@ enum ovn_stage {
> > > PIPELINE_STAGE(SWITCH, IN, L2_UNKNOWN,24, "ls_in_l2_unknown")
> > \
> > >
> > \
> > > /* Logical switch egress stages. */
> > \
> > > -PIPELINE_STAGE(SWITCH, OUT, PRE_LB, 0, "ls_out_pre_lb")
> > \
> > > -PIPELINE_STAGE(SWITCH, OUT, PRE_ACL, 1, "ls_out_pre_acl")
> > \
> > > +PIPELINE_STAGE(SWITCH, OUT, PRE_ACL, 0, "ls_out_pre_acl")
> > \
> > > +PIPELINE_STAGE(SWITCH, OUT, PRE_LB, 1, "ls_out_pre_lb")
> > \
> > > PIPELINE_STAGE(SWITCH, OUT, PRE_STATEFUL, 2, "ls_out_pre_stateful")
> > \
> > > PIPELINE_STAGE(SWITCH, OUT, ACL_HINT, 3, "ls_out_acl_hint")
> > \
> > > PIPELINE_STAGE(SWITCH, OUT, ACL, 4, "ls_out_acl")
> > \
> > > @@ -210,6 +210,7 @@ enum ovn_stage {
> > > #define REGBIT_ACL_LABEL "reg0[13]"
> > > #define REGBIT_FROM_RAMP "reg0[14]"
> > > #define REGBIT_PORT_SEC_DROP "reg0[15]"
> > > +#define REGBIT_ACL_STATELESS "reg0[16]"
> > >
> > > #define REG_ORIG_DIP_IPV4 "reg1"
> > > #define REG_ORIG_DIP_IPV6 "xxreg1"
> > > @@ -271,7 +272,7 @@ enum ovn_stage {
> > > * | R0 | REGBIT_{CONNTRACK/DHCP/DNS} | |
> > |
> > > * || REGBIT_{HAIRPIN/HAIRPIN_REPLY} | |
> > |
> > > * || REGBIT_ACL_HINT_{ALLOW_NEW/ALLOW/DROP/BLOCK} | |
> > |
> > > - * || REGBIT_ACL_LABEL | X |
> > |
> > > + * || REGBIT_ACL_{LABEL/STATELESS} | X |
> > |
> > > * ++--+ X |
> > |
> > > * | R1 | ORIG_DIP_IPV4 (>= IN_PRE_STATEFUL) | R |
> > |
> > > * ++--+ E |
> > |
> > > @@ -5677,17 +5678,18 @@ build_stateless_filter(struct ovn_datapath *od,
> > > const struct nbrec_acl *acl,
> > > struct hmap *lflows)
> > > {
> > > +const char *action = REGBIT_ACL_STATELESS" = 1; next;";
> > > if (!strcmp(acl->direction, "from-lport")) {
> > > ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_PRE_ACL,
> > > acl->priority + OVN_ACL_PRI_OFFSET,
> > > acl->match,
> > > -"next;",
> > > +action,
> > > &acl->header_);
> > > } else {
> > > ovn_lflow_add_with_hint(lflows, od, S_SWITCH_OUT_PRE_ACL,
> > > acl->priority + OVN_ACL_PRI_OFFSET,
> > > acl->match,
> > > -"next;",
> > > +action,
> > > &acl->header_);
> > > }
> > > }
> > > @@ -5779,6 +5781,10 @@ build_pre_acls(struct ovn_datapath *od, const
> > struct hmap *port_groups,
> > >REGBIT_CONNTRACK_DEFRAG" = 1; next;");
> > > ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 100, "ip",
> > >REGBIT_CONNTRACK_DEFRAG" = 1; nex
Re: [ovs-dev] [PATCH ovn] northd: bypass connection tracking for statless flows when there are LB flows present
Please ignore this patch, resending the patch with typos in the comments fixed.
thanks,
-venu
On Tuesday, November 8, 2022 at 07:40:41 AM PST, venu.iyer via dev
wrote:
Currently, even statless flows are subject to connection tracking when there
are
LB rules (for DNAT). However, if a flow needs to be subjected to LB, then it
shouldn't
be configured for stateless.
A stateles flows means we should not track it, and this change exempts stateless
flows from being tracked regardless of whether LB rules are present or not.
Signed-off-by: "venu.iyer"
Acked-by: Han Zhou
---
northd/northd.c | 24 +-
northd/ovn-northd.8.xml | 56 ++---
ovn-nb.xml | 3 +++
tests/ovn-northd.at | 48 ---
tests/ovn.at | 4 +--
5 files changed, 69 insertions(+), 66 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index b7388afc5..da4beede6 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -137,8 +137,8 @@ enum ovn_stage {
PIPELINE_STAGE(SWITCH, IN, L2_UNKNOWN, 24, "ls_in_l2_unknown") \
\
/* Logical switch egress stages. */ \
- PIPELINE_STAGE(SWITCH, OUT, PRE_LB, 0, "ls_out_pre_lb") \
- PIPELINE_STAGE(SWITCH, OUT, PRE_ACL, 1, "ls_out_pre_acl") \
+ PIPELINE_STAGE(SWITCH, OUT, PRE_ACL, 0, "ls_out_pre_acl") \
+ PIPELINE_STAGE(SWITCH, OUT, PRE_LB, 1, "ls_out_pre_lb") \
PIPELINE_STAGE(SWITCH, OUT, PRE_STATEFUL, 2, "ls_out_pre_stateful") \
PIPELINE_STAGE(SWITCH, OUT, ACL_HINT, 3, "ls_out_acl_hint") \
PIPELINE_STAGE(SWITCH, OUT, ACL, 4, "ls_out_acl") \
@@ -210,6 +210,7 @@ enum ovn_stage {
#define REGBIT_ACL_LABEL "reg0[13]"
#define REGBIT_FROM_RAMP "reg0[14]"
#define REGBIT_PORT_SEC_DROP "reg0[15]"
+#define REGBIT_ACL_STATELESS "reg0[16]"
#define REG_ORIG_DIP_IPV4 "reg1"
#define REG_ORIG_DIP_IPV6 "xxreg1"
@@ -271,7 +272,7 @@ enum ovn_stage {
* | R0 | REGBIT_{CONNTRACK/DHCP/DNS} | | |
* | | REGBIT_{HAIRPIN/HAIRPIN_REPLY} | | |
* | | REGBIT_ACL_HINT_{ALLOW_NEW/ALLOW/DROP/BLOCK} | | |
- * | | REGBIT_ACL_LABEL | X | |
+ * | | REGBIT_ACL_{LABEL/STATELESS} | X | |
* ++--+ X | |
* | R1 | ORIG_DIP_IPV4 (>= IN_PRE_STATEFUL) | R | |
* ++--+ E | |
@@ -5677,17 +5678,18 @@ build_stateless_filter(struct ovn_datapath *od,
const struct nbrec_acl *acl,
struct hmap *lflows)
{
+ const char *action = REGBIT_ACL_STATELESS" = 1; next;";
if (!strcmp(acl->direction, "from-lport")) {
ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_PRE_ACL,
acl->priority + OVN_ACL_PRI_OFFSET,
acl->match,
- "next;",
+ action,
&acl->header_);
} else {
ovn_lflow_add_with_hint(lflows, od, S_SWITCH_OUT_PRE_ACL,
acl->priority + OVN_ACL_PRI_OFFSET,
acl->match,
- "next;",
+ action,
&acl->header_);
}
}
@@ -5779,6 +5781,10 @@ build_pre_acls(struct ovn_datapath *od, const struct
hmap *port_groups,
REGBIT_CONNTRACK_DEFRAG" = 1; next;");
ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 100, "ip",
REGBIT_CONNTRACK_DEFRAG" = 1; next;");
+ } else if (od->has_lb_vip) {
+ /* We'll build stateless filters if there are LB rules so that
+ * the stateless flows are not tracked in pre-lb. */
+ build_stateless_filters(od, port_groups, lflows);
}
}
@@ -5913,6 +5919,11 @@ build_pre_lb(struct ovn_datapath *od, const struct shash
*meter_groups,
S_SWITCH_IN_PRE_LB, S_SWITCH_OUT_PRE_LB,
110, lflows);
}
+ /* Do not sent statless flows via conntrack */
+ ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
+ REGBIT_ACL_STATELESS" == 1", "next;");
+ ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
+ REGBIT_ACL_STATELESS" == 1", "next;");
/* 'REGBIT_CONNTRACK_NAT' is set to let the pre-stateful table send
* packet to conntrack for defragmentation and possibly for unNATting.
@@ -6918,7 +6929,8 @@ build_lb_rules_pre_stateful(struct hmap *lflows, struct
ovn_northd_lb *lb,
Re: [ovs-dev] stateless acl on ovn-k8s interface
Hi, Numan:
On Thursday, September 15, 2022 at 08:38:26 AM PDT, Numan Siddique
wrote:
On Tue, Sep 13, 2022 at 6:41 PM venugopal iyer via dev
wrote:
>
> Hi, Han, Numan:
> While testing a use case in our ovn-k8s cluster we ran into an issue where
> wecouldn't effectively use stateless ACL on the OVN interface. Turns out we
> will track
> all the packets here, since there will be a some VIPs configured on the
> logical
> switch and...
>
>
> if (od->has_lb_vip) {
> ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB,
> 100, "ip", REGBIT_CONNTRACK_NAT" = 1; next;");...
>
> even though pre_acl skips stateless flows. One option is to just try an
> excludethe flows in pre_lb with hints from pre_acl, but that alone won't work
> since
> pre_lb precedes pre_acl in egress.
>
> So, wanted to check if the following will work: - Use info from pre_acl to
> exclude stateless flows when we track for vip- Switch pre_lb and pre_acl in
> egress.
>
> I tested this with our ovn-k8s cicd, manual stateless rules and using a
> specificuse case (RoCE with stateless rules) and it worked, but not sure if i
> am missinganything by switching pre acl and lb in egress.
>
> I can send the official patch, but just as an FYI, following is the change i
> amlooking at as a proof of concept:
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 98ef97f90..022b20660 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -128,8 +128,8 @@ enum ovn_stage {
> PIPELINE_STAGE(SWITCH, IN, L2_UNKNOWN, 25, "ls_in_l2_unknown") \
> \
> /* Logical switch egress stages. */ \
> - PIPELINE_STAGE(SWITCH, OUT, PRE_LB, 0, "ls_out_pre_lb") \
> - PIPELINE_STAGE(SWITCH, OUT, PRE_ACL, 1, "ls_out_pre_acl") \
> + PIPELINE_STAGE(SWITCH, OUT, PRE_ACL, 0, "ls_out_pre_acl") \
> + PIPELINE_STAGE(SWITCH, OUT, PRE_LB, 1, "ls_out_pre_lb") \
> PIPELINE_STAGE(SWITCH, OUT, PRE_STATEFUL, 2, "ls_out_pre_stateful") \
> PIPELINE_STAGE(SWITCH, OUT, ACL_HINT, 3, "ls_out_acl_hint") \
> PIPELINE_STAGE(SWITCH, OUT, ACL, 4, "ls_out_acl") \
> @@ -5839,6 +5839,54 @@ build_empty_lb_event_flow(struct ovn_lb_vip *lb_vip,
> return true;
> }
>
> static void
> build_pre_lb(struct ovn_datapath *od, struct hmap *lflows)
> {
> @@ -5904,12 +5952,17 @@ build_pre_lb(struct ovn_datapath *od, struct hmap
> *lflows)
> * To fix this issue, we send all the packets to the conntrack in the
> * ingress pipeline if a load balancer is configured. We can now
> * add a lflow to drop ct.inv packets.
> + *
> + * Skip stateless rules from being tracked.
> + * stateless flows should not have REGBIT_CONNTRACK_DEFRAG+ * set in
> build pre_acls, so we can use that as an hint here.
> */
> if (od->has_lb_vip) {
> ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB,
> - 100, "ip", REGBIT_CONNTRACK_NAT" = 1; next;");
> + 100, REGBIT_CONNTRACK_DEFRAG" == 1",
> REGBIT_CONNTRACK_NAT" = 1; next;");
> ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB,
> - 100, "ip", REGBIT_CONNTRACK_NAT" = 1; next;");
> + 100, REGBIT_CONNTRACK_DEFRAG" == 1",
> REGBIT_CONNTRACK_NAT" = 1; next;");
> }
> }
Hi Venu,
I'm not really sure if this would work. In the case of a logical
switch with no stateful ACLs and load balancers configured,
how would the traffic work if a pod sends a packet destined to the lb VIP ?
From what I understand with your proposed patch, the register bit
REGBIT_CONNTRACK_NAT will not be set now and hence the packet will not
be
sent to conntrack.
Thanks for your response, the above was just to quickly check if this
works; I'll work on the official patch to send for review, CI etc.
My main concern was the switch in the egress pipeline and wanted to see
if that has any fundamental concerns.
thanks,
-venu
Also I'd suggest posting your patch so that the CI gets run.
Thanks
Numan
>
>
>
>
> thanks,
> -venu
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] stateless acl on ovn-k8s interface
Hi, Han, Numan:
While testing a use case in our ovn-k8s cluster we ran into an issue where
wecouldn't effectively use stateless ACL on the OVN interface. Turns out we
will track
all the packets here, since there will be a some VIPs configured on the logical
switch and...
if (od->has_lb_vip) {
ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB,
100, "ip", REGBIT_CONNTRACK_NAT" = 1; next;");...
even though pre_acl skips stateless flows. One option is to just try an
excludethe flows in pre_lb with hints from pre_acl, but that alone won't work
since
pre_lb precedes pre_acl in egress.
So, wanted to check if the following will work: - Use info from pre_acl to
exclude stateless flows when we track for vip- Switch pre_lb and pre_acl in
egress.
I tested this with our ovn-k8s cicd, manual stateless rules and using a
specificuse case (RoCE with stateless rules) and it worked, but not sure if i
am missinganything by switching pre acl and lb in egress.
I can send the official patch, but just as an FYI, following is the change i
amlooking at as a proof of concept:
diff --git a/northd/northd.c b/northd/northd.c
index 98ef97f90..022b20660 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -128,8 +128,8 @@ enum ovn_stage {
PIPELINE_STAGE(SWITCH, IN, L2_UNKNOWN, 25, "ls_in_l2_unknown") \
\
/* Logical switch egress stages. */ \
- PIPELINE_STAGE(SWITCH, OUT, PRE_LB, 0, "ls_out_pre_lb") \
- PIPELINE_STAGE(SWITCH, OUT, PRE_ACL, 1, "ls_out_pre_acl") \
+ PIPELINE_STAGE(SWITCH, OUT, PRE_ACL, 0, "ls_out_pre_acl") \
+ PIPELINE_STAGE(SWITCH, OUT, PRE_LB, 1, "ls_out_pre_lb") \
PIPELINE_STAGE(SWITCH, OUT, PRE_STATEFUL, 2, "ls_out_pre_stateful") \
PIPELINE_STAGE(SWITCH, OUT, ACL_HINT, 3, "ls_out_acl_hint") \
PIPELINE_STAGE(SWITCH, OUT, ACL, 4, "ls_out_acl") \
@@ -5839,6 +5839,54 @@ build_empty_lb_event_flow(struct ovn_lb_vip *lb_vip,
return true;
}
static void
build_pre_lb(struct ovn_datapath *od, struct hmap *lflows)
{
@@ -5904,12 +5952,17 @@ build_pre_lb(struct ovn_datapath *od, struct hmap
*lflows)
* To fix this issue, we send all the packets to the conntrack in the
* ingress pipeline if a load balancer is configured. We can now
* add a lflow to drop ct.inv packets.
+ *
+ * Skip stateless rules from being tracked.
+ * stateless flows should not have REGBIT_CONNTRACK_DEFRAG+ * set in
build pre_acls, so we can use that as an hint here.
*/
if (od->has_lb_vip) {
ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB,
- 100, "ip", REGBIT_CONNTRACK_NAT" = 1; next;");
+ 100, REGBIT_CONNTRACK_DEFRAG" == 1",
REGBIT_CONNTRACK_NAT" = 1; next;");
ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB,
- 100, "ip", REGBIT_CONNTRACK_NAT" = 1; next;");
+ 100, REGBIT_CONNTRACK_DEFRAG" == 1",
REGBIT_CONNTRACK_NAT" = 1; next;");
}
}
thanks,
-venu
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn] binding.c: Clear Port_Binding's encap column when encap-ip is removed.
Changes look good to me Han.
thanks!
-venu
From: Han Zhou
Sent: Tuesday, May 3, 2022 11:36 AM
To: d...@openvswitch.org
Cc: Han Zhou; Venugopal Iyer
Subject: [PATCH ovn] binding.c: Clear Port_Binding's encap column when encap-ip
is removed.
External email: Use caution opening links or attachments
The external_ids:encap-ip was supported to optionally specify encap IP
for a logical switch port. However, when the external_ids:encap-ip is
unset, the implementation didn't clear the column in Port_Binding. This
patch fixes it and updated the test to cover this scenario.
Fixes: dd527a283cd8 ("Support for multiple VTEP in OVN")
Cc: Venugopal Iyer
Signed-off-by: Han Zhou
---
controller/binding.c | 3 ++-
tests/ovn.at | 9 +
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/controller/binding.c b/controller/binding.c
index 7281b0485..e284704d8 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -960,7 +960,8 @@ claim_lport(const struct sbrec_port_binding *pb,
/* Check if the port encap binding, if any, has changed */
struct sbrec_encap *encap_rec =
sbrec_get_port_encap(chassis_rec, iface_rec);
-if (encap_rec && pb->encap != encap_rec) {
+if ((encap_rec && pb->encap != encap_rec) ||
+(!encap_rec && pb->encap)) {
if (sb_readonly) {
return false;
}
diff --git a/tests/ovn.at b/tests/ovn.at
index 34b6abfc0..799a6aabd 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -2462,6 +2462,15 @@ encap_rec=$(fetch_column Encap _uuid chassis_name=hv2
type=geneve ip=192.168.0.2
check_row_count Port_Binding 1 logical_port=lp21 encap=$encap_rec
check_row_count Port_Binding 1 logical_port=lp22 encap=$encap_rec
+# Remove the encap-ip setting in vif, which should trigger encap removal from
+# Port_Binding.
+as hv1 ovs-vsctl remove Interface vif11 external-ids encap-ip
+wait_row_count Port_Binding 1 logical_port=lp11 'encap=[[]]'
+
+# Change it back and continue the test
+as hv1 ovs-vsctl set Interface vif11 external-ids:encap-ip=192.168.0.1
+wait_row_count Port_Binding 1 logical_port=lp11 'encap!=[[]]'
+
# Pre-populate the hypervisors' ARP tables so that we don't lose any
# packets for ARP resolution (native tunneling doesn't queue packets
# for ARP resolution).
--
2.30.2
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] openvswitch 2.15 [re]start losing bridge IP
Hi, Ben et. al.: When we moved from 2.14.0 to 2.15.0 we noticed a change in behavior. A 2.14.0 openvswitch restart (even explicit stop followed by start) didn't lose the existing bridge IP (local bridge port). However, with ovs 2.15.0 we see that the bridge IP disappears after the [re]start. (I thinkwe tried ovs master too and saw the same) With debug we see "2021-06-01T19:19:40.231Z|00283|dpif|DBG|system@ovs-system: port_del(2) 2021-06-01T19:19:40.232Z|00288|dpif|DBG|system@ovs-system: port_del(1) " which we don't see in the 2.14.0 case (unless i missed it). Looking at the commits in our 2.15.0 branch, we see the issue starts with 1c337c43ac1c876d1a5c204884fbc949882c12c2 ovsdb-idl: Break into two layers. This change breaks the IDL into two layers: the IDL proper, whose interface to its client is unchanged, and a low-level library called... at least in our testing. We'll continue to check this at our end, but wanted to quickly check the reasoning behind this, i.e. if this isintentional. Most importantly, if anyone else has seen this (we use 2.15.0 with some cherry picks from master in our tests) to check if it is something at our end. This is an issue for us as the host IP is on the bridge and an update/restart causes the host to be inaccessible. thanks much! -venu ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v3 1/1] controller: Check for tunnel change in multi-vtep case is incorrect
Thanks, Numan!
-venu
On Monday, September 28, 2020, 3:34:20 AM PDT, Numan Siddique
wrote:
On Fri, Sep 25, 2020 at 11:44 PM wrote:
>
> From: venu iyer
>
> Prior to multi-vtep, there was one tunnel for each type, since
> there was only one encap-ip. So, the check in
> chassis_tunnels_changed():
>
> sset_count(encap_type_set) != encap_type_count
>
> worked. However, with multiple IPs per tunnel type, the above
> check won't work. So, once multiple encap-ips are configured,
> ovn-controller will always keep updating the encap list in
> the SB (due to the above check); this causes a lot of unnecessary
> churn, including recomputing the flows etc. which will put
> ovn-controller in overdrive thereby consuming lot of CPU cycles
> (see almost 100%)
>
> Verified ovn-controller cpu utilization with the fix (and also
> that SB doesn't keep constantly updating). make check didn't
> show any additional failures.
>
> Fixes: Fixes: b520ca7 ("Support for multiple VTEP in OVN")
> Signed-off-by: venu iyer
Thanks for the verson 3.
Instead of having multiple returns in the function - chassis_tunnels_changed(),
I modified the function to have just one return and applied the patch
to master and backported
to branch-20.,09 and branch-20.06.
Below are the changes I did on top of your patch
**
diff --git a/controller/chassis.c b/controller/chassis.c
index 7d78366e34..7748fb94cf 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -417,42 +417,46 @@ chassis_tunnels_changed(const struct sset *encap_type_set,
{
struct sset chassis_rec_encap_type_set =
SSET_INITIALIZER(&chassis_rec_encap_type_set);
+ bool changed = false;
for (size_t i = 0; i < chassis_rec->n_encaps; i++) {
if (!sset_contains(encap_type_set, chassis_rec->encaps[i]->type)) {
- sset_destroy(&chassis_rec_encap_type_set);
- return true;
+ changed = true;
+ break;
}
sset_add(&chassis_rec_encap_type_set, chassis_rec->encaps[i]->type);
if (!sset_contains(encap_ip_set, chassis_rec->encaps[i]->ip)) {
- sset_destroy(&chassis_rec_encap_type_set);
- return true;
+ changed = true;
+ break;
}
if (strcmp(smap_get_def(&chassis_rec->encaps[i]->options, "csum", ""),
encap_csum)) {
- sset_destroy(&chassis_rec_encap_type_set);
- return true;
+ changed = true;
+ break;
}
}
- size_t tunnel_count =
- sset_count(encap_type_set) * sset_count(encap_ip_set);
+ if (!changed) {
+ size_t tunnel_count =
+ sset_count(encap_type_set) * sset_count(encap_ip_set);
- if (tunnel_count != chassis_rec->n_encaps) {
- sset_destroy(&chassis_rec_encap_type_set);
- return true;
+ if (tunnel_count != chassis_rec->n_encaps) {
+ changed = true;
+ }
}
- if (sset_count(encap_type_set) !=
- sset_count(&chassis_rec_encap_type_set)) {
- sset_destroy(&chassis_rec_encap_type_set);
- return true;
+ if (!changed) {
+ if (sset_count(encap_type_set) !=
+ sset_count(&chassis_rec_encap_type_set)) {
+ changed = true;
+ }
}
+
sset_destroy(&chassis_rec_encap_type_set);
- return false;
+ return changed;
}
/*
diff --git a/tests/ovn.at b/tests/ovn.at
index bd50157058..7769b69ed0 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -22321,8 +22321,7 @@ ovs-vsctl add-br br-phys
ovn_attach n1 br-phys 192.168.0.1 24 geneve
# Get the encap rec, should be just one - with geneve/192.168.0.1
-encap_rec=`ovn-sbctl --data=bare --no-heading --column encaps list chassis hv1`
-#echo "Encap Rec before mvtep is $encap_rec"
+encap_rec=$(ovn-sbctl --data=bare --no-heading --column encaps list
chassis hv1)
# Set multiple IPs
as hv1
@@ -22331,8 +22330,7 @@ ovs-vsctl \
# Check if the encap_rec changed - should have, no need to
# compare the exact values.
-encap_rec_mvtep=`ovn-sbctl --data=bare --no-heading --column encaps
list chassis hv1`
-#echo "Encap Rec after mvtep is $encap_rec_mvtep"
+encap_rec_mvtep=$(ovn-sbctl --data=bare --no-heading --column encaps
list chassis hv1)
AT_CHECK([test "$encap_rec" != "$encap_rec_mvtep"], [0], [])
@@ -22340,8 +22338,7 @@ AT_CHECK([test "$encap_rec" !=
"$encap_rec_mvtep"], [0], [])
sleep 2
# Check if the encap_rec changed (it should not have)
-encap_rec_mvtep1=`ovn-sbctl --data=bare --no-heading --column encaps
list chassis hv1`
-#echo "Encap Rec after wait is $encap_rec_mvtep1"
+encap_rec_mvtep1=$(ovn-sbctl --data=bare --no-heading --column encaps
list chassis hv1)
AT_CHECK([test "$encap_rec_mvtep" == "$encap_rec_mvtep1"], [0], [])
**
Thanks
Numan
> ---
> controller/chassis.c | 15 ++
> tests/ovn.at | 48
> 2 files changed, 59 insertions(+), 4 de
Re: [ovs-dev] [PATCH ovn 1/1] ovn-controller: Check for tunnel change in multi-vtep case is incorrect
Hi, Numan:
I'll add the Fixes tag, let me think about how to write a test case for this.
thanks!
-venu
On Wednesday, September 23, 2020, 10:18:00 AM PDT, Numan Siddique
wrote:
On Wed, Sep 23, 2020 at 10:40 PM wrote:
> From: venu iyer
>
> Prior to multi-vtep, there was one tunnel for each type, since
> there was only one encap-ip. So, the check in
> chassis_tunnels_changed():
> sset_count(encap_type_set) != encap_type_count
> worked. However, with multiple IPs per tunnel type, the above
> check won't work. So, once multiple encap-ips are configured,
> ovn-controller will always keep updating the encap list in
> the SB (due to the above check); this causes a lot of unnecessary
> churn, including recomputing the flows etc. which will put
> ovn-controller in overdrive thereby consuming lot of CPU cycles
> (see almost 100%).
>
> Verified ovn-controller cpu utilization with the fix (and also
> that SB doesn't keep constantly updating). make check didn't
> show any additional failures.
>
> Signed-off-by: venu iyer (venugop...@nvidia.com)
> ---
>
hi Venu,
I didn't look into the patch. But I have a couple of requests.
Can you please put the Fixes tag with the commit id which introduced
multi-vtep ?
And also a test case to cover this scenario ?
Thanks
Numan
> controller/chassis.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/controller/chassis.c b/controller/chassis.c
> index a365188e8..a6dfb92df 100644
> --- a/controller/chassis.c
> +++ b/controller/chassis.c
> @@ -415,14 +415,15 @@ chassis_tunnels_changed(const struct sset
> *encap_type_set,
> const char *encap_csum,
> const struct sbrec_chassis *chassis_rec)
> {
> - size_t encap_type_count = 0;
> + struct sset chassis_rec_encap_type_set;
>
> + sset_init(&chassis_rec_encap_type_set);
> for (size_t i = 0; i < chassis_rec->n_encaps; i++) {
>
> if (!sset_contains(encap_type_set, chassis_rec->encaps[i]->type))
> {
> return true;
> }
> - encap_type_count++;
> + sset_add(&chassis_rec_encap_type_set,
> chassis_rec->encaps[i]->type);
>
> if (!sset_contains(encap_ip_set, chassis_rec->encaps[i]->ip)) {
> return true;
> @@ -441,7 +442,8 @@ chassis_tunnels_changed(const struct sset
> *encap_type_set,
> return true;
> }
>
> - if (sset_count(encap_type_set) != encap_type_count) {
> + if (sset_count(encap_type_set) !=
> + sset_count(&chassis_rec_encap_type_set)) {
> return true;
> }
>
> --
> 2.17.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v3] ovn-northd: ls_*_acl behavior not consistent for untracked flows
Thanks, Numan! Please let me know what I should be monitoring to check for
issues, if any,resulting from this change.
thanks,
-venu
On Friday, December 20, 2019, 11:36:30 AM PST, Numan Siddique
wrote:
On Fri, Dec 20, 2019 at 12:15 AM wrote:
>
> From: venu iyer
>
> If one creates a port group and a MAC address set, and an
> ACL that prevents packets being output to a port in that Port Group from
> any MAC address in that address set, the outcome is not consistent.
>
> The outcome depends on whether there is a stateful rule on the switch or not.
>
> Specifically:
>
> Assuming 'l2pg' is a port group with a list of ports and 'macs' is an Address
> Set with a list of MAC addresses and the intent is to drop all packets
> with source MAC address in 'macs' to any port in 'l2pg' using:
>
> ovn-nbctl acl-add to-lport 5000 \
> "outport == @l2pg && eth.src == $macs" drop
>
> Without any stateful rule on the logical switch, the corresponding
> logical flow looks like:
> table=4 (ls_out_acl ), priority=6000,\
> match=(outport == @l2pg && eth.src == $macs), \
> action=(/* drop */)
>
> Based on this rule, any packet destined to the ports in 'l2pg' with source
> Address in 'macs' will be dropped - as is expected from the ACL above.
>
> While with a Stateful rule on the switch (any stateful rule will do),
> the same rule looks like:
> table=4 (ls_out_acl ), priority=6000, \
> match=((!ct.est || (ct.est && ct_label.blocked == 1)) && \
> (outport == @l2pg && eth.src == $macs)), action=(/* drop */)
>
> With this, however, only packets that are tracked will match the rule
> and be dropped, e.g. IP packets will be dropped, but ARP etc., will go
> through - this is not expected.
>
> Based on whether there are stateful rules or not on the switch,
> untracked packets will see different behavior.
>
> The fix is to make the rule in the stateful case comprehensive, i.e.
> instead of just looking for flows that are not established (or not new),
> we should also look for flows that are not tracked.
>
> The fix was tested in the above scenario. Additionally, the following
> ACL was added to test the change in the "allow" case (i.e. to drop
> all the packets based on the above ACL, but have a higher priority
> rule that selectively allow ARP).
>
> ovn-nbctl acl-add ls1 to-lport 6000
> "outport == @l2pg && eth.type == 0x806" allow
>
> with and without the stateful rule to make sure the behavior is the
> same. The test suite has been enhanced to add the above test cases
> (with different ethertype) for drop and allow.
>
> OVN test cases were run with this fix and no failures were seen.
>
> Signed-off-by: venu iyer
Thanks for rebasing the patch. I applied this patch to master. I also added your
name in the AUTHORS.rst in the same commit.
Thanks
Numan
> ---
> northd/ovn-northd.c | 13 +--
> tests/ovn.at | 205
> 2 files changed, 212 insertions(+), 6 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 3a5cb7c91..d91a008b7 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -4900,12 +4900,12 @@ consider_acl(struct hmap *lflows, struct ovn_datapath
> *od,
> * deletion. There is no need to commit here, so we can just
> * proceed to the next table. We use this to ensure that this
> * connection is still allowed by the currently defined
> - * policy. */
> + * policy. Match untracked packets too. */
> ds_clear(&match);
> ds_clear(&actions);
> ds_put_format(&match,
> - "!ct.new && ct.est && !ct.rpl"
> - " && ct_label.blocked == 0 && (%s)",
> + "(!ct.trk || (!ct.new && ct.est && !ct.rpl"
> + " && ct_label.blocked == 0)) && (%s)",
> acl->match);
>
> build_acl_log(&actions, acl);
> @@ -4928,10 +4928,11 @@ consider_acl(struct hmap *lflows, struct ovn_datapath
> *od,
> * depending on whether the connection was previously committed
> * to the connection tracker with ct_commit. */
> if (has_stateful) {
> - /* If the packet is not part of an established connection, then
> - * we can simply reject/drop it. */
> + /* If the packet is not tracked or not part of an established
> + * connection, then we can simply reject/drop it. */
> ds_put_cstr(&match,
> - "(!ct.est || (ct.est && ct_label.blocked == 1))");
> + "(!ct.trk || !ct.est"
> + " || (ct.est && ct_label.blocked == 1))");
> if (!strcmp(acl->action, "reject")) {
> build_reject_acl_rules(od, lflows, stage, acl, &match,
> &actions);
>
Re: [ovs-dev] [PATCH ovn v2] ovn-northd: ls_*_acl behavior notconsistent for untracked flows
Would appreciate any thoughts on this.
thanks,
-venu
On Tuesday, November 19, 2019, 12:40:47 PM PST,
wrote:
From: venu iyer
If one creates a port group and a MAC address set, and an
ACL that prevents packets being output to a port in that Port Group from
any MAC address in that address set, the outcome is not consistent.
The outcome depends on whether there is a stateful rule on the switch or not.
Specifically:
Assuming 'l2pg' is a port group with a list of ports and 'macs' is an Address
Set with a list of MAC addresses and the intent is to drop all packets
with source MAC address in 'macs' to any port in 'l2pg' using:
ovn-nbctl acl-add to-lport 5000 \
"outport == @l2pg && eth.src == $macs" drop
Without any stateful rule on the logical switch, the corresponding
logical flow looks like:
table=4 (ls_out_acl ), priority=6000,\
match=(outport == @l2pg && eth.src == $macs), \
action=(/* drop */)
Based on this rule, any packet destined to the ports in 'l2pg' with source
Address in 'macs' will be dropped - as is expected from the ACL above.
While with a Stateful rule on the switch (any stateful rule will do),
the same rule looks like:
table=4 (ls_out_acl ), priority=6000, \
match=((!ct.est || (ct.est && ct_label.blocked == 1)) && \
(outport == @l2pg && eth.src == $macs)), action=(/* drop */)
With this, however, only packets that are tracked will match the rule
and be dropped, e.g. IP packets will be dropped, but ARP etc., will go
through - this is not expected.
Based on whether there are stateful rules or not on the switch,
untracked packets will see different behavior.
The fix is to make the rule in the stateful case comprehensive, i.e.
instead of just looking for flows that are not established (or not new),
we should also look for flows that are not tracked.
The fix was tested in the above scenario. Additionally, the following
ACL was added to test the change in the "allow" case (i.e. to drop
all the packets based on the above ACL, but have a higher priority
rule that selectively allow ARP).
ovn-nbctl acl-add ls1 to-lport 6000
"outport == @l2pg && eth.type == 0x806" allow
with and without the stateful rule to make sure the behavior is the
same. The test suite has been enhanced to add the above test cases
(with different ethertype) for drop and allow.
OVN test cases were run with this fix and no failures were seen.
Signed-off-by: venu iyer
---
northd/ovn-northd.c | 13 +--
tests/ovn.at | 206
2 files changed, 213 insertions(+), 6 deletions(-)
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index a5e711e69..69ba85321 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -4885,12 +4885,12 @@ consider_acl(struct hmap *lflows, struct ovn_datapath
*od,
* deletion. There is no need to commit here, so we can just
* proceed to the next table. We use this to ensure that this
* connection is still allowed by the currently defined
- * policy. */
+ * policy. Match untracked packets too. */
ds_clear(&match);
ds_clear(&actions);
ds_put_format(&match,
- "!ct.new && ct.est && !ct.rpl"
- " && ct_label.blocked == 0 && (%s)",
+ "(!ct.trk || (!ct.new && ct.est && !ct.rpl"
+ " && ct_label.blocked == 0)) && (%s)",
acl->match);
build_acl_log(&actions, acl);
@@ -4913,10 +4913,11 @@ consider_acl(struct hmap *lflows, struct ovn_datapath
*od,
* depending on whether the connection was previously committed
* to the connection tracker with ct_commit. */
if (has_stateful) {
- /* If the packet is not part of an established connection, then
- * we can simply reject/drop it. */
+ /* If the packet is not tracked or part of an established
+ * connection, then we can simply reject/drop it. */
ds_put_cstr(&match,
- "(!ct.est || (ct.est && ct_label.blocked == 1))");
+ "(!ct.trk || !ct.est"
+ " || (ct.est && ct_label.blocked == 1))");
if (!strcmp(acl->action, "reject")) {
build_reject_acl_rules(od, lflows, stage, acl, &match,
&actions);
diff --git a/tests/ovn.at b/tests/ovn.at
index 0cab187a3..e280a5ca3 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -12392,6 +12392,212 @@ AT_CHECK([cat 2.packets], [0], [])
AT_CLEANUP
+# 3 hypervisors, one logical switch, 3 logical ports per hypervisor
+AT_SETUP([ovn -- L2 Drop and Allow ACL w/ Stateful ACL])
+AT_SKIP_IF([test $HAVE_PYTHON = no])
+ovn_start
+
+# Create hypervisors hv[123].
+# Add vif11 to hv1, vif21 to hv2, vif31 to hv3.
+# Add all of the vifs to a single logical switch lsw0.
Re: [ovs-dev] L2 acl on the logical switch
t_label.blocked == 1))");+
"(!ct.trk || !ct.est || (ct.est && ct_label.blocked == 1))"); if
(!strcmp(acl->action, "reject")) { build_reject_acl_rules(od,
lflows, stage, acl, &match, &actions);
with this the rules above look like:
lflow: table=6 (ls_in_acl ), priority=3000 , match=(!ct.new &&
ct.est && !ct.rpl && ct_label.blocked == 0 && (inport == "ls1-vm3" && ip)),
action=(next;) table=6 (ls_in_acl ), priority=3000 ,
match=(((ct.new && !ct.est) || (!ct.new && ct.est && !ct.rpl &&
ct_label.blocked == 1)) && (inport == "ls1-vm3" && ip)), action=(reg0[1] = 1;
next;) table=4 (ls_out_acl ), priority=33767, match=((!ct.trk ||
!ct.est || (ct.est && ct_label.blocked == 1)) && (outport == @l2pg && eth.src
== $macs)), action=(/* drop */) table=4 (ls_out_acl ),
priority=33767, match=(ct.est && ct_label.blocked == 0 && (outport == @l2pg &&
eth.src == $macs)), action=(ct_commit(ct_label=1/1); /* drop */)
dpctl rules:
recirc_id(0),in_port(vm1),ct_state(-new-est-rel-rpl-inv-trk),ct_label(0/0x1),eth(src=02:ac:10:ff:00:11,dst=01:00:00:00:00:00/01:00:00:00:00:00),eth_type(0x0806),arp(sha=02:ac:10:ff:00:11),
packets:1, bytes:42, used:0.548s, actions:vm3
ARP doesn't resolve. Using the arbit packet with of ether type 0x7a05
recirc_id(0),in_port(vm1),ct_state(-new-est-rel-rpl-inv-trk),ct_label(0/0x1),eth(src=02:ac:10:ff:00:11,dst=02:ac:10:ff:00:22),eth_type(0x7a05),
packets:0, bytes:0, used:never, actions:drop
Let me know if I am missing anything, else I can look at proceeding with the
above change.
thanks,
-venu
On Friday, November 8, 2019, 03:09:37 AM PST, Numan Siddique
wrote:
Your email is really cluttered.
Could you please send again with plain text ?
Thanks
Numan
On Fri, Nov 8, 2019 at 6:22 AM venugopal iyer via dev
wrote:
>
> [Pardon the length of the mail; have left out the ofctl flows, but if it
> helps i can send them too]
> Assuming : logical Switch (ls1) with
> ls1_vm1 : 02:ac:10:ff:00:11/172.16.255.11
> ls1_vm2 : 02:ac:10:ff:00:22/172.16.255.22
> ls1_vm3 : 02:ac:10:ff:00:33/172.16.255.33
> Looking to : have MAC ACLs between vm1 and vm2 so they can't talk to
> each other.
> Using: # ovn-nbctl create Address_Set name=macs
> addresses=\"02:ac:10:ff:00:11\",\"02:ac:10:ff:00:22\" # ovn-nbctl
> create Port_Group name=l2pg # ovn-nbctl add Port_Group l2pg ports
> # ovn-nbctl add Port_Group l2pg ports #
> ovn-nbctl acl-add ls1 to-lport 32767 "outport == @l2pg && eth.src == \$macs"
> drop
> What we are seeing: The above, by itself, will limit L3 and L2 between
> vm1 and vm2, but not vm3 (as expected)
> lflow: table=4 (ls_out_acl ),
> priority=33767, match=(outport == @l2pg && eth.src == $macs), action=(/* drop
> */)
> dpflow (sending a arbit packet of ether type 0x7a05):--
>
> (0),in_port(vm1),eth(src=02:ac:10:ff:00:11,dst=02:ac:10:ff:00:22),eth_type(0x7a05),
> packets:0, bytes:0, used:never, actions:drop
> However, if there is a stateful rule, e.g.: # ovn-nbctl acl-add ls1
> from-lport 2000 "inport == \"ls1-vm3\" && ip" allow-related
> Then, the behavior differs
> lflow:--- table=6 (ls_in_acl ),
> priority=3000 , match=(!ct.new && ct.est && !ct.rpl && ct_label.blocked == 0
> && (inport == "ls1-vm3" && ip)), action=(next;) table=6 (ls_in_acl
> ), priority=3000 , match=(((ct.new && !ct.est) || (!ct.new && ct.est
> && !ct.rpl && ct_label.blocked == 1)) && (inport == "ls1-vm3" && ip)),
> action=(reg0[1] = 1; next;) table=4 (ls_out_acl ),
> priority=33767, match=((!ct.est || (ct.est && ct_label.blocked == 1)) &&
> (outport == @l2pg && eth.src == $macs)), action=(/* drop */) table=4
> (ls_out_acl ), priority=33767, match=(ct.est && ct_label.blocked == 0
> && (outport == @l2pg && eth.src == $macs)), action=(ct_commit(ct_label=1/1);
> /* drop */)
> *Note: the condition !ct.new && ct.est && !ct.rpl && ct_label.blocked ==
> 0**
> This does block L3 traffic:
> dpctl flow for ICMP-
> recirc_id(0),in_por
[ovs-dev] L2 acl on the logical switch
[Pardon the length of the mail; have left out the ofctl flows, but if it helps
i can send them too]
Assuming : logical Switch (ls1) with
ls1_vm1 : 02:ac:10:ff:00:11/172.16.255.11
ls1_vm2 : 02:ac:10:ff:00:22/172.16.255.22
ls1_vm3 : 02:ac:10:ff:00:33/172.16.255.33
Looking to : have MAC ACLs between vm1 and vm2 so they can't talk to
each other.
Using: # ovn-nbctl create Address_Set name=macs
addresses=\"02:ac:10:ff:00:11\",\"02:ac:10:ff:00:22\" # ovn-nbctl create
Port_Group name=l2pg # ovn-nbctl add Port_Group l2pg ports
# ovn-nbctl add Port_Group l2pg ports # ovn-nbctl acl-add
ls1 to-lport 32767 "outport == @l2pg && eth.src == \$macs" drop
What we are seeing: The above, by itself, will limit L3 and L2 between vm1
and vm2, but not vm3 (as expected)
lflow: table=4 (ls_out_acl ),
priority=33767, match=(outport == @l2pg && eth.src == $macs), action=(/* drop
*/)
dpflow (sending a arbit packet of ether type 0x7a05):--
(0),in_port(vm1),eth(src=02:ac:10:ff:00:11,dst=02:ac:10:ff:00:22),eth_type(0x7a05),
packets:0, bytes:0, used:never, actions:drop
However, if there is a stateful rule, e.g.: # ovn-nbctl acl-add ls1
from-lport 2000 "inport == \"ls1-vm3\" && ip" allow-related
Then, the behavior differs
lflow:--- table=6 (ls_in_acl ), priority=3000
, match=(!ct.new && ct.est && !ct.rpl && ct_label.blocked == 0 && (inport ==
"ls1-vm3" && ip)), action=(next;) table=6 (ls_in_acl ),
priority=3000 , match=(((ct.new && !ct.est) || (!ct.new && ct.est && !ct.rpl &&
ct_label.blocked == 1)) && (inport == "ls1-vm3" && ip)), action=(reg0[1] = 1;
next;) table=4 (ls_out_acl ), priority=33767, match=((!ct.est
|| (ct.est && ct_label.blocked == 1)) && (outport == @l2pg && eth.src ==
$macs)), action=(/* drop */) table=4 (ls_out_acl ),
priority=33767, match=(ct.est && ct_label.blocked == 0 && (outport == @l2pg &&
eth.src == $macs)), action=(ct_commit(ct_label=1/1); /* drop */)
*Note: the condition !ct.new && ct.est && !ct.rpl && ct_label.blocked ==
0**
This does block L3 traffic:
dpctl flow for ICMP-
recirc_id(0),in_port(vm1),eth(src=02:ac:10:ff:00:11),eth_type(0x0800),ipv4(proto=1,frag=no),icmp(type=8/0xf8),
packets:6, bytes:588, used:0.864s, actions:ct(zone=1),recirc(0x1)
recirc_id(0x1),in_port(vm1),ct_state(+new-est-rel-rpl-inv+trk),ct_label(0/0x1),eth(dst=02:ac:10:ff:00:22),eth_type(0x0800),ipv4(proto=1,frag=no),icmp(type=8/0xf8),
packets:6, bytes:588, used:0.864s,
actions:ct(commit,zone=1,label=0/0x1),ct(zone=4),recirc(0x2)
recirc_id(0x2),in_port(vm1),ct_state(+new-est-rel-rpl-inv+trk),ct_label(0/0x1),eth(src=02:ac:10:ff:00:11),eth_type(0x0800),ipv4(frag=no),
packets:6, bytes:588, used:0.864s, actions:drop
recirc_id(0),in_port(vm1),ct_state(-new-est-rel-rpl-inv-trk),ct_label(0/0x1),eth(src=02:ac:10:ff:00:11,dst=02:ac:10:ff:00:22),eth_type(0x0806),arp(sha=02:ac:10:ff:00:11),
packets:0, bytes:0, used:never, actions:vm2
recirc_id(0),in_port(vm2),ct_state(-new-est-rel-rpl-inv-trk),ct_label(0/0x1),eth(src=02:ac:10:ff:00:22,dst=02:ac:10:ff:00:11),eth_type(0x0806),arp(sha=02:ac:10:ff:00:22),
packets:0, bytes:0, used:never, actions:vm1
But, it doesn't block L2, see the ARP going through, without the stateful rule
the ARP would have been dropped too.
Using the arbit packet:
recirc_id(0),in_port(vm1),ct_state(-new-est-rel-rpl-inv-trk),ct_label(0/0x1),eth(src=02:ac:10:ff:00:11,dst=02:ac:10:ff:00:22),eth_type(0x7a05),
packets:0, bytes:0, used:never, actions:vm2
From what i understand it is because of the condition "!ct.new && ct.est &&
!ct.rpl && ct_label.blocked == 0", which implies trk, I suppose, which makes
sense for IP packets, but probably not for non-IP packets, i.e. the rules
above have "-trk". Hence, I believe it gets through
Proposed changes:
If my understanding is right (and objective to remove the inconsistency between
when there are stateful rules and not, w.r.t. L2 packets), we couldhave the
condition as:
# git diffdiff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.cindex
6c6de2afd..b0f524531 100644--- a/ovn/northd/ovn-northd.c+++
b/ovn/northd/ovn-northd.c@@ -4191,10 +4191,13 @@ consider_acl(struct hmap
*lflows, struct ovn_datapath *od, * depending on whether the
connection was previously committed * to the connection tracker with
ct_commit. */ if (has_stateful) {- /* If the packet is not
part of an established connection, then- * we can simply
reject/drop it. */++ /* If the packet is not tracked or part of an
established connection, then+ * we can simply reject/drop it.+
Re: [ovs-dev] [PATCH] ovn-controller: Fix parsing of OVN tunnel IDs
On Wednesday, May 22, 2019, 1:05:30 AM PDT, Dumitru Ceara
wrote:
On Thu, May 16, 2019 at 10:59 PM venugopal iyer wrote:
>
> Hi, Dumitru:
>
> On Thursday, May 16, 2019, 2:13:41 AM CDT, Dumitru Ceara
> wrote:
>
>
> On Wed, May 15, 2019 at 10:30 PM venugopal iyer wrote:
> >
> > Thanks for fixing this! a comment below:
> >
> > On Monday, May 13, 2019, 6:31:28 AM CDT, Dumitru Ceara
> > wrote:
> >
> >
> > Add tunnel-id creation and parsing functions in encaps.h.
> >
> > Reported-at: https://bugzilla.redhat.com/1708131
> > Reported-by: Haidong Li
> > Fixes: b520ca7 ("Support for multiple VTEP in OVN")
> > Signed-off-by: Dumitru Ceara
> > ---
> > ovn/controller/bfd.c | 31 ---
> > ovn/controller/encaps.c | 87
> > -
> > ovn/controller/encaps.h | 6 +++
> > ovn/controller/ovn-controller.h | 7
> > ovn/controller/physical.c | 51 +---
> > ovn/controller/pinctrl.c | 4 +-
> > 6 files changed, 130 insertions(+), 56 deletions(-)
> >
> > diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
> > index d016e27..b6aef04 100644
> > --- a/ovn/controller/bfd.c
> > +++ b/ovn/controller/bfd.c
> > @@ -15,6 +15,7 @@
> >
> > #include
> > #include "bfd.h"
> > +#include "encaps.h"
> > #include "lport.h"
> > #include "ovn-controller.h"
> >
> > @@ -72,14 +73,16 @@ bfd_calculate_active_tunnels(const struct ovsrec_bridge
> > *br_int,
> > const char *id = smap_get(&port_rec->external_ids,
> > "ovn-chassis-id");
> > if (id) {
> > - char *chassis_name;
> > - char *save_ptr = NULL;
> > - char *tokstr = xstrdup(id);
> > - chassis_name = strtok_r(tokstr,
> > OVN_MVTEP_CHASSISID_DELIM, &save_ptr);
> > - if (chassis_name &&
> > !sset_contains(active_tunnels, chassis_name)) {
> > - sset_add(active_tunnels, chassis_name);
> > + char *chassis_name = NULL;
> > +
> > + if (encaps_tunnel_id_parse(id, &chassis_name,
> > + NULL)) {
> > + if (!sset_contains(active_tunnels,
> > + chassis_name)) {
> > + sset_add(active_tunnels, chassis_name);
> > + }
> > + free(chassis_name);
> > }
> > - free(tokstr);
> > }
> > }
> > }
> > @@ -193,17 +196,17 @@ bfd_run(const struct ovsrec_interface_table
> > *interface_table,
> > const char *tunnel_id = smap_get(&br_int->ports[k]->external_ids,
> > "ovn-chassis-id");
> > if (tunnel_id) {
> > - char *chassis_name;
> > - char *save_ptr = NULL;
> > - char *tokstr = xstrdup(tunnel_id);
> > + char *chassis_name = NULL;
> > char *port_name = br_int->ports[k]->name;
> >
> > sset_add(&tunnels, port_name);
> > - chassis_name = strtok_r(tokstr, OVN_MVTEP_CHASSISID_DELIM,
> > &save_ptr);
> > - if (chassis_name && sset_contains(&bfd_chassis, chassis_name))
> > {
> > - sset_add(&bfd_ifaces, port_name);
> > +
> > + if (encaps_tunnel_id_parse(tunnel_id, &chassis_name, NULL)) {
> > + if (sset_contains(&bfd_chassis, chassis_name)) {
> > + sset_add(&bfd_ifaces, port_name);
> > + }
> > + free(chassis_name);
> > }
> > - free(tokstr);
> > }
> > }
> >
> > diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
> > index dcf7810..d467540 100644
> > --- a/ovn/controller/encaps.c
> > +++ b/ovn/controller/encaps.c
> > @@ -26,6 +26,13 @@
> >
> > VLOG_DEFINE_THIS_MODULE(encaps);
> >
> > +/*
> > + * Given there could be multiple tunnels with different IPs to the same
> > + * chassis we annotate the ovn-chassis-id with
> > + * OVN_MVTEP_CHASSISID_DELIM.
> > + */
> > +#define OVN_MVTEP_CHASSISID_DELIM '@'
> > +
> > void
> > encaps_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> > {
> > @@ -78,6 +85,83 @@ tunnel_create_name(struct tunnel_ctx *tc, const char
> > *chassis_id)
> > return NULL;
> > }
> >
> > +/*
> > + * Returns a tunnel-id of the form 'chassis_id'-delimiter-'encap_ip'.
> > + */
> > +char *
> > +encaps_tunnel_id_create(const char *chassis_id, const char *encap_ip)
> > +{
> > + return xasprintf("%s%c%s", chassis_id, OVN_MVTEP_CHASSISID_DELIM,
> > + encap_ip);
> > +}
> > +
> > +/*
> > + * Parses a 'tunnel_id' of the form .
> > + * If the 'chassis_id' argument is not
Re: [ovs-dev] [PATCH] ovn-controller: Fix parsing of OVN tunnel IDs
Hi, Dumitru:
On Thursday, May 16, 2019, 2:13:41 AM CDT, Dumitru Ceara
wrote:
On Wed, May 15, 2019 at 10:30 PM venugopal iyer wrote:
>
> Thanks for fixing this! a comment below:
>
> On Monday, May 13, 2019, 6:31:28 AM CDT, Dumitru Ceara
> wrote:
>
>
> Add tunnel-id creation and parsing functions in encaps.h.
>
> Reported-at: https://bugzilla.redhat.com/1708131
> Reported-by: Haidong Li
> Fixes: b520ca7 ("Support for multiple VTEP in OVN")
> Signed-off-by: Dumitru Ceara
> ---
> ovn/controller/bfd.c | 31 ---
> ovn/controller/encaps.c | 87 -
> ovn/controller/encaps.h | 6 +++
> ovn/controller/ovn-controller.h | 7
> ovn/controller/physical.c | 51 +---
> ovn/controller/pinctrl.c | 4 +-
> 6 files changed, 130 insertions(+), 56 deletions(-)
>
> diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
> index d016e27..b6aef04 100644
> --- a/ovn/controller/bfd.c
> +++ b/ovn/controller/bfd.c
> @@ -15,6 +15,7 @@
>
> #include
> #include "bfd.h"
> +#include "encaps.h"
> #include "lport.h"
> #include "ovn-controller.h"
>
> @@ -72,14 +73,16 @@ bfd_calculate_active_tunnels(const struct ovsrec_bridge
> *br_int,
> const char *id = smap_get(&port_rec->external_ids,
> "ovn-chassis-id");
> if (id) {
> - char *chassis_name;
> - char *save_ptr = NULL;
> - char *tokstr = xstrdup(id);
> - chassis_name = strtok_r(tokstr,
> OVN_MVTEP_CHASSISID_DELIM, &save_ptr);
> - if (chassis_name &&
> !sset_contains(active_tunnels, chassis_name)) {
> - sset_add(active_tunnels, chassis_name);
> + char *chassis_name = NULL;
> +
> + if (encaps_tunnel_id_parse(id, &chassis_name,
> + NULL)) {
> + if (!sset_contains(active_tunnels,
> + chassis_name)) {
> + sset_add(active_tunnels, chassis_name);
> + }
> + free(chassis_name);
> }
> - free(tokstr);
> }
> }
> }
> @@ -193,17 +196,17 @@ bfd_run(const struct ovsrec_interface_table
> *interface_table,
> const char *tunnel_id = smap_get(&br_int->ports[k]->external_ids,
> "ovn-chassis-id");
> if (tunnel_id) {
> - char *chassis_name;
> - char *save_ptr = NULL;
> - char *tokstr = xstrdup(tunnel_id);
> + char *chassis_name = NULL;
> char *port_name = br_int->ports[k]->name;
>
> sset_add(&tunnels, port_name);
> - chassis_name = strtok_r(tokstr, OVN_MVTEP_CHASSISID_DELIM,
> &save_ptr);
> - if (chassis_name && sset_contains(&bfd_chassis, chassis_name)) {
> - sset_add(&bfd_ifaces, port_name);
> +
> + if (encaps_tunnel_id_parse(tunnel_id, &chassis_name, NULL)) {
> + if (sset_contains(&bfd_chassis, chassis_name)) {
> + sset_add(&bfd_ifaces, port_name);
> + }
> + free(chassis_name);
> }
> - free(tokstr);
> }
> }
>
> diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
> index dcf7810..d467540 100644
> --- a/ovn/controller/encaps.c
> +++ b/ovn/controller/encaps.c
> @@ -26,6 +26,13 @@
>
> VLOG_DEFINE_THIS_MODULE(encaps);
>
> +/*
> + * Given there could be multiple tunnels with different IPs to the same
> + * chassis we annotate the ovn-chassis-id with
> + * OVN_MVTEP_CHASSISID_DELIM.
> + */
> +#define OVN_MVTEP_CHASSISID_DELIM '@'
> +
> void
> encaps_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> {
> @@ -78,6 +85,83 @@ tunnel_create_name(struct tunnel_ctx *tc, const char
> *chassis_id)
> return NULL;
> }
>
> +/*
> + * Returns a tunnel-id of the form 'chassis_id'-delimiter-'encap_ip'.
> + */
> +char *
> +encaps_tunnel_id_create(const char *chassis_id, const char *encap_ip)
> +{
> + return xasprintf("%s%c%s", chassis_id, OVN_MVTEP_CHASSISID_DELIM,
> + encap_ip);
> +}
> +
> +/*
> + * Parses a 'tunnel_id' of the form .
> + * If the 'chassis_id' argument is not NULL the function will allocate memory
> + * and store the chassis-id part of the tunnel-id at '*chassis_id'.
> + * If the 'encap_ip' argument is not NULL the function will allocate memory
> + * and store the encapsulation IP part of the tunnel-id at '*encap_ip'.
> + */
> +bool encaps_tunnel_id_parse(const char *tunnel_id, char **chassis_id,
> + char **encap_ip)
> +{
Re: [ovs-dev] [PATCH] ovn-controller: Fix parsing of OVN tunnel IDs
Thanks for fixing this! a comment below:
On Monday, May 13, 2019, 6:31:28 AM CDT, Dumitru Ceara
wrote:
Add tunnel-id creation and parsing functions in encaps.h.
Reported-at: https://bugzilla.redhat.com/1708131
Reported-by: Haidong Li
Fixes: b520ca7 ("Support for multiple VTEP in OVN")
Signed-off-by: Dumitru Ceara
---
ovn/controller/bfd.c | 31 ---
ovn/controller/encaps.c | 87 -
ovn/controller/encaps.h | 6 +++
ovn/controller/ovn-controller.h | 7
ovn/controller/physical.c | 51 +---
ovn/controller/pinctrl.c | 4 +-
6 files changed, 130 insertions(+), 56 deletions(-)
diff --git a/ovn/controller/bfd.c b/ovn/controller/bfd.c
index d016e27..b6aef04 100644
--- a/ovn/controller/bfd.c
+++ b/ovn/controller/bfd.c
@@ -15,6 +15,7 @@
#include
#include "bfd.h"
+#include "encaps.h"
#include "lport.h"
#include "ovn-controller.h"
@@ -72,14 +73,16 @@ bfd_calculate_active_tunnels(const struct ovsrec_bridge
*br_int,
const char *id = smap_get(&port_rec->external_ids,
"ovn-chassis-id");
if (id) {
- char *chassis_name;
- char *save_ptr = NULL;
- char *tokstr = xstrdup(id);
- chassis_name = strtok_r(tokstr,
OVN_MVTEP_CHASSISID_DELIM, &save_ptr);
- if (chassis_name && !sset_contains(active_tunnels,
chassis_name)) {
- sset_add(active_tunnels, chassis_name);
+ char *chassis_name = NULL;
+
+ if (encaps_tunnel_id_parse(id, &chassis_name,
+ NULL)) {
+ if (!sset_contains(active_tunnels,
+ chassis_name)) {
+ sset_add(active_tunnels, chassis_name);
+ }
+ free(chassis_name);
}
- free(tokstr);
}
}
}
@@ -193,17 +196,17 @@ bfd_run(const struct ovsrec_interface_table
*interface_table,
const char *tunnel_id = smap_get(&br_int->ports[k]->external_ids,
"ovn-chassis-id");
if (tunnel_id) {
- char *chassis_name;
- char *save_ptr = NULL;
- char *tokstr = xstrdup(tunnel_id);
+ char *chassis_name = NULL;
char *port_name = br_int->ports[k]->name;
sset_add(&tunnels, port_name);
- chassis_name = strtok_r(tokstr, OVN_MVTEP_CHASSISID_DELIM,
&save_ptr);
- if (chassis_name && sset_contains(&bfd_chassis, chassis_name)) {
- sset_add(&bfd_ifaces, port_name);
+
+ if (encaps_tunnel_id_parse(tunnel_id, &chassis_name, NULL)) {
+ if (sset_contains(&bfd_chassis, chassis_name)) {
+ sset_add(&bfd_ifaces, port_name);
+ }
+ free(chassis_name);
}
- free(tokstr);
}
}
diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
index dcf7810..d467540 100644
--- a/ovn/controller/encaps.c
+++ b/ovn/controller/encaps.c
@@ -26,6 +26,13 @@
VLOG_DEFINE_THIS_MODULE(encaps);
+/*
+ * Given there could be multiple tunnels with different IPs to the same
+ * chassis we annotate the ovn-chassis-id with
+ * OVN_MVTEP_CHASSISID_DELIM.
+ */
+#define OVN_MVTEP_CHASSISID_DELIM '@'
+
void
encaps_register_ovs_idl(struct ovsdb_idl *ovs_idl)
{
@@ -78,6 +85,83 @@ tunnel_create_name(struct tunnel_ctx *tc, const char
*chassis_id)
return NULL;
}
+/*
+ * Returns a tunnel-id of the form 'chassis_id'-delimiter-'encap_ip'.
+ */
+char *
+encaps_tunnel_id_create(const char *chassis_id, const char *encap_ip)
+{
+ return xasprintf("%s%c%s", chassis_id, OVN_MVTEP_CHASSISID_DELIM,
+ encap_ip);
+}
+
+/*
+ * Parses a 'tunnel_id' of the form .
+ * If the 'chassis_id' argument is not NULL the function will allocate memory
+ * and store the chassis-id part of the tunnel-id at '*chassis_id'.
+ * If the 'encap_ip' argument is not NULL the function will allocate memory
+ * and store the encapsulation IP part of the tunnel-id at '*encap_ip'.
+ */
+bool encaps_tunnel_id_parse(const char *tunnel_id, char **chassis_id,
+ char **encap_ip)
+{
+ const char *match = strchr(tunnel_id, OVN_MVTEP_CHASSISID_DELIM);
+
+ if (!match) {
+ return false;
+ }
+
+ if (chassis_id) {
+ size_t chassis_id_len = (match - tunnel_id);
+
+ *chassis_id = xmemdup0(tunnel_id, chassis_id_len);
+ }
+
+ /* Consume the tunnel-id delimiter. */
+ match++;
+
+ if (encap_ip
Re: [ovs-dev] [ovs-discuss] Geneve remote_ip as flow for OVN hosts
Hi, Ben: Agreed; mid/late next week should work for a meeting, will check with you about availability/logistics. thanks! -venu On Wednesday, December 12, 2018, 12:50:41 PM PST, Ben Pfaff wrote: If I'm not mistaken, we briefly discussed this at ovscon. It seems to me that this is a fairly complicated issue and proposal, and it might benefit from in-person discussion. I seem to recall that you are local to the Bay Area, and, if so, do you think we could take some time, perhaps next week, to have a meeting over it? Otherwise, I will continue to study it. On Thu, Nov 29, 2018 at 05:40:45PM +, venugopal iyer wrote: > Sorry for the resend, I am not sure how the pictures will render in the text >doc, so am attaching the PDF too. > thanks, > -venu > > On Thursday, November 29, 2018, 9:26:54 AM PST, venugopal iyer > wrote: > > Thanks, Ben. > > Sorry for the delay. Please find attached a draft design proposal and let me > know your comments etc. I did some quick > prototyping to check for feasibility too; I can share that, if it helps. > Note, the document is a draft and, I admit, there might be things that I > haven't thought about/through, or missed. I am > attaching a text doc, assuming it might be easier, but if you'd like it in a > different format, please let me know. > > thanks! > -venu > > On Wednesday, October 31, 2018, 10:30:23 AM PDT, Ben Pfaff >wrote: > > Honestly the best thing to do is probably to propose a design or, if > it's simple enough, to send a patch. That will probably be more > effective at sparking a discussion. > > On Wed, Oct 31, 2018 at 03:33:48PM +, venugopal iyer wrote: > > Hi: > > Just wanted to check if folks had any thoughts on the use case Girish > > outlined below. We do have > > a real use case for this and are interested in looking at options for > > supporting more than one VTEP IP.It is currently a limitation for us, > > wanted to know if there are similar use cases folks are looking > > at/interested in addressing. > > > > thanks, > > -venu > > > > On Thursday, September 6, 2018, 9:19:01 AM PDT, venugopal iyer via dev > > wrote: > > > > Would it be possible for the association to > >be made > > when the logical port is instantiated on a node? and relayed on to the SB by > > the controller, e.g. assuming a mechanism to specify/determine a physical > > port mapping for a > > logical port for a VM. The mappings can be > > specified as > > configuration on the chassis. In the absence of physical port information > > for > > a logical port/VM, I suppose we could default to an encap-ip. > > > > > > just a thought, > > -venu > > On Wednesday, September 5, 2018, 2:03:35 PM PDT, Ben Pfaff > > wrote: > > > > How would OVN know which IP to use for a given logical port on a > > chassis? > > > > I think that the "multiple tunnel encapsulations" is meant to cover, > > say, Geneve vs. STT vs. VXLAN, not the case you have in mind. > > > > On Wed, Sep 05, 2018 at 09:50:32AM -0700, Girish Moodalbail wrote: > > > Hello all, > > > > > > I would like to add more context here. In the diagram below > > > > > > +--+ > > > |ovn-host | > > > | | > > > | | > > > | +-+| > > > | | br-int || > > > | ++-+--+| > > > | | | | > > > | +--v-+ +---v+ | > > > | | geneve | | geneve | | > > > | +--+-+ +---++ | > > > | | | | > > > | +-v+ +--v---+ | > > > | | IP0 | | IP1 | | > > > | +--+ +--+ | > > > +--+ eth0 +-+ eth1 +---+ > > > +--+ +--+ > > > > > > eth0 and eth are, say, in its own physical segments. The VMs that are > > > instantiated in the above ovn-host will have multiple interfaces and each > > > of those interface need to be on a different Geneve VTEP. > > > > > > I think the following entry in OVN TODOs ( > > > https://github.com/openvswitch/ovs/blob/master/ovn/TODO.rst) > > > > > > ---8<--8<--- > > > Support multiple tunnel encapsulations in Chassis. > > &
Re: [ovs-dev] [ovs-discuss] Geneve remote_ip as flow for OVN hosts
Sorry for the resend, I am not sure how the pictures will render in the text doc, so am attaching the PDF too. thanks, -venu On Thursday, November 29, 2018, 9:26:54 AM PST, venugopal iyer wrote: Thanks, Ben. Sorry for the delay. Please find attached a draft design proposal and let me know your comments etc. I did some quick prototyping to check for feasibility too; I can share that, if it helps. Note, the document is a draft and, I admit, there might be things that I haven't thought about/through, or missed. I am attaching a text doc, assuming it might be easier, but if you'd like it in a different format, please let me know. thanks! -venu On Wednesday, October 31, 2018, 10:30:23 AM PDT, Ben Pfaff wrote: Honestly the best thing to do is probably to propose a design or, if it's simple enough, to send a patch. That will probably be more effective at sparking a discussion. On Wed, Oct 31, 2018 at 03:33:48PM +, venugopal iyer wrote: > Hi: > Just wanted to check if folks had any thoughts on the use case Girish > outlined below. We do have > a real use case for this and are interested in looking at options for > supporting more than one VTEP IP.It is currently a limitation for us, wanted > to know if there are similar use cases folks are looking at/interested in > addressing. > > thanks, > -venu > > On Thursday, September 6, 2018, 9:19:01 AM PDT, venugopal iyer via dev > wrote: > > Would it be possible for the association to be >made > when the logical port is instantiated on a node? and relayed on to the SB by > the controller, e.g. assuming a mechanism to specify/determine a physical > port mapping for a > logical port for a VM. The mappings can be > specified as > configuration on the chassis. In the absence of physical port information for > a logical port/VM, I suppose we could default to an encap-ip. > > > just a thought, > -venu > On Wednesday, September 5, 2018, 2:03:35 PM PDT, Ben Pfaff > wrote: > > How would OVN know which IP to use for a given logical port on a > chassis? > > I think that the "multiple tunnel encapsulations" is meant to cover, > say, Geneve vs. STT vs. VXLAN, not the case you have in mind. > > On Wed, Sep 05, 2018 at 09:50:32AM -0700, Girish Moodalbail wrote: > > Hello all, > > > > I would like to add more context here. In the diagram below > > > > +--+ > > |ovn-host | > > | | > > | | > > | +-+| > > | | br-int || > > | ++-+--+| > > | | | | > > | +--v-+ +---v+ | > > | | geneve | | geneve | | > > | +--+-+ +---++ | > > | | | | > > | +-v+ +--v---+ | > > | | IP0 | | IP1 | | > > | +--+ +--+ | > > +--+ eth0 +-+ eth1 +---+ > > +--+ +--+ > > > > eth0 and eth are, say, in its own physical segments. The VMs that are > > instantiated in the above ovn-host will have multiple interfaces and each > > of those interface need to be on a different Geneve VTEP. > > > > I think the following entry in OVN TODOs ( > > https://github.com/openvswitch/ovs/blob/master/ovn/TODO.rst) > > > > ---8<--8<--- > > Support multiple tunnel encapsulations in Chassis. > > > > So far, both ovn-controller and ovn-controller-vtep only allow chassis to > > have one tunnel encapsulation entry. We should extend the implementation to > > support multiple tunnel encapsulations > > ---8<--8<--- > > > > captures the above requirement. Is that the case? > > > > Thanks again. > > > > Regards, > > ~Girish > > > > > > > > > > On Tue, Sep 4, 2018 at 3:00 PM Girish Moodalbail > > wrote: > > > > > Hello all, > > > > > > Is it possible to configure remote_ip as a 'flow' instead of an IP address > > > (i.e., setting ovn-encap-ip to a single IP address)? > > > > > > Today, we have one VTEP endpoint per OVN host and all the VMs that > > > connects to br-int on that OVN host are reachable behind this VTEP > > > endpoint. Is it possible to have multiple VTEP endpoints for a br-int > > > bridge and use Open Flow flows to select one of the VTEP endpoint? > >
Re: [ovs-dev] [ovs-discuss] Geneve remote_ip as flow for OVN hosts
Thanks, Ben. Sorry for the delay. Please find attached a draft design proposal and let me know your comments etc. I did some quick prototyping to check for feasibility too; I can share that, if it helps. Note, the document is a draft and, I admit, there might be things that I haven't thought about/through, or missed. I am attaching a text doc, assuming it might be easier, but if you'd like it in a different format, please let me know. thanks! -venu On Wednesday, October 31, 2018, 10:30:23 AM PDT, Ben Pfaff wrote: Honestly the best thing to do is probably to propose a design or, if it's simple enough, to send a patch. That will probably be more effective at sparking a discussion. On Wed, Oct 31, 2018 at 03:33:48PM +, venugopal iyer wrote: > Hi: > Just wanted to check if folks had any thoughts on the use case Girish > outlined below. We do have > a real use case for this and are interested in looking at options for > supporting more than one VTEP IP.It is currently a limitation for us, wanted > to know if there are similar use cases folks are looking at/interested in > addressing. > > thanks, > -venu > > On Thursday, September 6, 2018, 9:19:01 AM PDT, venugopal iyer via dev > wrote: > > Would it be possible for the association to be >made > when the logical port is instantiated on a node? and relayed on to the SB by > the controller, e.g. assuming a mechanism to specify/determine a physical > port mapping for a > logical port for a VM. The mappings can be > specified as > configuration on the chassis. In the absence of physical port information for > a logical port/VM, I suppose we could default to an encap-ip. > > > just a thought, > -venu > On Wednesday, September 5, 2018, 2:03:35 PM PDT, Ben Pfaff > wrote: > > How would OVN know which IP to use for a given logical port on a > chassis? > > I think that the "multiple tunnel encapsulations" is meant to cover, > say, Geneve vs. STT vs. VXLAN, not the case you have in mind. > > On Wed, Sep 05, 2018 at 09:50:32AM -0700, Girish Moodalbail wrote: > > Hello all, > > > > I would like to add more context here. In the diagram below > > > > +--+ > > |ovn-host | > > | | > > | | > > | +-+| > > | | br-int || > > | ++-+--+| > > | | | | > > | +--v-+ +---v+ | > > | | geneve | | geneve | | > > | +--+-+ +---++ | > > | | | | > > | +-v+ +--v---+ | > > | | IP0 | | IP1 | | > > | +--+ +--+ | > > +--+ eth0 +-+ eth1 +---+ > > +--+ +--+ > > > > eth0 and eth are, say, in its own physical segments. The VMs that are > > instantiated in the above ovn-host will have multiple interfaces and each > > of those interface need to be on a different Geneve VTEP. > > > > I think the following entry in OVN TODOs ( > > https://github.com/openvswitch/ovs/blob/master/ovn/TODO.rst) > > > > ---8<--8<--- > > Support multiple tunnel encapsulations in Chassis. > > > > So far, both ovn-controller and ovn-controller-vtep only allow chassis to > > have one tunnel encapsulation entry. We should extend the implementation to > > support multiple tunnel encapsulations > > ---8<--8<--- > > > > captures the above requirement. Is that the case? > > > > Thanks again. > > > > Regards, > > ~Girish > > > > > > > > > > On Tue, Sep 4, 2018 at 3:00 PM Girish Moodalbail > > wrote: > > > > > Hello all, > > > > > > Is it possible to configure remote_ip as a 'flow' instead of an IP address > > > (i.e., setting ovn-encap-ip to a single IP address)? > > > > > > Today, we have one VTEP endpoint per OVN host and all the VMs that > > > connects to br-int on that OVN host are reachable behind this VTEP > > > endpoint. Is it possible to have multiple VTEP endpoints for a br-int > > > bridge and use Open Flow flows to select one of the VTEP endpoint? > > > > > > > > > +--+ > > > |ovn-host | > > > | | > > > |
Re: [ovs-dev] [ovs-discuss] Geneve remote_ip as flow for OVN hosts
Hi: Just wanted to check if folks had any thoughts on the use case Girish outlined below. We do have a real use case for this and are interested in looking at options for supporting more than one VTEP IP.It is currently a limitation for us, wanted to know if there are similar use cases folks are looking at/interested in addressing. thanks, -venu On Thursday, September 6, 2018, 9:19:01 AM PDT, venugopal iyer via dev wrote: Would it be possible for the association to be made when the logical port is instantiated on a node? and relayed on to the SB by the controller, e.g. assuming a mechanism to specify/determine a physical port mapping for a logical port for a VM. The mappings can be specified as configuration on the chassis. In the absence of physical port information for a logical port/VM, I suppose we could default to an encap-ip. just a thought, -venu On Wednesday, September 5, 2018, 2:03:35 PM PDT, Ben Pfaff wrote: How would OVN know which IP to use for a given logical port on a chassis? I think that the "multiple tunnel encapsulations" is meant to cover, say, Geneve vs. STT vs. VXLAN, not the case you have in mind. On Wed, Sep 05, 2018 at 09:50:32AM -0700, Girish Moodalbail wrote: > Hello all, > > I would like to add more context here. In the diagram below > > +--+ > |ovn-host | > | | > | | > | +-+| > | | br-int || > | ++-+--+| > | | | | > | +--v-+ +---v+ | > | | geneve | | geneve | | > | +--+-+ +---++ | > | | | | > | +-v+ +--v---+ | > | | IP0 | | IP1 | | > | +--+ +--+ | > +--+ eth0 +-+ eth1 +---+ > +--+ +--+ > > eth0 and eth are, say, in its own physical segments. The VMs that are > instantiated in the above ovn-host will have multiple interfaces and each > of those interface need to be on a different Geneve VTEP. > > I think the following entry in OVN TODOs ( > https://github.com/openvswitch/ovs/blob/master/ovn/TODO.rst) > > ---8<--8<--- > Support multiple tunnel encapsulations in Chassis. > > So far, both ovn-controller and ovn-controller-vtep only allow chassis to > have one tunnel encapsulation entry. We should extend the implementation to > support multiple tunnel encapsulations > ---8<--8<--- > > captures the above requirement. Is that the case? > > Thanks again. > > Regards, > ~Girish > > > > > On Tue, Sep 4, 2018 at 3:00 PM Girish Moodalbail > wrote: > > > Hello all, > > > > Is it possible to configure remote_ip as a 'flow' instead of an IP address > > (i.e., setting ovn-encap-ip to a single IP address)? > > > > Today, we have one VTEP endpoint per OVN host and all the VMs that > > connects to br-int on that OVN host are reachable behind this VTEP > > endpoint. Is it possible to have multiple VTEP endpoints for a br-int > > bridge and use Open Flow flows to select one of the VTEP endpoint? > > > > > > +--+ > > |ovn-host | > > | | > > | | > > | +-+| > > | | br-int || > > | ++-+--+| > > | | | | > > | +--v-+ +---v+ | > > | | geneve | | geneve | | > > | +--+-+ +---++ | > > | | | | > > | +-v+ +--v---+ | > > | | IP0 | | IP1 | | > > | +--+ +--+ | > > +--+ eth0 +-+ eth1 +---+ > > +--+ +--+ > > > > Also, we don't want to bond eth0 and eth1 into a bond interface and then > > use bond's IP as VTEP endpoint. > > > > Thanks in advance, > > ~Girish > > > > > > > > > ___ > discuss mailing list > disc...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-discuss ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-discuss] Geneve remote_ip as flow for OVN hosts
Would it be possible for the association to be made when the logical port is instantiated on a node? and relayed on to the SB by the controller, e.g. assuming a mechanism to specify/determine a physical port mapping for a logical port for a VM. The mappings can be specified as configuration on the chassis. In the absence of physical port information for a logical port/VM, I suppose we could default to an encap-ip. just a thought, -venu On Wednesday, September 5, 2018, 2:03:35 PM PDT, Ben Pfaff wrote: How would OVN know which IP to use for a given logical port on a chassis? I think that the "multiple tunnel encapsulations" is meant to cover, say, Geneve vs. STT vs. VXLAN, not the case you have in mind. On Wed, Sep 05, 2018 at 09:50:32AM -0700, Girish Moodalbail wrote: > Hello all, > > I would like to add more context here. In the diagram below > > +--+ > |ovn-host | > | | > | | > | +-+| > | | br-int || > | ++-+--+| > | | | | > | +--v-+ +---v+ | > | | geneve | | geneve | | > | +--+-+ +---++ | > | | | | > | +-v+ +--v---+ | > | | IP0 | | IP1 | | > | +--+ +--+ | > +--+ eth0 +-+ eth1 +---+ > +--+ +--+ > > eth0 and eth are, say, in its own physical segments. The VMs that are > instantiated in the above ovn-host will have multiple interfaces and each > of those interface need to be on a different Geneve VTEP. > > I think the following entry in OVN TODOs ( > https://github.com/openvswitch/ovs/blob/master/ovn/TODO.rst) > > ---8<--8<--- > Support multiple tunnel encapsulations in Chassis. > > So far, both ovn-controller and ovn-controller-vtep only allow chassis to > have one tunnel encapsulation entry. We should extend the implementation to > support multiple tunnel encapsulations > ---8<--8<--- > > captures the above requirement. Is that the case? > > Thanks again. > > Regards, > ~Girish > > > > > On Tue, Sep 4, 2018 at 3:00 PM Girish Moodalbail > wrote: > > > Hello all, > > > > Is it possible to configure remote_ip as a 'flow' instead of an IP address > > (i.e., setting ovn-encap-ip to a single IP address)? > > > > Today, we have one VTEP endpoint per OVN host and all the VMs that > > connects to br-int on that OVN host are reachable behind this VTEP > > endpoint. Is it possible to have multiple VTEP endpoints for a br-int > > bridge and use Open Flow flows to select one of the VTEP endpoint? > > > > > > +--+ > > |ovn-host | > > | | > > | | > > | +-+| > > | | br-int || > > | ++-+--+| > > | | | | > > | +--v-+ +---v+ | > > | | geneve | | geneve | | > > | +--+-+ +---++ | > > | | | | > > | +-v+ +--v---+ | > > | | IP0 | | IP1 | | > > | +--+ +--+ | > > +--+ eth0 +-+ eth1 +---+ > > +--+ +--+ > > > > Also, we don't want to bond eth0 and eth1 into a bond interface and then > > use bond's IP as VTEP endpoint. > > > > Thanks in advance, > > ~Girish > > > > > > > > > ___ > discuss mailing list > disc...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-discuss ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
