Re: [ovs-dev] [PATCH] ovn: Add a new logical switch port type - 'virtual'
Bleep bloop. Greetings Numan Siddique, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 236 characters long (recommended limit is 79) #497 FILE: ovn/northd/ovn-northd.8.xml:530: inport == P !is_chassis_resident(V) ((arp.op == 1 arp.spa == VIP arp.tpa == VIP) || (arp.op == 2 arp.spa == VIP)) Lines checked: 1409, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@bytheb.org Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ovn: Add a new logical switch port type - 'virtual'
From: Numan Siddique
This new type is added for the following reasons:
- When a load balancer is created in an OpenStack deployment with Octavia
service, it creates a logical port 'VIP' for the virtual ip.
- This logical port is not bound to any VIF.
- Octavia service creates a service VM (with another logical port 'P' which
belongs to the same logical switch)
- The virtual ip 'VIP' is configured on this service VM.
- This service VM provides the load balancing for the VIP with the configured
backend IPs.
- Octavia service can be configured to create few service VMs with
active-standby mode
with the active VM configured with the VIP. The VIP can move between
these service nodes.
Presently there are few problems:
- When a floating ip (externally reachable IP) is associated to the VIP and if
the compute nodes have external connectivity then the external traffic
cannot
reach the VIP using the floating ip as the VIP logical port would be down.
dnat_and_snat entry in NAT table for this vip will have 'external_mac' and
'logical_port' configured.
- The only way to make it work is to clear the 'external_mac' entry so that
the gateway chassis does the DNAT for the VIP.
To solve these problems, this patch proposes a new logical port type - virtual.
CMS when creating the logical port for the VIP, should
- set the type as 'virtual'
- configure the VIP in the options - Logical_Switch_Port.options:virtual-ip
- And set the virtual parents in the options
Logical_Switch_Port.options:virtual-parents.
These virtual parents are the one which can be configured with the VIP.
If suppose the virtual_ip is configured to 10.0.0.10 on a virtual logical port
'sw0-vip'
and the virtual_parents are set to - [sw0-p1, sw0-p2] then below logical flows
are added in the
lsp_in_arp_rsp logical switch pipeline
- table=11(ls_in_arp_rsp), priority=100,
match=(inport == "sw0-p1" && !is_chassis_resident("sw0-vip") &&
((arp.op == 1 && arp.spa == 10.0.0.10 && arp.tpa == 10.0.0.10) ||
(arp.op == 2 && arp.spa == 10.0.0.10))),
action=(bind_vport("sw0-vip", inport); next;)
- table=11(ls_in_arp_rsp), priority=100,
match=(inport == "sw0-p2" && !is_chassis_resident("sw0-vip") &&
((arp.op == 1 && arp.spa == 10.0.0.10 && arp.tpa == 10.0.0.10) ||
(arp.op == 2 && arp.spa == 10.0.0.10))),
action=(bind_vport("sw0-vip", inport); next;)
The action bind_vport will claim the logical port - sw0-vip on the chassis
where this action
is executed. Since the port - sw0-vip is claimed by a chassis, the
dnat_and_snat rule for
the VIP will be handled by the compute node.
Signed-off-by: Numan Siddique
---
v4 -> v5
===
* Rebased to master to resolve merge conflicts.
v3 -> v4
===
* Addressed the review comment and removed the code in northd which
referenced the Southbound db state while adding the logical flows. Instead
using the ovn match - is_chassis_resident() - which I should have used
it in the first place.
v2 -> v3
===
* Addressed the review comments from Ben - deleted the new columns -
virtual_ip and virtual_parents from Logical_Switch_Port and instead
is making use of options column for this purpose.
v1 -> v2
* In v1, was not updating the 'put_vport_binding' struct if it already
exists in the put_vport_bindings hmap in the function -
pinctrl_handle_bind_vport().
In v2 handled it.
* Improved the if else check in binding.c when releasing the lports.
include/ovn/actions.h | 18 ++-
ovn/controller/binding.c| 30 +++-
ovn/controller/pinctrl.c| 174
ovn/lib/actions.c | 60 +++
ovn/lib/ovn-util.c | 1 +
ovn/northd/ovn-northd.8.xml | 61 ++-
ovn/northd/ovn-northd.c | 306 +++-
ovn/ovn-nb.xml | 45 ++
ovn/ovn-sb.ovsschema| 6 +-
ovn/ovn-sb.xml | 46 ++
ovn/utilities/ovn-trace.c | 3 +
tests/ovn.at| 281 +
tests/test-ovn.c| 1 +
13 files changed, 945 insertions(+), 87 deletions(-)
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index f42bbc277..48c64f792 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -83,7 +83,8 @@ struct ovn_extend_table;
OVNACT(ND_NS, ovnact_nest)\
OVNACT(SET_METER, ovnact_set_meter) \
OVNACT(OVNFIELD_LOAD, ovnact_load)\
-OVNACT(CHECK_PKT_LARGER, ovnact_check_pkt_larger)
+OVNACT(CHECK_PKT_LARGER, ovnact_check_pkt_larger)\
+OVNACT(BIND_VPORT,ovnact_bind_vport)
/* enum ovnact_type, with a member OVNACT_ for each action. */
enum OVS_PACKED_ENUM ovnact_type {
@@ -318,6 +319,13 @@ struct ovnact_check_pkt_larger {
struct expr_field dst; /* 1-bit destination field. */
};
+/* OVNACT_BIND_VPORT. */
+struct
Re: [ovs-dev] [PATCH] ovn: Add a new logical switch port type - 'virtual'
On Sun, May 26, 2019 at 12:59 PM Numan Siddique wrote: > > > On Sat, May 25, 2019 at 2:29 AM Ben Pfaff wrote: > >> This commit adds new columns virtual_ip and virtual_parents in the >> Logical_Switch_Port table, which are used for the new 'virtual' port >> type. Most of the time, if a particular port type has type-specific >> options, they go in the "options" column. This has the advantage that >> we don't end up with lots of columns that are rarely used. It does have >> some disadvantages. For example, "options" can't be strong or weak >> references, and it requires some convention for having more than one >> value. virtual_ip and virtual_parents don't have the former issue, >> though virtual_parents does have the latter issue. Did you think >> through whether these should be options or new columns? >> > > Actually I didn't think through this. Thanks for pointing this out. I > think it > makes sense to have these as options. It would require a little bit of > parsing for virtual_parents - it has to be a comma separated string of > logical port names. I think that's reasonable as a logical > switch most likely will not have many ports of this new type. > > I will work on it and submit v2. > Done. v3 submitted - https://patchwork.ozlabs.org/patch/1105665/ Numan > Thanks > Numan > > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovn: Add a new logical switch port type - 'virtual'
On Sat, May 25, 2019 at 2:29 AM Ben Pfaff wrote: > This commit adds new columns virtual_ip and virtual_parents in the > Logical_Switch_Port table, which are used for the new 'virtual' port > type. Most of the time, if a particular port type has type-specific > options, they go in the "options" column. This has the advantage that > we don't end up with lots of columns that are rarely used. It does have > some disadvantages. For example, "options" can't be strong or weak > references, and it requires some convention for having more than one > value. virtual_ip and virtual_parents don't have the former issue, > though virtual_parents does have the latter issue. Did you think > through whether these should be options or new columns? > Actually I didn't think through this. Thanks for pointing this out. I think it makes sense to have these as options. It would require a little bit of parsing for virtual_parents - it has to be a comma separated string of logical port names. I think that's reasonable as a logical switch most likely will not have many ports of this new type. I will work on it and submit v2. Thanks Numan ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovn: Add a new logical switch port type - 'virtual'
This commit adds new columns virtual_ip and virtual_parents in the Logical_Switch_Port table, which are used for the new 'virtual' port type. Most of the time, if a particular port type has type-specific options, they go in the "options" column. This has the advantage that we don't end up with lots of columns that are rarely used. It does have some disadvantages. For example, "options" can't be strong or weak references, and it requires some convention for having more than one value. virtual_ip and virtual_parents don't have the former issue, though virtual_parents does have the latter issue. Did you think through whether these should be options or new columns? ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovn: Add a new logical switch port type - 'virtual'
Bleep bloop. Greetings Numan Siddique, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line is 190 characters long (recommended limit is 79) #491 FILE: ovn/northd/ovn-northd.8.xml:531: inport == P ((arp.op == 1 arp.spa == VIP arp.tpa == VIP) || (arp.op == 2 arp.spa == VIP)) Lines checked: 1440, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@bytheb.org Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ovn: Add a new logical switch port type - 'virtual'
From: Numan Siddique
This new type is added for the following reasons:
- When a load balancer is created in an OpenStack deployment with Octavia
service, it creates a logical port 'VIP' for the virtual ip.
- This logical port is not bound to any VIF.
- Octavia service creates a service VM (with another logical port 'P' which
belongs to the same logical switch)
- The virtual ip 'VIP' is configured on this service VM.
- This service VM provides the load balancing for the VIP with the configured
backend IPs.
- Octavia service can be configured to create few service VMs with
active-standby mode
with the active VM configured with the VIP. The VIP can move between
these service nodes.
Presently there are few problems:
- When a floating ip (externally reachable IP) is associated to the VIP and if
the compute nodes have external connectivity then the external traffic
cannot
reach the VIP using the floating ip as the VIP logical port would be down.
dnat_and_snat entry in NAT table for this vip will have 'external_mac' and
'logical_port' configured.
- The only way to make it work is to clear the 'external_mac' entry so that
the gateway chassis does the DNAT for the VIP.
To solve these problems, this patch proposes a new logical port type - virtual.
CMS when creating the logical port for the VIP, should
- set the type as 'virtual'
- configure the VIP in the newly added column Logical_Switch_Port.virtual_ip
- And set the virtual parents in the new added column
Logical_Switch_Port.virtual_parents.
These virtual parents are the one which can be configured wit the VIP.
If suppose the virtual_ip is configured to 10.0.0.10 on a virtual logical port
'sw0-vip'
and the virtual_parents are set to - [sw0-p1, sw0-p2] then below logical flows
are added in the
lsp_in_arp_rsp logical switch pipeline
- table=11(ls_in_arp_rsp), priority=100,
match=(inport == "sw0-p1" && ((arp.op == 1 && arp.spa == 10.0.0.10 &&
arp.tpa == 10.0.0.10) ||
(arp.op == 2 && arp.spa == 10.0.0.10))),
action=(bind_vport("sw0-vip", inport); next;)
- table=11(ls_in_arp_rsp), priority=100,
match=(inport == "sw0-p2" && ((arp.op == 1 && arp.spa == 10.0.0.10 &&
arp.tpa == 10.0.0.10) ||
(arp.op == 2 && arp.spa == 10.0.0.10))),
action=(bind_vport("sw0-vip", inport); next;)
The action bind_vport will claim the logical port - sw0-vip on the chassis
where this action
is executed. Since the port - sw0-vip is claimed by a chassis, the
dnat_and_snat rule for
the VIP will be handled by the compute node.
Signed-off-by: Numan Siddique
---
include/ovn/actions.h | 18 ++-
ovn/controller/binding.c| 21 ++-
ovn/controller/pinctrl.c| 173
ovn/lib/actions.c | 60 +++
ovn/lib/ovn-util.c | 1 +
ovn/northd/ovn-northd.8.xml | 62 +++-
ovn/northd/ovn-northd.c | 298 +-
ovn/ovn-nb.ovsschema| 8 +-
ovn/ovn-nb.xml | 40 +
ovn/ovn-sb.ovsschema| 6 +-
ovn/ovn-sb.xml | 46 ++
ovn/utilities/ovn-trace.c | 3 +
tests/ovn.at| 307
tests/test-ovn.c| 1 +
14 files changed, 958 insertions(+), 86 deletions(-)
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index e07ad9aa3..e1511ff06 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -83,7 +83,8 @@ struct ovn_extend_table;
OVNACT(ND_NS, ovnact_nest)\
OVNACT(SET_METER, ovnact_set_meter) \
OVNACT(OVNFIELD_LOAD, ovnact_load)\
-OVNACT(CHECK_PKT_LARGER, ovnact_check_pkt_larger)
+OVNACT(CHECK_PKT_LARGER, ovnact_check_pkt_larger)\
+OVNACT(BIND_VPORT,ovnact_bind_vport)
/* enum ovnact_type, with a member OVNACT_ for each action. */
enum OVS_PACKED_ENUM ovnact_type {
@@ -318,6 +319,13 @@ struct ovnact_check_pkt_larger {
struct expr_field dst; /* 1-bit destination field. */
};
+/* OVNACT_BIND_VPORT. */
+struct ovnact_bind_vport {
+struct ovnact ovnact;
+char *vport;
+struct expr_field vport_parent; /* Logical virtual port's port name. */
+};
+
/* Internal use by the helpers below. */
void ovnact_init(struct ovnact *, enum ovnact_type, size_t len);
void *ovnact_put(struct ofpbuf *, enum ovnact_type, size_t len);
@@ -486,6 +494,14 @@ enum action_opcode {
* The actions, in OpenFlow 1.3 format, follow the action_header.
*/
ACTION_OPCODE_ICMP4_ERROR,
+
+/* "bind_vport(vport, vport_parent)".
+ *
+ * 'vport' follows the action_header, in the format - 32-bit field.
+ * 'vport_parent' is passed through the packet metadata as
+ *MFF_LOG_INPORT.
+ */
+ACTION_OPCODE_BIND_VPORT,
};
/* Header. */
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
index
