[ovs-dev] [PATCH v4 0/5] netdev-offload-tc: Add support for the check_pkt_len action.
This series adds support for the datapath action check_pkt_len for TC offload. It also includes some offload self-tests. v2: - Add ACKs - Unified all the OVS_TRAFFIC_VSWITCHD_START macro's - Added section in the NEWS document v3: - Add ACKs - Was using TCA_CSUM_PARMS instead of TCA_POLICE_TBF v4: - Add ACKs - Use the existing dbinit-aux-args argument, rather than creating a new pre-vswitchd command option. - Removed ACKs for patch 4/5 Eelco Chaudron (5): netdev-offload-tc: Move flow_put action handling to isolated function. netdev-offload-tc: Move flower_to_match action handling to isolated function. netdev-offload-tc: Handle check_pkt_len datapath action. system-offloads-traffic: Properly initialize offload before testing. tests: Add check_pkt_len action test to system-offload-traffic. NEWS | 2 + lib/netdev-offload-tc.c | 844 +++ lib/tc.c | 455 +++-- lib/tc.h | 12 +- tests/ofproto-macros.at | 3 +- tests/system-kmod-macros.at | 4 +- tests/system-offloads-traffic.at | 422 +++- tests/system-tso-macros.at | 4 +- tests/system-userspace-macros.at | 4 +- 9 files changed, 1376 insertions(+), 374 deletions(-) ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 1/5] netdev-offload-tc: Move flow_put action handling to isolated function.
Move handling of the individual actions in the netdev_tc_flow_put()
function to a separate function that will make recursive action handling
easier.
Signed-off-by: Eelco Chaudron
Acked-by: Mike Pattrick
Acked-by: Roi Dayan
---
lib/netdev-offload-tc.c | 255 +--
1 file changed, 138 insertions(+), 117 deletions(-)
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index a41b62758..3c2e8f510 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -1564,6 +1564,139 @@ parse_match_ct_state_to_flower(struct tc_flower
*flower, struct match *match)
}
}
+static int
+netdev_tc_parse_nl_actions(struct netdev *netdev, struct tc_flower *flower,
+ struct offload_info *info,
+ const struct nlattr *actions, size_t actions_len,
+ bool *recirc_act)
+{
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
+const struct nlattr *nla;
+size_t left;
+
+NL_ATTR_FOR_EACH (nla, left, actions, actions_len) {
+struct tc_action *action;
+int err;
+
+if (flower->action_count >= TCA_ACT_MAX_NUM) {
+VLOG_DBG_RL(&rl, "Can only support %d actions", TCA_ACT_MAX_NUM);
+return EOPNOTSUPP;
+}
+
+action = &flower->actions[flower->action_count];
+
+if (nl_attr_type(nla) == OVS_ACTION_ATTR_OUTPUT) {
+odp_port_t port = nl_attr_get_odp_port(nla);
+struct netdev *outdev = netdev_ports_get(
+port, netdev_get_dpif_type(netdev));
+
+if (!outdev) {
+VLOG_DBG_RL(&rl, "Can't find netdev for output port %d", port);
+return ENODEV;
+}
+
+if (!netdev_flow_api_equals(netdev, outdev)) {
+VLOG_DBG_RL(&rl,
+"Flow API provider mismatch between ingress (%s) "
+"and egress (%s) ports",
+netdev_get_name(netdev), netdev_get_name(outdev));
+netdev_close(outdev);
+return EOPNOTSUPP;
+}
+
+action->out.ifindex_out = netdev_get_ifindex(outdev);
+if (action->out.ifindex_out < 0) {
+VLOG_DBG_RL(&rl,
+"Can't find ifindex for output port %s, error %d",
+netdev_get_name(outdev), action->out.ifindex_out);
+netdev_close(outdev);
+return -action->out.ifindex_out;
+}
+
+action->out.ingress = is_internal_port(netdev_get_type(outdev));
+action->type = TC_ACT_OUTPUT;
+flower->action_count++;
+netdev_close(outdev);
+} else if (nl_attr_type(nla) == OVS_ACTION_ATTR_PUSH_VLAN) {
+const struct ovs_action_push_vlan *vlan_push = nl_attr_get(nla);
+
+action->vlan.vlan_push_tpid = vlan_push->vlan_tpid;
+action->vlan.vlan_push_id = vlan_tci_to_vid(vlan_push->vlan_tci);
+action->vlan.vlan_push_prio = vlan_tci_to_pcp(vlan_push->vlan_tci);
+action->type = TC_ACT_VLAN_PUSH;
+flower->action_count++;
+} else if (nl_attr_type(nla) == OVS_ACTION_ATTR_POP_VLAN) {
+action->type = TC_ACT_VLAN_POP;
+flower->action_count++;
+} else if (nl_attr_type(nla) == OVS_ACTION_ATTR_PUSH_MPLS) {
+const struct ovs_action_push_mpls *mpls_push = nl_attr_get(nla);
+
+action->mpls.proto = mpls_push->mpls_ethertype;
+action->mpls.label = mpls_lse_to_label(mpls_push->mpls_lse);
+action->mpls.tc = mpls_lse_to_tc(mpls_push->mpls_lse);
+action->mpls.ttl = mpls_lse_to_ttl(mpls_push->mpls_lse);
+action->mpls.bos = mpls_lse_to_bos(mpls_push->mpls_lse);
+action->type = TC_ACT_MPLS_PUSH;
+flower->action_count++;
+} else if (nl_attr_type(nla) == OVS_ACTION_ATTR_POP_MPLS) {
+action->mpls.proto = nl_attr_get_be16(nla);
+action->type = TC_ACT_MPLS_POP;
+flower->action_count++;
+} else if (nl_attr_type(nla) == OVS_ACTION_ATTR_SET) {
+const struct nlattr *set = nl_attr_get(nla);
+const size_t set_len = nl_attr_get_size(nla);
+
+err = parse_put_flow_set_action(flower, action, set, set_len);
+if (err) {
+return err;
+}
+if (action->type == TC_ACT_ENCAP) {
+action->encap.tp_dst = info->tp_dst_port;
+action->encap.no_csum = !info->tunnel_csum_on;
+}
+} else if (nl_attr_type(nla) == OVS_ACTION_ATTR_SET_MASKED) {
+const struct nlattr *set = nl_attr_get(nla);
+const size_t set_len = nl_attr_get_size(nla);
+
+err = parse_put_flow_set_masked_action(flower, action, set,
+
[ovs-dev] [PATCH v4 2/5] netdev-offload-tc: Move flower_to_match action handling to isolated function.
Move handling of the individual actions in the parse_tc_flower_to_match()
function to a separate function that will make recursive action handling
easier.
Signed-off-by: Eelco Chaudron
Acked-by: Mike Pattrick
Acked-by: Roi Dayan
---
lib/netdev-offload-tc.c | 422 +--
1 file changed, 221 insertions(+), 201 deletions(-)
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index 3c2e8f510..f657af0fd 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -617,6 +617,226 @@ parse_tc_flower_terse_to_match(struct tc_flower *flower,
return 0;
}
+static int
+_parse_tc_flower_to_actions(struct tc_flower *flower, struct ofpbuf *buf,
+int start_index, int max_index)
+{
+struct tc_action *action;
+int i;
+
+if (max_index <= 0 || max_index > flower->action_count) {
+max_index = flower->action_count;
+}
+
+for (i = start_index; i < max_index; i++) {
+action = &flower->actions[i];
+
+switch (action->type) {
+case TC_ACT_VLAN_POP: {
+nl_msg_put_flag(buf, OVS_ACTION_ATTR_POP_VLAN);
+}
+break;
+case TC_ACT_VLAN_PUSH: {
+struct ovs_action_push_vlan *push;
+
+push = nl_msg_put_unspec_zero(buf, OVS_ACTION_ATTR_PUSH_VLAN,
+ sizeof *push);
+push->vlan_tpid = action->vlan.vlan_push_tpid;
+push->vlan_tci = htons(action->vlan.vlan_push_id
+ | (action->vlan.vlan_push_prio << 13)
+ | VLAN_CFI);
+}
+break;
+case TC_ACT_MPLS_POP: {
+nl_msg_put_be16(buf, OVS_ACTION_ATTR_POP_MPLS,
+action->mpls.proto);
+}
+break;
+case TC_ACT_MPLS_PUSH: {
+struct ovs_action_push_mpls *push;
+ovs_be32 mpls_lse = 0;
+
+flow_set_mpls_lse_label(&mpls_lse, action->mpls.label);
+flow_set_mpls_lse_tc(&mpls_lse, action->mpls.tc);
+flow_set_mpls_lse_ttl(&mpls_lse, action->mpls.ttl);
+flow_set_mpls_lse_bos(&mpls_lse, action->mpls.bos);
+
+push = nl_msg_put_unspec_zero(buf, OVS_ACTION_ATTR_PUSH_MPLS,
+ sizeof *push);
+push->mpls_ethertype = action->mpls.proto;
+push->mpls_lse = mpls_lse;
+}
+break;
+case TC_ACT_MPLS_SET: {
+size_t set_offset = nl_msg_start_nested(buf,
+OVS_ACTION_ATTR_SET);
+struct ovs_key_mpls *set_mpls;
+ovs_be32 mpls_lse = 0;
+
+flow_set_mpls_lse_label(&mpls_lse, action->mpls.label);
+flow_set_mpls_lse_tc(&mpls_lse, action->mpls.tc);
+flow_set_mpls_lse_ttl(&mpls_lse, action->mpls.ttl);
+flow_set_mpls_lse_bos(&mpls_lse, action->mpls.bos);
+
+set_mpls = nl_msg_put_unspec_zero(buf, OVS_KEY_ATTR_MPLS,
+ sizeof *set_mpls);
+set_mpls->mpls_lse = mpls_lse;
+nl_msg_end_nested(buf, set_offset);
+}
+break;
+case TC_ACT_PEDIT: {
+parse_flower_rewrite_to_netlink_action(buf, action);
+}
+break;
+case TC_ACT_ENCAP: {
+size_t set_offset = nl_msg_start_nested(buf, OVS_ACTION_ATTR_SET);
+size_t tunnel_offset =
+nl_msg_start_nested(buf, OVS_KEY_ATTR_TUNNEL);
+
+if (action->encap.id_present) {
+nl_msg_put_be64(buf, OVS_TUNNEL_KEY_ATTR_ID, action->encap.id);
+}
+if (action->encap.ipv4.ipv4_src) {
+nl_msg_put_be32(buf, OVS_TUNNEL_KEY_ATTR_IPV4_SRC,
+action->encap.ipv4.ipv4_src);
+}
+if (action->encap.ipv4.ipv4_dst) {
+nl_msg_put_be32(buf, OVS_TUNNEL_KEY_ATTR_IPV4_DST,
+action->encap.ipv4.ipv4_dst);
+}
+if (ipv6_addr_is_set(&action->encap.ipv6.ipv6_src)) {
+nl_msg_put_in6_addr(buf, OVS_TUNNEL_KEY_ATTR_IPV6_SRC,
+&action->encap.ipv6.ipv6_src);
+}
+if (ipv6_addr_is_set(&action->encap.ipv6.ipv6_dst)) {
+nl_msg_put_in6_addr(buf, OVS_TUNNEL_KEY_ATTR_IPV6_DST,
+&action->encap.ipv6.ipv6_dst);
+}
+if (action->encap.tos) {
+nl_msg_put_u8(buf, OVS_TUNNEL_KEY_ATTR_TOS,
+ action->encap.tos);
+}
+if (action->encap.ttl) {
+nl_msg_put_u8(buf, OVS_TUNNEL_KEY_ATTR_TTL,
+ action->encap.ttl);
+}
+if (action->encap.tp_dst) {
+nl_msg_put_be16(buf, OVS_TUNNEL_KEY_ATTR_TP_DST,
+
[ovs-dev] [PATCH v4 3/5] netdev-offload-tc: Handle check_pkt_len datapath action.
This change implements support for the check_pkt_len
action using the TC police action, which supports MTU
checking.
Signed-off-by: Eelco Chaudron
Acked-by: Mike Pattrick
Acked-by: Roi Dayan
---
v3:
Was using TCA_CSUM_PARMS instead of TCA_POLICE_TBF.
lib/netdev-offload-tc.c | 171 +-
lib/tc.c| 455 +++
lib/tc.h| 12 +
3 files changed, 594 insertions(+), 44 deletions(-)
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index f657af0fd..9b523c5f5 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -62,6 +62,13 @@ struct chain_node {
uint32_t chain;
};
+static int
+netdev_tc_parse_nl_actions(struct netdev *netdev, struct tc_flower *flower,
+ struct offload_info *info,
+ const struct nlattr *actions, size_t actions_len,
+ bool *recirc_act, bool more_actions,
+ struct tc_action **need_jump_update);
+
static bool
is_internal_port(const char *type)
{
@@ -825,6 +832,45 @@ _parse_tc_flower_to_actions(struct tc_flower *flower,
struct ofpbuf *buf,
nl_msg_put_u32(buf, OVS_ACTION_ATTR_RECIRC, action->chain);
}
break;
+case TC_ACT_POLICE_MTU: {
+size_t offset, act_offset;
+uint32_t jump;
+
+offset = nl_msg_start_nested(buf,
+ OVS_ACTION_ATTR_CHECK_PKT_LEN);
+
+nl_msg_put_u16(buf, OVS_CHECK_PKT_LEN_ATTR_PKT_LEN,
+ action->police.mtu);
+
+act_offset = nl_msg_start_nested(
+buf, OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER);
+i = _parse_tc_flower_to_actions(flower, buf, i + 1,
+action->police.result_jump);
+nl_msg_end_nested(buf, act_offset);
+
+act_offset = nl_msg_start_nested(
+buf, OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL);
+
+jump = flower->actions[i - 1].jump_action;
+if (jump == JUMP_ACTION_STOP) {
+jump = max_index;
+}
+if (jump != 0) {
+i = _parse_tc_flower_to_actions(flower, buf, i, jump);
+}
+nl_msg_end_nested(buf, act_offset);
+
+i--;
+nl_msg_end_nested(buf, offset);
+}
+break;
+}
+
+if (action->jump_action && action->type != TC_ACT_POLICE_MTU) {
+/* If there is a jump, it means this was the end of an action
+ * set and we need to end this branch. */
+i++;
+break;
}
}
return i;
@@ -1584,11 +1630,121 @@ parse_match_ct_state_to_flower(struct tc_flower
*flower, struct match *match)
}
}
+
+static int
+parse_check_pkt_len_action(struct netdev *netdev, struct tc_flower *flower,
+ struct offload_info *info, struct tc_action *action,
+ const struct nlattr *nla, bool last_action,
+ struct tc_action **need_jump_update,
+ bool *recirc_act)
+{
+struct tc_action *ge_jump_update = NULL, *le_jump_update = NULL;
+const struct nlattr *nl_actions;
+int err, le_offset, gt_offset;
+uint16_t pkt_len;
+
+static const struct nl_policy ovs_cpl_policy[] = {
+[OVS_CHECK_PKT_LEN_ATTR_PKT_LEN] = { .type = NL_A_U16 },
+[OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER] = { .type = NL_A_NESTED },
+[OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL]
+= { .type = NL_A_NESTED },
+};
+struct nlattr *a[ARRAY_SIZE(ovs_cpl_policy)];
+
+if (!nl_parse_nested(nla, ovs_cpl_policy, a, ARRAY_SIZE(a))) {
+VLOG_INFO("Received invalid formatted OVS_ACTION_ATTR_CHECK_PKT_LEN!");
+return EOPNOTSUPP;
+}
+
+if (!a[OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER] ||
+!a[OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL]) {
+VLOG_INFO("Received invalid OVS_CHECK_PKT_LEN_ATTR_ACTION_IF_*!");
+return EOPNOTSUPP;
+}
+
+pkt_len = nl_attr_get_u16(a[OVS_CHECK_PKT_LEN_ATTR_PKT_LEN]);
+
+/* Add the police mtu action first in the allocated slot. */
+action->police.mtu = pkt_len;
+action->type = TC_ACT_POLICE_MTU;
+le_offset = flower->action_count++;
+
+/* Parse and add the greater than action(s).
+ * NOTE: The last_action parameter means that there are no more actions
+ * after the if () then ... else () case. */
+nl_actions = a[OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER];
+err = netdev_tc_parse_nl_actions(netdev, flower, info,
+ nl_attr_get(nl_actions),
+ nl_attr_get_size(nl_actions),
+ recirc_act, !last_action,
+ &ge_jump_update);
+
[ovs-dev] [PATCH v4 4/5] system-offloads-traffic: Properly initialize offload before testing.
This patch will properly initialize offload, as it requires the setting to be enabled before starting ovs-vswitchd (or do a restart once configured). Signed-off-by: Eelco Chaudron --- v4: - Use the existing dbinit-aux-args argument, rather than creating a new pre-vswitchd command option. v3: - No changes. v2: - Unified all the OVS_TRAFFIC_VSWITCHD_START macro's tests/ofproto-macros.at |3 ++- tests/system-kmod-macros.at |4 ++-- tests/system-offloads-traffic.at |9 +++-- tests/system-tso-macros.at |4 ++-- tests/system-userspace-macros.at |4 ++-- 5 files changed, 11 insertions(+), 13 deletions(-) diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at index b18f0fbc1..af956bdcb 100644 --- a/tests/ofproto-macros.at +++ b/tests/ofproto-macros.at @@ -153,7 +153,7 @@ m4_divert_pop([PREPARE_TESTS]) m4_define([TESTABLE_LOG], [-vPATTERN:ANY:'%c|%p|%m']) -# _OVS_VSWITCHD_START([vswitchd-aux-args]) +# _OVS_VSWITCHD_START([vswitchd-aux-args] [dbinit-aux-args]) # # Creates an empty database and starts ovsdb-server. # Starts ovs-vswitchd, with additional arguments 'vswitchd-aux-args'. @@ -189,6 +189,7 @@ m4_define([_OVS_VSWITCHD_START], /ofproto|INFO|datapath ID changed to fedcba9876543210/d /dpdk|INFO|DPDK Disabled - Use other_config:dpdk-init to enable/d /netlink_socket|INFO|netlink: could not enable listening to all nsid/d +/netdev_offload|INFO|netdev: Flow API Enabled/d /probe tc:/d /setting extended ack support failed/d /tc: Using policy/d']]) diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at index 86d633ac4..b8a6b67c8 100644 --- a/tests/system-kmod-macros.at +++ b/tests/system-kmod-macros.at @@ -4,7 +4,7 @@ # appropriate type and properties m4_define([_ADD_BR], [[add-br $1 -- set Bridge $1 protocols=OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15 fail-mode=secure ]]) -# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [=override]) +# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [dbinit-aux-args]]) # # Creates a database and starts ovsdb-server, starts ovs-vswitchd # connected to that database, calls ovs-vsctl to create a bridge named @@ -24,7 +24,7 @@ m4_define([OVS_TRAFFIC_VSWITCHD_START], ]) on_exit 'ovs-dpctl del-dp ovs-system' on_exit 'ovs-appctl dpctl/flush-conntrack' - _OVS_VSWITCHD_START([]) + _OVS_VSWITCHD_START([], [$3]) dnl Add bridges, ports, etc. AT_CHECK([ovs-vsctl -- _ADD_BR([br0]) -- $1 m4_if([$2], [], [], [| uuidfilt])], [0], [$2]) ]) diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at index 80bc1dd5c..f7373664c 100644 --- a/tests/system-offloads-traffic.at +++ b/tests/system-offloads-traffic.at @@ -39,9 +39,8 @@ AT_CLEANUP AT_SETUP([offloads - ping between two ports - offloads enabled]) -OVS_TRAFFIC_VSWITCHD_START() +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) -AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true]) AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) ADD_NAMESPACES(at_ns0, at_ns1) @@ -97,8 +96,7 @@ AT_CLEANUP AT_SETUP([offloads - set ingress_policing_rate and ingress_policing_burst - offloads enabled]) AT_KEYWORDS([ingress_policing]) AT_SKIP_IF([test $HAVE_TC = "no"]) -OVS_TRAFFIC_VSWITCHD_START() -AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true]) +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) ADD_NAMESPACES(at_ns0) ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") @@ -146,8 +144,7 @@ AT_CLEANUP AT_SETUP([offloads - set ingress_policing_kpkts_rate and ingress_policing_kpkts_burst - offloads enabled]) AT_KEYWORDS([ingress_policing_kpkts]) AT_SKIP_IF([test $SUPPORT_TC_INGRESS_PPS = "no"]) -OVS_TRAFFIC_VSWITCHD_START() -AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true]) +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) ADD_NAMESPACES(at_ns0) ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") diff --git a/tests/system-tso-macros.at b/tests/system-tso-macros.at index 1a8004761..1881c28fb 100644 --- a/tests/system-tso-macros.at +++ b/tests/system-tso-macros.at @@ -4,7 +4,7 @@ # appropriate type and properties m4_define([_ADD_BR], [[add-br $1 -- set Bridge $1 datapath_type="netdev" protocols=OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15 fail-mode=secure ]]) -# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [=override]) +# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [pre-vswitchd-commands]) # # Creates a database and starts ovsdb-server, starts ovs-vswitchd # connected to that database, calls ovs-vsctl to create a bridge named @@ -15,7 +15,7 @@ m4_define([_ADD_BR], [[add-br $1 -- set Bridge $1 datapath_type="netdev" protoco m4_define
[ovs-dev] [PATCH v4 5/5] tests: Add check_pkt_len action test to system-offload-traffic.
Signed-off-by: Eelco Chaudron
Acked-by: Mike Pattrick
Acked-by: Roi Dayan
---
v2:
- Added section in the NEWS document
NEWS |2
tests/system-offloads-traffic.at | 413 ++
2 files changed, 414 insertions(+), 1 deletion(-)
diff --git a/NEWS b/NEWS
index 9fe3f44f4..e664731af 100644
--- a/NEWS
+++ b/NEWS
@@ -32,6 +32,8 @@ Post-v2.17.0
- DPDK:
* OVS validated with DPDK 21.11.1. It is recommended to use this version
until further releases.
+ - Linux datapath:
+ * Add support for offloading the check_pkt_len action.
v2.17.0 - 17 Feb 2022
diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at
index f7373664c..313e3ce3c 100644
--- a/tests/system-offloads-traffic.at
+++ b/tests/system-offloads-traffic.at
@@ -4,7 +4,20 @@ AT_BANNER([datapath offloads])
#
# Normilizes output ports, recirc_id, packets and macs.
#
-m4_define([DUMP_CLEAN_SORTED], [sed -e
's/used:[[0-9]].[[0-9]]*s/used:0.001s/;s/eth(src=[[a-z0-9:]]*,dst=[[a-z0-9:]]*)/eth(macs)/;s/actions:[[0-9,]]*/actions:output/;s/recirc_id(0),//'
| sort])
+m4_define([DUMP_CLEAN_SORTED], [sed -e
's/used:\([[0-9.]]*s\|never\)/used:0.001s/;s/eth(src=[[a-z0-9:]]*,dst=[[a-z0-9:]]*)/eth(macs)/;s/actions:[[0-9,]]\+/actions:output/;s/recirc_id(0),//'
| sort])
+
+
+# AT_CHECK_ACTIONS([ACTIONS])
+#
+# This extracts and matches the action for IPv4 rules for ingress port p0
+#
+m4_define([AT_CHECK_ACTIONS], [
+ AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded |
+sed -n
's/^.*in_port(2),eth(.*),eth_type(0x0800).*actions:\(.*\)/\1/p' |
+tr -d '\n'],
+ [0], [$1])
+])
+
AT_SETUP([offloads - ping between two ports - offloads disabled])
OVS_TRAFFIC_VSWITCHD_START()
@@ -165,3 +178,401 @@ matchall
])
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
+
+
+AT_SETUP([offloads - check_pkt_len action - offloads disabled])
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_NAMESPACES(at_ns1, at_ns2, at_ns3, at_ns4)
+
+ADD_VETH(p1, at_ns1, br0, "10.1.1.1/24")
+ADD_VETH(p2, at_ns2, br0, "10.1.1.2/24")
+ADD_VETH(p3, at_ns3, br0, "10.1.1.3/24")
+ADD_VETH(p4, at_ns4, br0, "10.1.1.4/24")
+
+AT_DATA([flows.txt], [dnl
+table=0,in_port=2 actions=output:1
+table=0,in_port=1
actions=load:0x1->NXM_NX_REG1[[]],resubmit(,1),load:0x2->NXM_NX_REG1[[]],resubmit(,1)
+table=1,in_port=1,reg1=0x1
actions=check_pkt_larger(200)->NXM_NX_REG0[[0]],resubmit(,4)
+table=1,in_port=1,reg1=0x2 actions=output:2
+table=4,in_port=1,reg0=0x1 actions=output:3
+table=4,in_port=1,reg0=0x0 actions=output:4
+])
+AT_CHECK([ovs-ofctl --protocols=OpenFlow10 add-flows br0 flows.txt])
+
+NS_CHECK_EXEC([at_ns3], [tcpdump -l -n -U -i p3 dst 10.1.1.2 and icmp >
p3.pcap 2>/dev/null &])
+NS_CHECK_EXEC([at_ns4], [tcpdump -l -n -U -i p4 dst 10.1.1.2 and icmp >
p4.pcap 2>/dev/null &])
+sleep 1
+
+NS_CHECK_EXEC([at_ns1], [ping -q -c 10 -i 0.1 -w 2 -s 64 10.1.1.2 |
FORMAT_PING], [0], [dnl
+10 packets transmitted, 10 received, 0% packet loss, time 0ms
+], [], [ovs-appctl dpctl/dump-flows; ovs-ofctl dump-flows br0])
+
+NS_CHECK_EXEC([at_ns1], [ping -q -c 10 -i 0.1 -w 2 -s 1024 10.1.1.2 |
FORMAT_PING], [0], [dnl
+10 packets transmitted, 10 received, 0% packet loss, time 0ms
+], [], [ovs-appctl dpctl/dump-flows; ovs-ofctl dump-flows br0])
+
+AT_CHECK([ovs-appctl dpctl/dump-flows | grep "eth_type(0x0800)" |
DUMP_CLEAN_SORTED], [0], [dnl
+in_port(2),eth(),eth_type(0x0800),ipv4(frag=no), packets:19, bytes:11614,
used:0.001s, actions:check_pkt_len(size=200,gt(4),le(5)),3
+in_port(3),eth(),eth_type(0x0800),ipv4(frag=no), packets:19, bytes:11614,
used:0.001s, actions:output
+])
+
+AT_CHECK([ovs-appctl dpctl/dump-flows type=ovs | grep "eth_type(0x0800)" |
DUMP_CLEAN_SORTED], [0], [dnl
+in_port(2),eth(),eth_type(0x0800),ipv4(frag=no), packets:19, bytes:11614,
used:0.001s, actions:check_pkt_len(size=200,gt(4),le(5)),3
+in_port(3),eth(),eth_type(0x0800),ipv4(frag=no), packets:19, bytes:11614,
used:0.001s, actions:output
+])
+
+AT_CHECK([ovs-appctl dpctl/dump-flows type=tc,offloaded], [0], [])
+
+AT_CHECK([test $(ovs-appctl upcall/show | grep -c "offloaded flows") -eq 0],
[0], [ignore])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+
+AT_CHECK([cat p3.pcap | awk '{print $NF}' | uniq -c | awk '{$1=$1;print}'],
[0], [dnl
+10 1032
+])
+AT_CHECK([cat p4.pcap | awk '{print $NF}' | uniq -c | awk '{$1=$1;print}'],
[0], [dnl
+10 72
+])
+
+AT_CLEANUP
+
+
+AT_SETUP([offloads - check_pkt_len action - offloads enabled])
+OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch .
other_config:hw-offload=true])
+
+ADD_NAMESPACES(at_ns1, at_ns2, at_ns3, at_ns4)
+
+ADD_VETH(p1, at_ns1, br0, "10.1.1.1/24")
+ADD_VETH(p2, at_ns2, br0, "10.1.1.2/24")
+ADD_VETH(p3, at_ns3, br0, "10.1.1.3/24")
+ADD_VETH(p4, at_ns4, br0, "10.1.1.4/24")
+
+AT_DATA([flows.txt], [dnl
+table=0,in_port=2 actions=output:1
+table=0,in_port=1
actions=load:0x1->NXM_NX_REG1[[]],resubmit(,1),load:0x2->NXM_NX_REG1[[]],resubmit(,1)
+table=1,in_port=1,reg1=
Re: [ovs-dev] [PATCH v2 1/1] netdev-dpdk: Delay vhost mempool creation.
On 02/06/2022 17:10, Ilya Maximets wrote: On 6/2/22 16:28, Kevin Traynor wrote: On 02/06/2022 15:00, Ilya Maximets wrote: On 6/2/22 13:01, Kevin Traynor wrote: On 01/06/2022 14:30, David Marchand wrote: On Wed, May 25, 2022 at 1:11 PM Kevin Traynor wrote: Currently mempools for vhost are being assigned before the vhost device is added. In some cases this may be just reusing an existing mempool but in others it can require creation of a mempool. For multi-NUMA, the NUMA info of the vhost port is not known until a device is added to the port, so on multi-NUMA systems the initial NUMA node for the mempool is a best guess based on vswitchd affinity. When a device is added to the vhost port, the NUMA info can be checked and if the guess was incorrect a mempool on the correct NUMA node created. For multi-NUMA, the current scheme can have the effect of creating a mempool on a NUMA node that will not be needed and at least for a certain time period requires more memory on a NUMA node. It is also difficult for a user trying to provision memory on different NUMA nodes, if they are not sure which NUMA node the initial mempool for a vhost port will be on. For single NUMA, even though the mempool will be on the correct NUMA, it is assigned ahead of time and if a vhost device was not added, it could also be using uneeded memory. This patch delays the creation of the mempool for a vhost port until the vhost device is added. Signed-off-by: Kevin Traynor Reviewed-by: David Marchand Thanks for aligning the behavior, and updating the commitlog. I did not test the mono numa part but I guess you did, and I don't see what could be wrong. I just wonder if we should add some note in NEWS for this change of behavior, though it is really low-level/internal... Regardless of the update on NEWS, this version lgtm. Thanks David. I don't mind about the NEWS. It is a low-level change, but it might help a user to know if it is the new or old behaviour, in case they are debugging memory problems, logs etc. How about: "Delay creating or reusing a mempool for vhost ports until the vhost device has been added." I'm curious, does this change cause delayed error reporting or we do not report error on add-port already? Yeah, if it failed to get a mempool under both schemes, then the log would appear at a later time now. That is part of what i was thinking about when i mentioned debugging/logs. I don't mind it be either way, it just maybe worth highlighting for users. Sure, I can add a short sentence to mention. e.g. "Delay creating or reusing a mempool for vhost ports until the vhost device has been added. A failure to create a mempool will now also be logged when the vhost device is added." Sounds ok? "until the vhost device has been added" sounds a bit too formal, I guess. "until VM is started" would be more user-friendly. I know that this is not really correct (e.g. virtio-user), but should be more understandable? Also, I'm not sure if "vhost device" is a right term. "until the virtio device attached/connected" maybe? I mean, OVS registers a vhost device, but it's kind of an internal thing (OVS creates vhost device for itself), the event users are interested in is a client's virtio device connecting to our vhost-user port. Naming is confusing around vhost/virtio. I hope I got it right. :) I think you proved the point that while not fully accurate, "until the VM is started." is probably the best for understanding for most OVS users :-) Unless there's some better suggestion by the afternoon, I will resend with that. Does that make any sense? Other suggestions are welcome. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4 4/5] system-offloads-traffic: Properly initialize offload before testing.
Hi Mike/Roi, This is the patch with the changes in this revision. I removed your ACKs, so can you please re-review this patch? Below are the changes compared to the previous version to make things easy for you! Thanks, Eelco 1: f0020c3d5 ! 1: 6c21ec1f9 system-offloads-traffic: Properly initialize offload before testing. @@ Commit message restart once configured). Signed-off-by: Eelco Chaudron -Acked-by: Mike Pattrick +v4: + - Use the existing dbinit-aux-args argument, rather than +creating a new pre-vswitchd command option. +v3: + - No changes. v2: - Unified all the OVS_TRAFFIC_VSWITCHD_START macro's @@ tests/ofproto-macros.at: m4_divert_pop([PREPARE_TESTS]) m4_define([TESTABLE_LOG], [-vPATTERN:ANY:'%c|%p|%m']) -# _OVS_VSWITCHD_START([vswitchd-aux-args]) -+# _OVS_VSWITCHD_START([vswitchd-aux-args] [dbinit-aux-args] [pre-vswitchd-commands]) ++# _OVS_VSWITCHD_START([vswitchd-aux-args] [dbinit-aux-args]) # # Creates an empty database and starts ovsdb-server. # Starts ovs-vswitchd, with additional arguments 'vswitchd-aux-args'. -@@ tests/ofproto-macros.at: m4_define([_OVS_VSWITCHD_START], -dnl Initialize database. -AT_CHECK([ovs-vsctl --no-wait init $2]) - -+ dnl Run extra commands before ovs-vswitchd starts. -+ AT_CHECK([:; $3]) -+ -dnl Start ovs-vswitchd. -AT_CHECK([ovs-vswitchd $1 --detach --no-chdir --pidfile --log-file -vvconn -vofproto_dpif -vunixctl], [0], [], [stderr]) -AT_CAPTURE_FILE([ovs-vswitchd.log]) @@ tests/ofproto-macros.at: m4_define([_OVS_VSWITCHD_START], /ofproto|INFO|datapath ID changed to fedcba9876543210/d /dpdk|INFO|DPDK Disabled - Use other_config:dpdk-init to enable/d @@ tests/system-kmod-macros.at m4_define([_ADD_BR], [[add-br $1 -- set Bridge $1 protocols=OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15 fail-mode=secure ]]) -# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [=override]) -+# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [pre-vswitchd-commands]) ++# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [dbinit-aux-args]]) # # Creates a database and starts ovsdb-server, starts ovs-vswitchd # connected to that database, calls ovs-vsctl to create a bridge named @@ tests/system-kmod-macros.at: m4_define([OVS_TRAFFIC_VSWITCHD_START], on_exit 'ovs-dpctl del-dp ovs-system' on_exit 'ovs-appctl dpctl/flush-conntrack' - _OVS_VSWITCHD_START([]) -+ _OVS_VSWITCHD_START([], [], [$3]) ++ _OVS_VSWITCHD_START([], [$3]) dnl Add bridges, ports, etc. AT_CHECK([ovs-vsctl -- _ADD_BR([br0]) -- $1 m4_if([$2], [], [], [| uuidfilt])], [0], [$2]) ]) @@ tests/system-offloads-traffic.at: AT_CLEANUP AT_SETUP([offloads - ping between two ports - offloads enabled]) -OVS_TRAFFIC_VSWITCHD_START() -+OVS_TRAFFIC_VSWITCHD_START([], [], [ovs-vsctl --no-wait set Open_vSwitch . other_config:hw-offload=true]) ++OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) -AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true]) AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) @@ tests/system-offloads-traffic.at: AT_CLEANUP AT_SKIP_IF([test $HAVE_TC = "no"]) -OVS_TRAFFIC_VSWITCHD_START() -AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true]) -+OVS_TRAFFIC_VSWITCHD_START([], [], [ovs-vsctl --no-wait set Open_vSwitch . other_config:hw-offload=true]) ++OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) ADD_NAMESPACES(at_ns0) ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") @@ tests/system-offloads-traffic.at: AT_CLEANUP AT_SKIP_IF([test $SUPPORT_TC_INGRESS_PPS = "no"]) -OVS_TRAFFIC_VSWITCHD_START() -AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true]) -+OVS_TRAFFIC_VSWITCHD_START([], [], [ovs-vsctl --no-wait set Open_vSwitch . other_config:hw-offload=true]) ++OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . other_config:hw-offload=true]) AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) ADD_NAMESPACES(at_ns0) ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") @@ tests/system-userspace-macros.at m4_define([_ADD_BR], [[add-br $1 -- set Bridge $1 datapath_type="netdev" protocols=OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15 fail-mode=secure ]]) -# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [=override]) -+# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [pre-vswitchd-commands]) ++# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [dbinit-aux-args]) # # Creat
Re: [ovs-dev] [PATCH v2 1/1] netdev-dpdk: Delay vhost mempool creation.
On 6/3/22 11:19, Kevin Traynor wrote: > On 02/06/2022 17:10, Ilya Maximets wrote: >> On 6/2/22 16:28, Kevin Traynor wrote: >>> On 02/06/2022 15:00, Ilya Maximets wrote: On 6/2/22 13:01, Kevin Traynor wrote: > On 01/06/2022 14:30, David Marchand wrote: >> On Wed, May 25, 2022 at 1:11 PM Kevin Traynor >> wrote: >>> >>> Currently mempools for vhost are being assigned before the vhost device >>> is added. In some cases this may be just reusing an existing mempool >>> but >>> in others it can require creation of a mempool. >>> >>> For multi-NUMA, the NUMA info of the vhost port is not known until a >>> device is added to the port, so on multi-NUMA systems the initial NUMA >>> node for the mempool is a best guess based on vswitchd affinity. >>> >>> When a device is added to the vhost port, the NUMA info can be checked >>> and if the guess was incorrect a mempool on the correct NUMA node >>> created. >>> >>> For multi-NUMA, the current scheme can have the effect of creating a >>> mempool on a NUMA node that will not be needed and at least for a >>> certain >>> time period requires more memory on a NUMA node. >>> >>> It is also difficult for a user trying to provision memory on different >>> NUMA nodes, if they are not sure which NUMA node the initial mempool >>> for a vhost port will be on. >>> >>> For single NUMA, even though the mempool will be on the correct NUMA, it >>> is assigned ahead of time and if a vhost device was not added, it could >>> also be using uneeded memory. >>> >>> This patch delays the creation of the mempool for a vhost port until the >>> vhost device is added. >>> >>> Signed-off-by: Kevin Traynor >>> Reviewed-by: David Marchand >> >> Thanks for aligning the behavior, and updating the commitlog. >> I did not test the mono numa part but I guess you did, and I don't see >> what could be wrong. >> >> I just wonder if we should add some note in NEWS for this change of >> behavior, though it is really low-level/internal... >> >> Regardless of the update on NEWS, this version lgtm. >> > > Thanks David. I don't mind about the NEWS. It is a low-level change, but > it might help a user to know if it is the new or old behaviour, in case > they are debugging memory problems, logs etc. How about: > > "Delay creating or reusing a mempool for vhost ports until the vhost > device has been added." I'm curious, does this change cause delayed error reporting or we do not report error on add-port already? >>> >>> Yeah, if it failed to get a mempool under both schemes, then the log would >>> appear at a later time now. That is part of what i was thinking about when >>> i mentioned debugging/logs. >>> I don't mind it be either way, it just maybe worth highlighting for users. >>> >>> Sure, I can add a short sentence to mention. e.g. >>> >>> "Delay creating or reusing a mempool for vhost ports until the vhost device >>> has been added. A failure to create a mempool will now also be logged when >>> the vhost device is added." >>> >>> Sounds ok? >> >> "until the vhost device has been added" sounds a bit too formal, I guess. >> "until VM is started" would be more user-friendly. I know that this is >> not really correct (e.g. virtio-user), but should be more understandable? >> Also, I'm not sure if "vhost device" is a right term. "until the virtio >> device attached/connected" maybe? I mean, OVS registers a vhost device, >> but it's kind of an internal thing (OVS creates vhost device for itself), >> the event users are interested in is a client's virtio device connecting >> to our vhost-user port. >> >> Naming is confusing around vhost/virtio. I hope I got it right. :) >> > > I think you proved the point that while not fully accurate, "until the VM is > started." is probably the best for understanding for most OVS users :-) > > Unless there's some better suggestion by the afternoon, I will resend with > that. I'm OK with that. On the other note, " will now also be logged when" should probbaly be "will now also be logged only when", maybe even without "also". Otherwise it sounds like the error will be logged twice. What do you think? And, please, keep 2 empty lines between NEWS sections. :) > >> Does that make any sense? Other suggestions are welcome. >> > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 14/18] python: introduce unit tests
On 3/11/22 16:21, Adrian Moreno wrote:
> Use pytest to run unit tests as part of the standard testsuite.
>
> Acked-by: Eelco Chaudron
> Signed-off-by: Adrian Moreno
> ---
> .github/workflows/build-and-test.yml| 3 +
> Documentation/intro/install/general.rst | 4 ++
> python/automake.mk | 9 ++-
> python/ovs/tests/test_kv.py | 76 +
> python/test_requirements.txt| 3 +
> tests/atlocal.in| 19 +++
> tests/automake.mk | 1 +
> tests/pytest.at | 7 +++
> tests/testsuite.at | 1 +
> 9 files changed, 121 insertions(+), 2 deletions(-)
> create mode 100644 python/ovs/tests/test_kv.py
> create mode 100644 python/test_requirements.txt
> create mode 100644 tests/pytest.at
>
> diff --git a/.github/workflows/build-and-test.yml
> b/.github/workflows/build-and-test.yml
> index eac3504e4..44df1c2d5 100644
> --- a/.github/workflows/build-and-test.yml
> +++ b/.github/workflows/build-and-test.yml
> @@ -123,6 +123,9 @@ jobs:
>with:
> python-version: '3.9'
>
> +- name: install python dependencies
> + run: pip install -r python/test_requirements.txt
Can we place this after the 'prepare' stage or inside the
prepare.sh script? Just to ensure that we have 'wheel'
installed before trying to install anything else.
> +
> - name: create ci signature file for the dpdk cache key
>if: matrix.dpdk != '' || matrix.dpdk_shared != ''
># This will collect most of DPDK related lines, so hash will be
> different
> diff --git a/Documentation/intro/install/general.rst
> b/Documentation/intro/install/general.rst
> index c4300cd53..711fb98a4 100644
> --- a/Documentation/intro/install/general.rst
> +++ b/Documentation/intro/install/general.rst
> @@ -181,6 +181,10 @@ following to obtain better warnings:
>come from the "hacking" flake8 plugin. If it's not installed, the warnings
>just won't occur until it's run on a system with "hacking" installed.
>
> +- the python packages listed in "python/test_requirements.txt" (compatible
> + with pip). If they are installed, the pytest-based Python unit tests will
> + be run.
> +
> You may find the ovs-dev script found in ``utilities/ovs-dev.py`` useful.
>
> .. _general-install-reqs:
> diff --git a/python/automake.mk b/python/automake.mk
> index e5501c58e..43a27d3cb 100644
> --- a/python/automake.mk
> +++ b/python/automake.mk
> @@ -53,6 +53,9 @@ ovs_pyfiles = \
> python/ovs/vlog.py \
> python/ovs/winutils.py
>
> +ovs_pytests = \
> + python/ovs/tests/test_kv.py
> +
> # These python files are used at build time but not runtime,
> # so they are not installed.
> EXTRA_DIST += \
> @@ -66,12 +69,14 @@ EXTRA_DIST += \
> EXTRA_DIST += \
> python/ovs/compat/sortedcontainers/LICENSE \
> python/README.rst \
> - python/setup.py
> + python/setup.py \
> + python/test_requirements.txt
>
> # C extension support.
> EXTRA_DIST += python/ovs/_json.c
>
> -PYFILES = $(ovs_pyfiles) python/ovs/dirs.py $(ovstest_pyfiles)
> +PYFILES = $(ovs_pyfiles) python/ovs/dirs.py $(ovstest_pyfiles) $(ovs_pytests)
> +
> EXTRA_DIST += $(PYFILES)
> PYCOV_CLEAN_FILES += $(PYFILES:.py=.py,cover)
>
> diff --git a/python/ovs/tests/test_kv.py b/python/ovs/tests/test_kv.py
> new file mode 100644
> index 0..e81804d49
> --- /dev/null
> +++ b/python/ovs/tests/test_kv.py
> @@ -0,0 +1,76 @@
> +import pytest
> +
> +from ovs.flows.kv import KVParser, KeyValue
> +
> +
> +@pytest.mark.parametrize(
> +"input_data,expected",
> +[
> +(
> +(
> +"cookie=0x0, duration=147566.365s, table=0, n_packets=39,
> n_bytes=2574, idle_age=65534, hard_age=65534", # noqa: E501
> +None,
> +),
> +[
> +KeyValue("cookie", 0),
> +KeyValue("duration", "147566.365s"),
> +KeyValue("table", 0),
> +KeyValue("n_packets", 39),
> +KeyValue("n_bytes", 2574),
> +KeyValue("idle_age", 65534),
> +KeyValue("hard_age", 65534),
> +],
> +),
> +(
> +(
> +
> "load:0x4->NXM_NX_REG13[],load:0x9->NXM_NX_REG11[],load:0x8->NXM_NX_REG12[],load:0x1->OXM_OF_METADATA[],load:0x1->NXM_NX_REG14[],mod_dl_src:0a:58:a9:fe:00:02,resubmit(,8)",
> # noqa: E501
> +None,
> +),
> +[
> +KeyValue("load", "0x4->NXM_NX_REG13[]"),
> +KeyValue("load", "0x9->NXM_NX_REG11[]"),
> +KeyValue("load", "0x8->NXM_NX_REG12[]"),
> +KeyValue("load", "0x1->OXM_OF_METADATA[]"),
> +KeyValue("load", "0x1->NXM_NX_REG14[]"),
> +KeyValue("mod_dl_src", "0a:58:a9:fe:00:02"),
> +KeyValue("resubmit", ",8"),
> +],
> +),
> +
[ovs-dev] [PATCH v4 0/1] Delayed vhost mempool creation.
V2: - Also implement delayed vhost mempool creation/reuse for single-NUMA - Added David's RvB, as minor code change from v1 and David had preference for common single/multi NUMA behaviour V1 only operated for multi-NUMA, as potentially having to create a mempool and then recreate it on a different NUMA was the behaviour I was trying to remove. After comment from David and some testing, V2 also adds the delayed mempool creation for single NUMA. Reason being that for single NUMA the mempool was still potentially being created before it was needed. Also, if the vhost device was never added, then the mempool might not be needed at all. A minor advantage is consistency between single and multi NUMA operation. V3 - Added NEWS item, kept David's RvB V4 - minor NEWS reword, fix spacing - kept David's RvB GHA: https://github.com/kevintraynor/ovs/actions/runs/2428995992 Kevin Traynor (1): netdev-dpdk: Delay vhost mempool creation. NEWS | 3 +++ lib/netdev-dpdk.c | 21 - 2 files changed, 15 insertions(+), 9 deletions(-) -- 2.34.3 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 1/1] netdev-dpdk: Delay vhost mempool creation.
Currently mempools for vhost are being assigned before the vhost device
is added. In some cases this may be just reusing an existing mempool but
in others it can require creation of a mempool.
For multi-NUMA, the NUMA info of the vhost port is not known until a
device is added to the port, so on multi-NUMA systems the initial NUMA
node for the mempool is a best guess based on vswitchd affinity.
When a device is added to the vhost port, the NUMA info can be checked
and if the guess was incorrect a mempool on the correct NUMA node
created.
For multi-NUMA, the current scheme can have the effect of creating a
mempool on a NUMA node that will not be needed and at least for a certain
time period requires more memory on a NUMA node.
It is also difficult for a user trying to provision memory on different
NUMA nodes, if they are not sure which NUMA node the initial mempool
for a vhost port will be on.
For single NUMA, even though the mempool will be on the correct NUMA, it
is assigned ahead of time and if a vhost device was not added, it could
also be using uneeded memory.
This patch delays the creation of the mempool for a vhost port until the
vhost device is added.
Signed-off-by: Kevin Traynor
Reviewed-by: David Marchand
---
NEWS | 3 +++
lib/netdev-dpdk.c | 21 -
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/NEWS b/NEWS
index 9fe3f44f4..9871e8ce9 100644
--- a/NEWS
+++ b/NEWS
@@ -33,4 +33,7 @@ Post-v2.17.0
* OVS validated with DPDK 21.11.1. It is recommended to use this version
until further releases.
+ * Delay creating or reusing a mempool for vhost ports until the VM
+ is started. A failure to create a mempool will now be logged only
+ when the VM is started.
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index f9535bfb4..081900576 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -3927,5 +3927,6 @@ new_device(int vid)
if (dev->requested_n_txq < qp_num
|| dev->requested_n_rxq < qp_num
-|| dev->requested_socket_id != newnode) {
+|| dev->requested_socket_id != newnode
+|| dev->dpdk_mp == NULL) {
dev->requested_socket_id = newnode;
dev->requested_n_rxq = qp_num;
@@ -4977,5 +4978,4 @@ dpdk_vhost_reconfigure_helper(struct netdev_dpdk *dev)
dev->up.n_txq = dev->requested_n_txq;
dev->up.n_rxq = dev->requested_n_rxq;
-int err;
/* Always keep RX queue 0 enabled for implementations that won't
@@ -4995,12 +4995,15 @@ dpdk_vhost_reconfigure_helper(struct netdev_dpdk *dev)
netdev_dpdk_remap_txqs(dev);
-err = netdev_dpdk_mempool_configure(dev);
-if (!err) {
-/* A new mempool was created or re-used. */
-netdev_change_seq_changed(&dev->up);
-} else if (err != EEXIST) {
-return err;
-}
if (netdev_dpdk_get_vid(dev) >= 0) {
+int err;
+
+err = netdev_dpdk_mempool_configure(dev);
+if (!err) {
+/* A new mempool was created or re-used. */
+netdev_change_seq_changed(&dev->up);
+} else if (err != EEXIST) {
+return err;
+}
+
if (dev->vhost_reconfigured == false) {
dev->vhost_reconfigured = true;
--
2.34.3
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v2 0/3] Use masked ct_mark in a backwards compatible way.
Explicit backports for these patches (except 3/3 which should only be applied to main) are available at: - branch-22.06: https://github.com/dceara/ovn/tree/bz2091565-ct-masked-mark-follow-up-22.06 - branch-22.03: https://github.com/dceara/ovn/tree/bz2091565-ct-masked-mark-follow-up-22.03 - branch-21.12: https://github.com/dceara/ovn/tree/bz2091565-ct-masked-mark-follow-up-21.12 V2: - Addressed comments from Mark and Han: - added a 3rd patch to flip the lb_hairpin_use_ct_mark default to 'true'. - changed I-P of the northd internal version into I-P of the SB_Global options. - consolidated all ovn-controller ct_mark masked access support under the "ct-no-masked-label" feature name. Dumitru Ceara (3): northd: ovn-controller: Use ct_mark.natted only when ct_lb_mark is used. northd: Use ct_mark.blocked and ecmp_reply_port only when all chassis support it. controller: Use ct_mark by default for load balancer hairpin flows. controller/chassis.c| 5 +- controller/ovn-controller.c | 2 +- include/ovn/features.h | 4 +- northd/northd.c | 172 +++- northd/northd.h | 2 +- ovn-sb.xml | 12 ++- tests/ovn-northd.at | 96 +++- 7 files changed, 222 insertions(+), 71 deletions(-) ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn v2 1/3] northd: ovn-controller: Use ct_mark.natted only when ct_lb_mark is used.
Change the way ovn-controller decides whether it should match on
ct_mark.natted or ct_label.natted for hairpin load balancer traffic.
Until now this was done solely based on the northd-reported internal
version.
However, to cover the case when OVN central components are not the last
ones to be updated, ovn-northd now explicitly informs ovn-controller
whether it should use ct_mark or ct_label.natted via a new option in the
OVN_Southbound.SB_Global record: lb_hairpin_use_ct_mark.
Signed-off-by: Dumitru Ceara
---
controller/lflow.c | 40 ++--
controller/lflow.h |2 +
controller/ovn-controller.c | 86 ---
northd/northd.c | 31 ++--
ovn-sb.xml | 16
tests/ovn-controller.at |8 ++--
6 files changed, 99 insertions(+), 84 deletions(-)
diff --git a/controller/lflow.c b/controller/lflow.c
index 7a3419305..934b23698 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -1942,7 +1942,7 @@ add_lb_vip_hairpin_flows(struct ovn_controller_lb *lb,
struct ovn_lb_vip *lb_vip,
struct ovn_lb_backend *lb_backend,
uint8_t lb_proto,
- bool check_ct_label_for_lb_hairpin,
+ bool use_ct_mark,
struct ovn_desired_flow_table *flow_table)
{
uint64_t stub[1024 / 8];
@@ -2033,18 +2033,19 @@ add_lb_vip_hairpin_flows(struct ovn_controller_lb *lb,
* - packets must have ip.src == ip.dst at this point.
* - the destination protocol and port must be of a valid backend that
* has the same IP as ip.dst.
+ *
+ * During upgrades logical flows might still use the old way of storing
+ * ct.natted in ct_label. For backwards compatibility, only use ct_mark
+ * if ovn-northd notified ovn-controller to do that.
*/
-uint32_t lb_ct_mark = OVN_CT_NATTED;
-match_set_ct_mark_masked(&hairpin_match, lb_ct_mark, lb_ct_mark);
-
-ofctrl_add_flow(flow_table, OFTABLE_CHK_LB_HAIRPIN, 100,
-lb->slb->header_.uuid.parts[0], &hairpin_match,
-&ofpacts, &lb->slb->header_.uuid);
+if (use_ct_mark) {
+uint32_t lb_ct_mark = OVN_CT_NATTED;
+match_set_ct_mark_masked(&hairpin_match, lb_ct_mark, lb_ct_mark);
-/* The below flow is identical to the above except that it checks
- * ct_label.natted instead of ct_mark.natted, for backward compatibility
- * during the upgrade from a previous version that uses ct_label. */
-if (check_ct_label_for_lb_hairpin) {
+ofctrl_add_flow(flow_table, OFTABLE_CHK_LB_HAIRPIN, 100,
+lb->slb->header_.uuid.parts[0], &hairpin_match,
+&ofpacts, &lb->slb->header_.uuid);
+} else {
match_set_ct_mark_masked(&hairpin_match, 0, 0);
ovs_u128 lb_ct_label = {
.u64.lo = OVN_CT_NATTED,
@@ -2328,7 +2329,7 @@ add_lb_ct_snat_hairpin_flows(struct ovn_controller_lb *lb,
static void
consider_lb_hairpin_flows(const struct sbrec_load_balancer *sbrec_lb,
const struct hmap *local_datapaths,
- bool check_ct_label_for_lb_hairpin,
+ bool use_ct_mark,
struct ovn_desired_flow_table *flow_table,
struct simap *ids)
{
@@ -2368,8 +2369,7 @@ consider_lb_hairpin_flows(const struct
sbrec_load_balancer *sbrec_lb,
struct ovn_lb_backend *lb_backend = &lb_vip->backends[j];
add_lb_vip_hairpin_flows(lb, lb_vip, lb_backend, lb_proto,
- check_ct_label_for_lb_hairpin,
- flow_table);
+ use_ct_mark, flow_table);
}
}
@@ -2382,8 +2382,7 @@ consider_lb_hairpin_flows(const struct
sbrec_load_balancer *sbrec_lb,
* backends to handle the load balanced hairpin traffic. */
static void
add_lb_hairpin_flows(const struct sbrec_load_balancer_table *lb_table,
- const struct hmap *local_datapaths,
- bool check_ct_label_for_lb_hairpin,
+ const struct hmap *local_datapaths, bool use_ct_mark,
struct ovn_desired_flow_table *flow_table,
struct simap *ids,
struct id_pool *pool)
@@ -2406,8 +2405,7 @@ add_lb_hairpin_flows(const struct
sbrec_load_balancer_table *lb_table,
ovs_assert(id_pool_alloc_id(pool, &id));
simap_put(ids, lb->name, id);
}
-consider_lb_hairpin_flows(lb, local_datapaths,
- check_ct_label_for_lb_hairpin,
+consider_lb_hairpin_flows(lb, local_datapaths, use_ct_mark,
flow_table, ids);
}
}
@@ -2545,7 +2543,7 @@ lflow_run(struct lflow_ctx_in *l_ctx_in
[ovs-dev] [PATCH ovn v2 2/3] northd: Use ct_mark.blocked and ecmp_reply_port only when all chassis support it.
Commit a075230e4a0f ("Use ct_mark for masked access to make flows
HW-offloading friendly.") started using the ct_mark.blocked and
ct_mark.ecmp_reply_port ct_label. In usual scenarios this new feature would
be picked up when the next stable release becomes available (i.e., 22.06.0).
However, the commit was also backported to stable branches (branch-22.03 and
branch-21.12).
While the supported upgrade scenario for OVN when moving to a new
stable release is to ensure that ovn-controllers are upgraded first,
it's not really clear that this restriction applies to "z-stream"
upgrades too (e.g., from v21.12.1 to v21.12.2).
Some CMSs, like RHV (Red Hat Virtualization), expect ovn-controllers
running older code from a stable branch to be able to interpret all
Southbound contents generated by ovn-northd instances built from the
same or newer versions of the stable branch.
Ensure that ct_mark.blocked and ecmp_reply_port are used only when all
chassis registered in the Southbound support it.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2091565
Signed-off-by: Dumitru Ceara
---
controller/chassis.c |5 +
include/ovn/features.h |4 +
northd/northd.c| 162 +++-
northd/northd.h|2 -
tests/ovn-northd.at| 96
5 files changed, 206 insertions(+), 63 deletions(-)
diff --git a/controller/chassis.c b/controller/chassis.c
index 239658461..92850fcc1 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -350,7 +350,7 @@ chassis_build_other_config(const struct ovs_chassis_cfg
*ovs_cfg,
smap_replace(config, "is-interconn",
ovs_cfg->is_interconn ? "true" : "false");
smap_replace(config, OVN_FEATURE_PORT_UP_NOTIF, "true");
-smap_replace(config, OVN_FEATURE_CT_LB_MARK, "true");
+smap_replace(config, OVN_FEATURE_CT_NO_MASKED_LABEL, "true");
}
/*
@@ -456,7 +456,8 @@ chassis_other_config_changed(const struct ovs_chassis_cfg
*ovs_cfg,
return true;
}
-if (!smap_get_bool(&chassis_rec->other_config, OVN_FEATURE_CT_LB_MARK,
+if (!smap_get_bool(&chassis_rec->other_config,
+ OVN_FEATURE_CT_NO_MASKED_LABEL,
false)) {
return true;
}
diff --git a/include/ovn/features.h b/include/ovn/features.h
index 09f002287..8fbdbf19a 100644
--- a/include/ovn/features.h
+++ b/include/ovn/features.h
@@ -21,8 +21,8 @@
#include "smap.h"
/* ovn-controller supported feature names. */
-#define OVN_FEATURE_PORT_UP_NOTIF "port-up-notif"
-#define OVN_FEATURE_CT_LB_MARK"ct-lb-mark"
+#define OVN_FEATURE_PORT_UP_NOTIF "port-up-notif"
+#define OVN_FEATURE_CT_NO_MASKED_LABEL "ct-no-masked-label"
/* OVS datapath supported features. Based on availability OVN might generate
* different types of openflows.
diff --git a/northd/northd.c b/northd/northd.c
index 3b3900cea..43e87be6b 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -405,13 +405,14 @@ build_chassis_features(const struct northd_input
*input_data,
const struct sbrec_chassis *chassis;
SBREC_CHASSIS_TABLE_FOR_EACH (chassis, input_data->sbrec_chassis) {
-if (!smap_get_bool(&chassis->other_config, OVN_FEATURE_CT_LB_MARK,
+if (!smap_get_bool(&chassis->other_config,
+ OVN_FEATURE_CT_NO_MASKED_LABEL,
false)) {
-chassis_features->ct_lb_mark = false;
+chassis_features->ct_no_masked_label = false;
return;
}
}
-chassis_features->ct_lb_mark = true;
+chassis_features->ct_no_masked_label = true;
}
struct ovn_chassis_qdisc_queues {
@@ -5816,7 +5817,9 @@ build_pre_stateful(struct ovn_datapath *od,
ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 0, "1", "next;");
ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_STATEFUL, 0, "1", "next;");
-const char *ct_lb_action = features->ct_lb_mark ? "ct_lb_mark" : "ct_lb";
+const char *ct_lb_action = features->ct_no_masked_label
+ ? "ct_lb_mark"
+ : "ct_lb";
const char *lb_protocols[] = {"tcp", "udp", "sctp"};
struct ds actions = DS_EMPTY_INITIALIZER;
struct ds match = DS_EMPTY_INITIALIZER;
@@ -5865,7 +5868,9 @@ build_pre_stateful(struct ovn_datapath *od,
}
static void
-build_acl_hints(struct ovn_datapath *od, struct hmap *lflows)
+build_acl_hints(struct ovn_datapath *od,
+const struct chassis_features *features,
+struct hmap *lflows)
{
/* This stage builds hints for the IN/OUT_ACL stage. Based on various
* combinations of ct flags packets may hit only a subset of the logical
@@ -5887,6 +5892,7 @@ build_acl_hints(struct ovn_datapath *od, struct hmap
*lflows)
for (size_t i = 0; i < ARRAY_SIZE(stages); i++) {
enum ovn_stage stage = stages[i];
+const char *match;
/* In any case, advance to the next sta
[ovs-dev] [PATCH ovn v2 3/3] controller: Use ct_mark by default for load balancer hairpin flows.
This allows us to remove the SB_Global.options:lb_hairpin_use_ct_mark
config knob in the future. For now, ovn-northd will just remove it
from the Southbound if it's explicitly set to 'true'.
Signed-off-by: Dumitru Ceara
---
controller/ovn-controller.c |2 +-
northd/northd.c | 12
ovn-sb.xml | 12
3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 4bb33b1ca..2793c8687 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -132,7 +132,7 @@ static const char *ssl_ca_cert_file;
#define DEFAULT_LFLOW_CACHE_TRIM_TO_MS 3
/* SB Global options defaults. */
-#define DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK false
+#define DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK true
struct controller_engine_ctx {
struct lflow_cache *lflow_cache;
diff --git a/northd/northd.c b/northd/northd.c
index 43e87be6b..f120c2366 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -15401,11 +15401,15 @@ ovnnb_db_run(struct northd_input *input_data,
sbrec_sb_global_set_ipsec(sb, nb->ipsec);
}
-/* Inform ovn-controllers whether LB flows will use ct_mark
- * (i.e., only if all chassis support it).
+/* Inform ovn-controllers whether LB flows will use ct_mark (i.e., only
+ * if all chassis support it). If not explicitly present in the database
+ * the default value to be used for this option is 'true'.
*/
-smap_replace(&options, "lb_hairpin_use_ct_mark",
- data->features.ct_no_masked_label ? "true" : "false");
+if (!data->features.ct_no_masked_label) {
+smap_replace(&options, "lb_hairpin_use_ct_mark", "false");
+} else {
+smap_remove(&options, "lb_hairpin_use_ct_mark");
+}
if (!smap_equal(&sb->options, &options)) {
sbrec_sb_global_set_options(sb, &options);
}
diff --git a/ovn-sb.xml b/ovn-sb.xml
index 529e3ee7a..9f47a037e 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -207,10 +207,14 @@
- This value is automatically set to true by
- ovn-northd when action ct_lb_mark is used
- for new load balancer sessions. ovn-controller then
- knows that it should check ct_mark.natted to detect
+ By default this option is turned on (even if not present in the
+ database) unless its value is explicitly set to false.
+
+ This value is automatically set to false by
+ ovn-northd when action ct_lb_mark cannot be
+ used for new load balancer sessions and action ct_lb
+ will be used instead. ovn-controller then
+ knows that it should check ct_label.natted to detect
load balanced traffic.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn 1/2] northd: ovn-controller: Use ct_mark.natted only when ct_lb_mark is used.
On 6/2/22 21:08, Han Zhou wrote:
> On Thu, Jun 2, 2022 at 11:39 AM Dumitru Ceara wrote:
>>
>> On 6/2/22 19:22, Han Zhou wrote:
>>> On Thu, Jun 2, 2022 at 10:09 AM Mark Michelson
> wrote:
On 6/2/22 11:22, Dumitru Ceara wrote:
> Change the way ovn-controller decides whether it should match on
> ct_mark.natted or ct_label.natted for hairpin load balancer traffic.
> Until now this was done solely based on the northd-reported internal
> version.
>
> However, to cover the case when OVN central components are not the
> last
> ones to be updated, ovn-northd now explicitly informs ovn-controller
> whether it should use ct_mark or ct_label.natted via a new option in
> the
> OVN_Southbound.Load_Balancer record: hairpin_use_ct_mark.
>
> Signed-off-by: Dumitru Ceara
> ---
> controller/lflow.c | 32
> controller/lflow.h |1 -
> controller/ovn-controller.c |9 -
> lib/lb.c|3 +++
> lib/lb.h|4
> northd/northd.c |8 ++--
> ovn-sb.xml |8
> tests/ovn-northd.at |2 +-
> 8 files changed, 34 insertions(+), 33 deletions(-)
>
> diff --git a/controller/lflow.c b/controller/lflow.c
> index 7a3419305..0d9184285 100644
> --- a/controller/lflow.c
> +++ b/controller/lflow.c
> @@ -1942,7 +1942,6 @@ add_lb_vip_hairpin_flows(struct
> ovn_controller_lb
>>> *lb,
>struct ovn_lb_vip *lb_vip,
>struct ovn_lb_backend *lb_backend,
>uint8_t lb_proto,
> - bool check_ct_label_for_lb_hairpin,
>struct ovn_desired_flow_table *flow_table)
> {
> uint64_t stub[1024 / 8];
> @@ -2033,18 +2032,19 @@ add_lb_vip_hairpin_flows(struct
>>> ovn_controller_lb *lb,
>* - packets must have ip.src == ip.dst at this point.
>* - the destination protocol and port must be of a valid
> backend
>>> that
>* has the same IP as ip.dst.
> + *
> + * During upgrades logical flow might still use the old way of
>>> storing
> + * ct.natted in ct_label. For backwards compatibility, only use
>>> ct_mark
> + * if ovn-northd notified ovn-controller to do that.
>*/
> -uint32_t lb_ct_mark = OVN_CT_NATTED;
> -match_set_ct_mark_masked(&hairpin_match, lb_ct_mark, lb_ct_mark);
> -
> -ofctrl_add_flow(flow_table, OFTABLE_CHK_LB_HAIRPIN, 100,
> -lb->slb->header_.uuid.parts[0], &hairpin_match,
> -&ofpacts, &lb->slb->header_.uuid);
> +if (lb->hairpin_use_ct_mark) {
> +uint32_t lb_ct_mark = OVN_CT_NATTED;
> +match_set_ct_mark_masked(&hairpin_match, lb_ct_mark,
>>> lb_ct_mark);
>
> -/* The below flow is identical to the above except that it checks
> - * ct_label.natted instead of ct_mark.natted, for backward
>>> compatibility
> - * during the upgrade from a previous version that uses ct_label.
>>> */
> -if (check_ct_label_for_lb_hairpin) {
> +ofctrl_add_flow(flow_table, OFTABLE_CHK_LB_HAIRPIN, 100,
> +lb->slb->header_.uuid.parts[0],
> &hairpin_match,
> +&ofpacts, &lb->slb->header_.uuid);
> +} else {
> match_set_ct_mark_masked(&hairpin_match, 0, 0);
> ovs_u128 lb_ct_label = {
> .u64.lo = OVN_CT_NATTED,
> @@ -2328,7 +2328,6 @@ add_lb_ct_snat_hairpin_flows(struct
>>> ovn_controller_lb *lb,
> static void
> consider_lb_hairpin_flows(const struct sbrec_load_balancer
> *sbrec_lb,
> const struct hmap *local_datapaths,
> - bool check_ct_label_for_lb_hairpin,
> struct ovn_desired_flow_table *flow_table,
> struct simap *ids)
> {
> @@ -2368,7 +2367,6 @@ consider_lb_hairpin_flows(const struct
>>> sbrec_load_balancer *sbrec_lb,
> struct ovn_lb_backend *lb_backend =
> &lb_vip->backends[j];
>
> add_lb_vip_hairpin_flows(lb, lb_vip, lb_backend,
> lb_proto,
> - check_ct_label_for_lb_hairpin,
>flow_table);
> }
> }
> @@ -2383,7 +2381,6 @@ consider_lb_hairpin_flows(const struct
>>> sbrec_load_balancer *sbrec_lb,
> static void
> add_lb_hairpin_flows(const struct sbrec_load_balancer_table
> *lb_table,
>const struct hmap *local_datapaths,
> - bool check_ct_label_for_lb_hairpin,
>struct ovn_desired_flow_table *fl
Re: [ovs-dev] [PATCH v3 11/18] tests: wrap ovs-ofctl calls to test python parser
On 3/11/22 16:21, Adrian Moreno wrote: > Some ovs-ofctl commands are used to parse or dump openflow flows, > specially in ofp-actions.at > > Use a wrapper around ovs-ofctl, called ovs-test-ofparse.py that, apart > from calling ovs-ofctl, also parses its output (or input, depending on > the command) to make sure the python flow parsing library can also parse > the flows. Hi, Adrian. Since you're only touching the ofp-actions.at, it's probably better to just add a separate check for these rules. i.e. create a simpler script that just parses the set of flows without executing ovs-ofctl. I think, it will be much more intuitive for the readers of the test file. In most of the tests you already have the flow as a string or a set of flows in the file. You can sed out the 'bad' ones from these files pretty easily before feeding them to the script. What do you think? And you should add same checks to positive tests in ovs-ofctl.at, otherwise we're missing a big chunk of different matches. The idea of having a wrapper is OK, but for me it makes sense only if it's a global wrapper defined as an alias for all the ovs-ofctl invocations in the testsuite. And even then we could create an alias that will run 2 separate commands without invoking one from another. And since all new actions should be tested in ofp-actions.at and new matches in ovs-ofctl.at, it's probably OK to not have a global alias. > > Acked-by: Eelco Chaudron > Signed-off-by: Adrian Moreno > --- > tests/automake.mk | 3 ++ > tests/ofp-actions.at | 46 > tests/ovs-test-ofparse.py | 107 ++ > 3 files changed, 133 insertions(+), 23 deletions(-) > create mode 100755 tests/ovs-test-ofparse.py > > diff --git a/tests/automake.mk b/tests/automake.mk > index 8a9151f81..230085236 100644 > --- a/tests/automake.mk > +++ b/tests/automake.mk > @@ -19,9 +19,11 @@ EXTRA_DIST += \ > $(OVSDB_CLUSTER_TESTSUITE) \ > tests/atlocal.in \ > $(srcdir)/package.m4 \ > + $(srcdir)/tests/ovs-test-ofparse.py \ > $(srcdir)/tests/testsuite \ > $(srcdir)/tests/testsuite.patch > > + No need for the extra line here. > COMMON_MACROS_AT = \ > tests/ovsdb-macros.at \ > tests/ovs-macros.at \ > @@ -523,6 +525,7 @@ CHECK_PYFILES = \ > tests/flowgen.py \ > tests/mfex_fuzzy.py \ > tests/ovsdb-monitor-sort.py \ > + tests/ovs-test-ofparse.py \ > tests/test-daemon.py \ > tests/test-json.py \ > tests/test-jsonrpc.py \ > diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at > index 9d820eba6..6ee0d4773 100644 > --- a/tests/ofp-actions.at > +++ b/tests/ofp-actions.at > @@ -327,7 +327,7 @@ AT_CAPTURE_FILE([input.txt]) > AT_CAPTURE_FILE([expout]) > AT_CAPTURE_FILE([experr]) > AT_CHECK( > - [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow10 < > input.txt], > + [ovs-test-ofparse.py '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow10 > < input.txt], >[0], [expout], [experr]) Something like: AT_CHECK( [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow10 < input.txt], [0], [expout], [experr]) AT_CHECK([cat expout | grep 'actions=' | ovs-test-ofparse.py]) Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 12/18] tests: Wrap test-odp to also run python parsers
On 3/11/22 16:21, Adrian Moreno wrote: > test-odp is used to parse datapath flow actions and matches within the > odp tests. Wrap calls to this tool in a python script that also parses > them using the python flow parsing library. Same comment here as for 11/18. A separate command should be easier to use and understand. > Acked-by: Eelco Chaudron > Signed-off-by: Adrian Moreno > --- > tests/automake.mk | 2 + > tests/odp.at | 36 - > tests/ovs-test-dpparse.py | 82 +++ > 3 files changed, 102 insertions(+), 18 deletions(-) > create mode 100755 tests/ovs-test-dpparse.py ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 13/18] python: detect changes in flow formatting code
On 3/18/22 11:28, Eelco Chaudron wrote: > > > On 11 Mar 2022, at 16:21, Adrian Moreno wrote: > >> In order to minimize the risk of having the python flow parsing code and >> the C flow formatting code divert, add a target that checks if the >> formatting code has been changed since the last revision and warn the >> developer if it has. >> >> The script also makes it easy to update the dependency file so hopefully >> it will not cause too much trouble for a developer that has modifed the >> file without changing the flow string format. Ugh. Trying to work with this change applied is very annoying. :) I mean, odp-util.c is getting changed frequently, and you have to update the hash every time you change it during development, otherwise the build fails. I'd like to not have this patch. The previous two should cover most of the cases, assuming people are adding tests for every new match/action they are adding. And they really should. Best regards, Ilya Maximets. >> >> Signed-off-by: Adrian Moreno >> --- > > Changes look good to me! > > Acked-by: Eelco Chaudron ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 01/18] python: add generic Key-Value parser
On 3/11/22 16:21, Adrian Moreno wrote: > Most of ofproto and dpif flows are based on key-value pairs. These > key-value pairs can be represented in several ways, eg: key:value, > key=value, key(value). > > Add the following classes that allow parsing of key-value strings: > * KeyValue: holds a key-value pair > * KeyMetadata: holds some metadata associated with a KeyValue such as > the original key and value strings and their position in the global > string > * KVParser: is able to parse a string and extract it's key-value pairs > as KeyValue instances. Before creating the KeyValue instance it tries > to decode the value via the KVDecoders > * KVDecoders holds a number of decoders that KVParser can use to decode > key-value pairs. It accepts a dictionary of keys and callables to > allow users to specify what decoder (i.e: callable) to use for each > key > > Also, flake8 seems to be incorrectly reporting an error (E203) in: > "slice[index + offset : index + offset]" which is PEP8 compliant. So, > ignore this error. > > Signed-off-by: Adrian Moreno > --- > Makefile.am | 3 +- > python/automake.mk | 6 +- > python/ovs/flows/__init__.py | 0 > python/ovs/flows/decoders.py | 18 ++ > python/ovs/flows/kv.py | 314 +++ > python/setup.py | 2 +- > 6 files changed, 340 insertions(+), 3 deletions(-) > create mode 100644 python/ovs/flows/__init__.py > create mode 100644 python/ovs/flows/decoders.py > create mode 100644 python/ovs/flows/kv.py Nit: Can we rename the library to just 'flow' instead of 'flows'? Seems more natural this way. Otherwise, I posted some comments on patches 11-14. If you can address those and post a new version, that would be great. Thanks! Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/2] handlers: Create additional handler threads when using CPU isolation
Michael Santana writes:
> Additional threads are required to service upcalls when we have CPU
> isolation (in per-cpu dispatch mode). The formula used to calculate
> the number of handler threads to create is as follows
>
> handlers_n = min(next_prime(active_cores+1), total_cores)
>
> Where next_prime is a function that returns the next numeric prime
> number that is strictly greater than active_cores
>
> Assume default behavior when total_cores <= 2, that is do not create
> additional threads when we have less than 2 total cores on the system
>
> Fixes: b1e517bd2f81 ("dpif-netlink: Introduce per-cpu upcall dispatch.")
> Signed-off-by: Michael Santana
> ---
> lib/dpif-netlink.c | 87 --
> lib/ovs-thread.c | 16 +
> lib/ovs-thread.h | 1 +
> 3 files changed, 102 insertions(+), 2 deletions(-)
>
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 06e1e8ca0..355ad1d09 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -31,6 +31,7 @@
> #include
> #include
> #include
> +#include
>
> #include "bitmap.h"
> #include "dpif-netlink-rtnl.h"
> @@ -2506,6 +2507,88 @@ dpif_netlink_handler_uninit(struct dpif_handler
> *handler)
> }
> #endif
>
> +/*
> + * Returns 1 if num is a prime number,
> + * Otherwise return 0
> + */
> +static uint32_t
> +is_prime(uint32_t num)
> +{
> +if ((num < 2) || ((num % 2) == 0)) {
> +return num == 2;
> +}
> +
> +uint32_t i;
> +uint32_t limit = sqrt((float) num);
> +for (i = 3; i <= limit; i += 2) {
> +if (num % i == 0) {
> +return 0;
> +}
> +}
> +
> +return 1;
> +}
> +
> +/*
> + * Returns num if num is a prime number. Otherwise returns the next
> + * prime greater than num. Search is limited by the limit variable.
> + *
> + * Returns 0 if num >= limit, or if no prime has been found between
> + * num and limit
> + */
> +static uint32_t
> +next_prime(uint32_t num, uint32_t limit)
> +{
> +if (num < 2) {
> +return 2;
> +}
> +if (num == 2) {
> +return 3;
> +}
> +if (num % 2 == 0) {
> +num++;
> +}
> +
> +uint32_t i;
> +for (i = num; i < limit; i += 2) {
> +if (is_prime(i)) {
> +return i;
> +}
> +}
> +
> +return 0;
> +}
> +
> +/*
> + * Calcuales and returns the number of handler threads needed based
> + * the following formula:
> + *
> + * handlers_n = min(next_prime(active_cores+1), total_cores)
> + *
> + * Where next_prime is a function that returns the next numeric prime
> + * number that is strictly greater than active_cores
> + */
> +static uint32_t
> +dpif_netlink_calculate_handlers_num(void)
> +{
> +uint32_t next_prime_num;
> +uint32_t n_handlers = count_cpu_cores();
> +uint32_t total_cores = count_total_cores();
> +
> +/*
> + * If we have isolated cores, add additional handler threads to
> + * service inactive cores in the unlikely event that traffic goes
> + * through inactive cores
> + */
> +if (n_handlers < total_cores && total_cores > 2) {
> +next_prime_num = next_prime(n_handlers +1, 1000);
Please insert a space after the '+'
Also, there should be some kind of comment why '1000' was chosen
(perhaps a #define or const instead of a raw number as well).
With those addressed,
Acked-by: Aaron Conole
> +n_handlers = next_prime_num >= total_cores ?
> +total_cores : next_prime_num;
> +}
> +
> +return n_handlers;
> +}
> +
> static int
> dpif_netlink_refresh_handlers_cpu_dispatch(struct dpif_netlink *dpif)
> OVS_REQ_WRLOCK(dpif->upcall_lock)
> @@ -2515,7 +2598,7 @@ dpif_netlink_refresh_handlers_cpu_dispatch(struct
> dpif_netlink *dpif)
> uint32_t n_handlers;
> uint32_t *upcall_pids;
>
> -n_handlers = count_cpu_cores();
> +n_handlers = dpif_netlink_calculate_handlers_num();
> if (dpif->n_handlers != n_handlers) {
> VLOG_DBG("Dispatch mode(per-cpu): initializing %d handlers",
> n_handlers);
> @@ -2755,7 +2838,7 @@ dpif_netlink_number_handlers_required(struct dpif
> *dpif_, uint32_t *n_handlers)
> struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
>
> if (dpif_netlink_upcall_per_cpu(dpif)) {
> -*n_handlers = count_cpu_cores();
> +*n_handlers = dpif_netlink_calculate_handlers_num();
> return true;
> }
>
> diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
> index 805cba622..2172b3d3f 100644
> --- a/lib/ovs-thread.c
> +++ b/lib/ovs-thread.c
> @@ -663,6 +663,22 @@ count_cpu_cores(void)
> return n_cores > 0 ? n_cores : 0;
> }
>
> +/* Returns the total number of cores on the system, or 0 if the
> + * number cannot be determined. */
> +int
> +count_total_cores(void) {
> +long int n_cores;
> +
> +#ifndef _WIN32
> +n_cores = sysconf(_SC_NPROCESSORS_CONF);
> +#else
> +n_cores = 0;
> +errno = ENOTSUP;
> +#endif
Re: [ovs-dev] [RFC PATCH 0/6] Remove OVS kernel driver
On Thu, Jun 2, 2022 at 6:58 PM Ilya Maximets wrote: > > On 6/1/22 22:53, Gregory Rose wrote: > > > > > > On 5/31/2022 12:22 PM, Frode Nordahl wrote: > >> On Tue, May 31, 2022 at 7:05 PM Ilya Maximets wrote: > >>> > >>> On 5/31/22 17:36, Gregory Rose wrote: > > > On 5/25/2022 6:47 AM, Flavio Leitner wrote: > > > > Hi Greg, > > > > > > On Mon, May 23, 2022 at 09:10:36PM +0200, Ilya Maximets wrote: > >> On 5/19/22 20:04, Gregory Rose wrote: > >>> > >>> > >>> On 4/15/2022 2:42 PM, Greg Rose wrote: > It is time to remove support for the OVS kernel driver and push > towards use of the upstream Linux openvswitch kernel driver > in it's place [1]. > > This patch series represents a first attempt but there are a few > primary remaining issues that I have yet to address. > > A) Removal of debian packing support for the dkms kernel driver > module. The debian/rules are not well known to me - I've never > actually made any changes in that area and do not have a > well formed understanding of how debian packaging works. I wil > attempt to fix that up in upcoming patch series. > B) Figuring out how the github workflow - I removed the tests I > could find that depend on the Linux kernel (i.e. they use > install_kernel() function. Several other tests are failing > that would not seem to depend on the Linux kernel. I need to > read and understand that code better. > C) There are many Linux specific source modules in the datapath that > will need eventual removal but some headers are still required > for > the userspace code (which seems counterintuitive but...) > > Reviews, suggestions, etc. are appreciated! > > 1. > https://mail.openvswitch.org/pipermail/ovs-dev/2022-April/393292.html > >>> > >>> I would like to suggest at this time that rather than removing the OVS > >>> Linux kernel path that we "freeze" it at Linux 5.8. This will make it > >>> easier for some consumers of OVS that are continuing to support the > >>> Linux kernel datapath in old distributions. > >>> > >>> The ultimate goal of shifting toward DPDK and AFXDP datapaths is still > >>> preserved but we are placing less burden on some consumers of OVS for > >>> older Linux distributions. > >>> > >>> Perhaps in suggesting removal of the kernel datapath I was being a bit > >>> overly aggressive. > >>> > >>> Thoughts? Concerns? Other suggestions? > >> > >> Hi. I think we discussed that before. Removal from the master branch > >> doesn't mean that we will stop supporting the kernel module > >> immediately. > >> It will remain in branch 2.17 which will become our new LTS series > >> soon. > >> This branch will be supported until 2025. And we also talked about > >> possibility of extending the support just for a kernel module on that > >> branch, if required. It's not necassary to use the kernel module and > >> OVS form the same branch, obviously. > >> > >> Removal from the master branch will just make it possible to remove > >> the maintenance burden eventually, not right away. > >> > >> And FWIW, the goal is not to force everyone to use userspace datapath, > >> but remove a maintenance burden and push users to use a better > >> supported > >> version of a code. Frankly, we're not doing a great job supporting the > >> out-of-tree module these days. It's getting hard to backport bug > >> fixes. > >> And will be even harder over time since the code drifts away from the > >> version in the upstream kernel. Mainly because we're not backporting > >> new features for a few years already. > >> > >> Does that make sense? > > > > Any thoughts on this? The freeze time is approaching, so it would > > be great to know your plans for this patch set. > > > > Thanks, > > fbl > > > > Hi Flavio and Ilya, > > I'll go ahead with the plans as per previous agreements - having issues > with removing the debian kernel module support since I have never > worked on debian rules type make environments. I seem to break > something with every attempt but I will keep at it. > > What's my time frame before the freeze? > >>> > >>> The "soft-freeze" supposed to be on July 1st. The branch creation > >>> for a new release - mid July. It would be great if we can get this > >>> in before the soft freeze, but branching point is also fine. > >>> So, we have about 6 weeks. > >>> > >>> If you can think of any part of the work that can be done separately > >>> by someone else, we could try and find someone to help you
Re: [ovs-dev] [PATCH] netdev-linux: skip some internal kernel stats gathering
> On Jun 2, 2022, at 5:50 PM, Ilya Maximets wrote:
>
> On 6/2/22 23:13, Jon Kohler wrote:
>> Any takers? I’m hoping I’ve got the right mailing list, as I did see
>> the thread get generated on the mailing list website?
>
> Hey, Jon.
>
> Yes, it's the right mailing list, though ovs-dev and dev are
> the same thing, so you don't need to send to both.
>
> For the subject, you sent a patch just about a week ago.
> Unfortunately, patches are not moving that fast. People
> are busy with working on issues and doing reviews of other
> patches, so please, be patient.
>
> Just from the patches received last month we still have
> about 60 under review right now. You can see them in the
> patchwork here:
>
> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_openvswitch_list_&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=NGPRGGo37mQiSXgHKm5rCQ&m=8oqFH3XK_nAzOxdHmUuHbWgSEPemwCFQKUdGExfEPEx3NfR3ioDpK5CZHwMdEyf_&s=Osr7hGzYKATNpHa9IvtWygokIFsGsmT4GrnDuUQ3FzY&e=
>
>
> And as you can see we have much older patches too. So, it
> will take some time until someone will get to yours.
>
> Best regards, Ilya Maximets.
Thanks for the feedback and pointers. My apologies, I wasn’t trying to be
ansty :) this is my first patch to this mailing list and wanted to make sure it
was going to the right place.
Ok no problems, I can fully empathize with patch review backlogs :) No
specific rush on this one. Nice CPU win whenever someone can get
to it, but would definitely rather have quality over speed to make sure
we haven’t missed anything! :)
Cheers,
Jon
>
>>
>>> On May 31, 2022, at 10:03 AM, Jon Kohler wrote:
>>>
>>>
>>>
On May 26, 2022, at 9:11 PM, Jon Kohler wrote:
For netdev_linux_update_via_netlink(), hint to the kernel that
we do not need it to gather netlink internal stats when we want
to update the netlink flags, as those stats are not rendered
within OVS.
Background:
ovs-vswitchd can spend quite a bit of time blocked by the kernel
during netlink calls, especially systems with many cores. This
time is dominated by the kernel-side internal stats gathering
mechanism in netlink, specifically:
inet6_fill_link_af
inet6_fill_ifla6_attrs
__snmp6_fill_stats64
In Linux 4.4+, there exists a hint for netlink requests to not
trigger the ipv6 stats gathering mechanism, which greatly reduces
the amount of time that ovs-vswitchd is on CPU.
Testing and Results:
Tested booting 320 VM's and measuring OVS utilization with perf
record, then visualized into a flamegraph using a patched version
of ovs 2.14.2. Calls under bridge_run() seem to get hit the worst
by this issue.
Before bridge_run() == 11.3% of samples
After bridge_run() == 3.4% of samples
Note that there are at least two observed netlink calls under
bridge_run that are still kernel stats heavy after this patch:
Call 1:
bridge_run -> netdev_run -> route_table_run -> route_table_reset ->
ovs_router_insert -> ovs_router_insert__ -> get_src_addr ->
netdev_ger_addr_list -> netdev_linux_get_addr_list -> getifaddrs
Since the actual netlink call is coming from getifaddrs() in glibc,
fixing would likely involve either duplicating glibc code in ovs
source or patch glibc.
Call 2:
bridge_run -> iface_refresh_stats -> netdev_get_stats ->
netdev_linux_get_stats -> get_stats_via_netlink
This does use netlink based stats; however, it isn't immediately
clear if just dropping the stats from inet6_fill_link_af would
impact anything or not. Given this call is more intermittent, its
of lesser concern.
Signed-off-by: Jon Kohler
Acked-by: Greg Smith
>>>
>>> Gentle bump
>>
>>
>>
>>>
---
lib/netdev-linux.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 2766b3f2bf..f0246d3b2b 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -247,6 +247,12 @@ enum {
VALID_NUMA_ID = 1 << 8,
};
+/* Linux 4.4 introduced the ability to skip the internal stats gathering
+ * that netlink does via an external filter mask that can be passed into
+ * a netlink request.
+ */
+#define RTEXT_FILTER_SKIP_STATS (1 << 3)
+
/* Use one for the packet buffer and another for the aux buffer to receive
* TSO packets. */
#define IOV_STD_SIZE 1
@@ -6418,6 +6424,9 @@ netdev_linux_update_via_netlink(struct netdev_linux
*netdev)
if (netdev_linux_netnsid_is_remote(netdev)) {
nl_msg_put_u32(&request, IFLA_IF_NETNSID, netdev->netnsid);
}
+
+nl_msg_put_u32(&request, IFLA_EXT_MASK, RTEXT_FILTER_SKIP_STATS);
+
error = nl_transact(NETLINK_ROUTE, &request, &reply);
ofpbuf_uninit(&req
Re: [ovs-dev] [PATCH ovn v2 0/3] Use masked ct_mark in a backwards compatible way.
Acked-by: Mark Michelson On 6/3/22 09:25, Dumitru Ceara wrote: Explicit backports for these patches (except 3/3 which should only be applied to main) are available at: I'm curious why patch 3 shouldn't be applied to branch-22.06. - branch-22.06: https://github.com/dceara/ovn/tree/bz2091565-ct-masked-mark-follow-up-22.06 - branch-22.03: https://github.com/dceara/ovn/tree/bz2091565-ct-masked-mark-follow-up-22.03 - branch-21.12: https://github.com/dceara/ovn/tree/bz2091565-ct-masked-mark-follow-up-21.12 V2: - Addressed comments from Mark and Han: - added a 3rd patch to flip the lb_hairpin_use_ct_mark default to 'true'. - changed I-P of the northd internal version into I-P of the SB_Global options. - consolidated all ovn-controller ct_mark masked access support under the "ct-no-masked-label" feature name. Dumitru Ceara (3): northd: ovn-controller: Use ct_mark.natted only when ct_lb_mark is used. northd: Use ct_mark.blocked and ecmp_reply_port only when all chassis support it. controller: Use ct_mark by default for load balancer hairpin flows. controller/chassis.c| 5 +- controller/ovn-controller.c | 2 +- include/ovn/features.h | 4 +- northd/northd.c | 172 +++- northd/northd.h | 2 +- ovn-sb.xml | 12 ++- tests/ovn-northd.at | 96 +++- 7 files changed, 222 insertions(+), 71 deletions(-) ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 2/7] Prefix netdev offload flags with NETDEV_OFFLOAD_.
From: Flavio Leitner
Use the 'NETDEV_OFFLOAD_' prefix in the flags to indicate
we are talking about hardware offloading capabilities.
Signed-off-by: Flavio Leitner
Co-authored-by: Mike Pattrick
Signed-off-by: Mike Pattrick
---
lib/netdev-dpdk.c | 20 ++--
lib/netdev-linux.c| 10 +-
lib/netdev-provider.h | 10 +-
lib/netdev.c | 8
4 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index d7350636e..e9b539197 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -4937,12 +4937,12 @@ netdev_dpdk_reconfigure(struct netdev *netdev)
err = dpdk_eth_dev_init(dev);
if (dev->hw_ol_features & NETDEV_TX_TSO_OFFLOAD) {
-netdev->ol_flags |= NETDEV_TX_OFFLOAD_TCP_TSO;
-netdev->ol_flags |= NETDEV_TX_OFFLOAD_TCP_CSUM;
-netdev->ol_flags |= NETDEV_TX_OFFLOAD_UDP_CSUM;
-netdev->ol_flags |= NETDEV_TX_OFFLOAD_IPV4_CSUM;
+netdev->ol_flags |= NETDEV_OFFLOAD_TX_TCP_TSO;
+netdev->ol_flags |= NETDEV_OFFLOAD_TX_TCP_CSUM;
+netdev->ol_flags |= NETDEV_OFFLOAD_TX_UDP_CSUM;
+netdev->ol_flags |= NETDEV_OFFLOAD_TX_IPV4_CSUM;
if (dev->hw_ol_features & NETDEV_TX_SCTP_CHECKSUM_OFFLOAD) {
-netdev->ol_flags |= NETDEV_TX_OFFLOAD_SCTP_CSUM;
+netdev->ol_flags |= NETDEV_OFFLOAD_TX_SCTP_CSUM;
}
}
@@ -5084,11 +5084,11 @@ netdev_dpdk_vhost_client_reconfigure(struct netdev
*netdev)
}
if (userspace_tso_enabled()) {
-netdev->ol_flags |= NETDEV_TX_OFFLOAD_TCP_TSO;
-netdev->ol_flags |= NETDEV_TX_OFFLOAD_TCP_CSUM;
-netdev->ol_flags |= NETDEV_TX_OFFLOAD_UDP_CSUM;
-netdev->ol_flags |= NETDEV_TX_OFFLOAD_SCTP_CSUM;
-netdev->ol_flags |= NETDEV_TX_OFFLOAD_IPV4_CSUM;
+netdev->ol_flags |= NETDEV_OFFLOAD_TX_TCP_TSO;
+netdev->ol_flags |= NETDEV_OFFLOAD_TX_TCP_CSUM;
+netdev->ol_flags |= NETDEV_OFFLOAD_TX_UDP_CSUM;
+netdev->ol_flags |= NETDEV_OFFLOAD_TX_SCTP_CSUM;
+netdev->ol_flags |= NETDEV_OFFLOAD_TX_IPV4_CSUM;
vhost_unsup_flags = 1ULL << VIRTIO_NET_F_HOST_ECN
| 1ULL << VIRTIO_NET_F_HOST_UFO;
} else {
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 3cf02b76a..129043812 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -928,11 +928,11 @@ netdev_linux_common_construct(struct netdev *netdev_)
ovs_mutex_init(&netdev->mutex);
if (userspace_tso_enabled()) {
-netdev_->ol_flags |= NETDEV_TX_OFFLOAD_TCP_TSO;
-netdev_->ol_flags |= NETDEV_TX_OFFLOAD_TCP_CSUM;
-netdev_->ol_flags |= NETDEV_TX_OFFLOAD_UDP_CSUM;
-netdev_->ol_flags |= NETDEV_TX_OFFLOAD_SCTP_CSUM;
-netdev_->ol_flags |= NETDEV_TX_OFFLOAD_IPV4_CSUM;
+netdev_->ol_flags |= NETDEV_OFFLOAD_TX_TCP_TSO;
+netdev_->ol_flags |= NETDEV_OFFLOAD_TX_TCP_CSUM;
+netdev_->ol_flags |= NETDEV_OFFLOAD_TX_UDP_CSUM;
+netdev_->ol_flags |= NETDEV_OFFLOAD_TX_SCTP_CSUM;
+netdev_->ol_flags |= NETDEV_OFFLOAD_TX_IPV4_CSUM;
}
return 0;
diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
index 08bf8b871..0a8538615 100644
--- a/lib/netdev-provider.h
+++ b/lib/netdev-provider.h
@@ -38,11 +38,11 @@ struct netdev_tnl_build_header_params;
#define NETDEV_NUMA_UNSPEC OVS_NUMA_UNSPEC
enum netdev_ol_flags {
-NETDEV_TX_OFFLOAD_IPV4_CSUM = 1 << 0,
-NETDEV_TX_OFFLOAD_TCP_CSUM = 1 << 1,
-NETDEV_TX_OFFLOAD_UDP_CSUM = 1 << 2,
-NETDEV_TX_OFFLOAD_SCTP_CSUM = 1 << 3,
-NETDEV_TX_OFFLOAD_TCP_TSO = 1 << 4,
+NETDEV_OFFLOAD_TX_IPV4_CSUM = 1 << 0,
+NETDEV_OFFLOAD_TX_TCP_CSUM = 1 << 1,
+NETDEV_OFFLOAD_TX_UDP_CSUM = 1 << 2,
+NETDEV_OFFLOAD_TX_SCTP_CSUM = 1 << 3,
+NETDEV_OFFLOAD_TX_TCP_TSO = 1 << 4,
};
/* A network device (e.g. an Ethernet device).
diff --git a/lib/netdev.c b/lib/netdev.c
index e9b2bbe83..a06138aca 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -795,7 +795,7 @@ netdev_send_prepare_packet(const uint64_t netdev_flags,
uint64_t l4_mask;
if (dp_packet_hwol_is_tso(packet)
-&& !(netdev_flags & NETDEV_TX_OFFLOAD_TCP_TSO)) {
+&& !(netdev_flags & NETDEV_OFFLOAD_TX_TCP_TSO)) {
/* Fall back to GSO in software. */
VLOG_ERR_BUF(errormsg, "No TSO support");
return false;
@@ -804,19 +804,19 @@ netdev_send_prepare_packet(const uint64_t netdev_flags,
l4_mask = dp_packet_hwol_l4_mask(packet);
if (l4_mask) {
if (dp_packet_hwol_l4_is_tcp(packet)) {
-if (!(netdev_flags & NETDEV_TX_OFFLOAD_TCP_CSUM)) {
+if (!(netdev_flags & NETDEV_OFFLOAD_TX_TCP_CSUM)) {
/* Fall back to TCP csum in software. */
VLOG_ERR_BUF(errormsg, "No TCP checksum support");
return false;
}
} else if (dp_packet_hwol_l4_i
[ovs-dev] [PATCH 1/7] Rename flags with CKSUM to CSUM.
From: Flavio Leitner
It seems csum is more common and shorter.
Signed-off-by: Flavio Leitner
Co-authored-by: Mike Pattrick
Signed-off-by: Mike Pattrick
---
lib/dp-packet.h | 72 +--
lib/netdev-dpdk.c | 16 +-
lib/netdev-linux.c| 8 ++---
lib/netdev-provider.h | 8 ++---
lib/netdev.c | 6 ++--
5 files changed, 55 insertions(+), 55 deletions(-)
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index bddaa2b5d..3ba5b0665 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -62,14 +62,14 @@ enum dp_packet_offload_mask {
/* Is the 'flow_mark' valid? */
DEF_OL_FLAG(DP_PACKET_OL_FLOW_MARK, RTE_MBUF_F_RX_FDIR_ID, 0x2),
/* Bad L4 checksum in the packet. */
-DEF_OL_FLAG(DP_PACKET_OL_RX_L4_CKSUM_BAD, RTE_MBUF_F_RX_L4_CKSUM_BAD, 0x4),
+DEF_OL_FLAG(DP_PACKET_OL_RX_L4_CSUM_BAD, RTE_MBUF_F_RX_L4_CKSUM_BAD, 0x4),
/* Bad IP checksum in the packet. */
-DEF_OL_FLAG(DP_PACKET_OL_RX_IP_CKSUM_BAD, RTE_MBUF_F_RX_IP_CKSUM_BAD, 0x8),
+DEF_OL_FLAG(DP_PACKET_OL_RX_IP_CSUM_BAD, RTE_MBUF_F_RX_IP_CKSUM_BAD, 0x8),
/* Valid L4 checksum in the packet. */
-DEF_OL_FLAG(DP_PACKET_OL_RX_L4_CKSUM_GOOD, RTE_MBUF_F_RX_L4_CKSUM_GOOD,
+DEF_OL_FLAG(DP_PACKET_OL_RX_L4_CSUM_GOOD, RTE_MBUF_F_RX_L4_CKSUM_GOOD,
0x10),
/* Valid IP checksum in the packet. */
-DEF_OL_FLAG(DP_PACKET_OL_RX_IP_CKSUM_GOOD, RTE_MBUF_F_RX_IP_CKSUM_GOOD,
+DEF_OL_FLAG(DP_PACKET_OL_RX_IP_CSUM_GOOD, RTE_MBUF_F_RX_IP_CKSUM_GOOD,
0x20),
/* TCP Segmentation Offload. */
DEF_OL_FLAG(DP_PACKET_OL_TX_TCP_SEG, RTE_MBUF_F_TX_TCP_SEG, 0x40),
@@ -78,34 +78,34 @@ enum dp_packet_offload_mask {
/* Offloaded packet is IPv6. */
DEF_OL_FLAG(DP_PACKET_OL_TX_IPV6, RTE_MBUF_F_TX_IPV6, 0x100),
/* Offload TCP checksum. */
-DEF_OL_FLAG(DP_PACKET_OL_TX_TCP_CKSUM, RTE_MBUF_F_TX_TCP_CKSUM, 0x200),
+DEF_OL_FLAG(DP_PACKET_OL_TX_TCP_CSUM, RTE_MBUF_F_TX_TCP_CKSUM, 0x200),
/* Offload UDP checksum. */
-DEF_OL_FLAG(DP_PACKET_OL_TX_UDP_CKSUM, RTE_MBUF_F_TX_UDP_CKSUM, 0x400),
+DEF_OL_FLAG(DP_PACKET_OL_TX_UDP_CSUM, RTE_MBUF_F_TX_UDP_CKSUM, 0x400),
/* Offload SCTP checksum. */
-DEF_OL_FLAG(DP_PACKET_OL_TX_SCTP_CKSUM, RTE_MBUF_F_TX_SCTP_CKSUM, 0x800),
+DEF_OL_FLAG(DP_PACKET_OL_TX_SCTP_CSUM, RTE_MBUF_F_TX_SCTP_CKSUM, 0x800),
/* Adding new field requires adding to DP_PACKET_OL_SUPPORTED_MASK. */
};
#define DP_PACKET_OL_SUPPORTED_MASK (DP_PACKET_OL_RSS_HASH | \
DP_PACKET_OL_FLOW_MARK| \
- DP_PACKET_OL_RX_L4_CKSUM_BAD | \
- DP_PACKET_OL_RX_IP_CKSUM_BAD | \
- DP_PACKET_OL_RX_L4_CKSUM_GOOD | \
- DP_PACKET_OL_RX_IP_CKSUM_GOOD | \
+ DP_PACKET_OL_RX_L4_CSUM_BAD | \
+ DP_PACKET_OL_RX_IP_CSUM_BAD | \
+ DP_PACKET_OL_RX_L4_CSUM_GOOD | \
+ DP_PACKET_OL_RX_IP_CSUM_GOOD | \
DP_PACKET_OL_TX_TCP_SEG | \
DP_PACKET_OL_TX_IPV4 | \
DP_PACKET_OL_TX_IPV6 | \
- DP_PACKET_OL_TX_TCP_CKSUM | \
- DP_PACKET_OL_TX_UDP_CKSUM | \
- DP_PACKET_OL_TX_SCTP_CKSUM)
-
-#define DP_PACKET_OL_TX_L4_MASK (DP_PACKET_OL_TX_TCP_CKSUM | \
- DP_PACKET_OL_TX_UDP_CKSUM | \
- DP_PACKET_OL_TX_SCTP_CKSUM)
-#define DP_PACKET_OL_RX_IP_CKSUM_MASK (DP_PACKET_OL_RX_IP_CKSUM_GOOD | \
- DP_PACKET_OL_RX_IP_CKSUM_BAD)
-#define DP_PACKET_OL_RX_L4_CKSUM_MASK (DP_PACKET_OL_RX_L4_CKSUM_GOOD | \
- DP_PACKET_OL_RX_L4_CKSUM_BAD)
+ DP_PACKET_OL_TX_TCP_CSUM | \
+ DP_PACKET_OL_TX_UDP_CSUM | \
+ DP_PACKET_OL_TX_SCTP_CSUM)
+
+#define DP_PACKET_OL_TX_L4_MASK (DP_PACKET_OL_TX_TCP_CSUM | \
+ DP_PACKET_OL_TX_UDP_CSUM | \
+ DP_PACKET_OL_TX_SCTP_CSUM)
+#define DP_PACKET_OL_RX_IP_CSUM_MASK (DP_PACKET_OL_RX_IP_CSUM_GOOD | \
+ DP_PACKET_OL_RX_IP_CSUM_BAD)
+#define DP_PACKET_OL_RX_L4_CSUM_MASK (DP_PACKET_OL_RX_L4_CSUM_GOOD | \
+ DP_PACKET_OL_RX_L4_CSUM_BAD)
/* Buffer for holding packet data. A dp_packet is automatically reallocated
* as necessary if it grows too large for the available memory.
@@ -991,7 +991,7 @@ static inline bool
dp_packet_hwol_l4_is_tcp(const struct dp_packet *
[ovs-dev] [PATCH 7/7] dp-packet: Add _ol_ to functions using OL flags.
From: Flavio Leitner
This helps to identify when it is about the flags or
the packet itself.
Signed-off-by: Flavio Leitner
Co-authored-by: Mike Pattrick
Signed-off-by: Mike Pattrick
---
lib/conntrack.c | 8
lib/dp-packet.c | 2 +-
lib/dp-packet.h | 10 +-
lib/ipf.c | 4 ++--
lib/netdev-native-tnl.c | 4 ++--
lib/netdev-offload-dpdk.c | 2 +-
lib/netdev.c | 2 +-
lib/packets.c | 2 +-
8 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 1e67ee3f6..c77efe362 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2089,12 +2089,12 @@ conn_key_extract(struct conntrack *ct, struct dp_packet
*pkt, ovs_be16 dl_type,
ctx->key.dl_type = dl_type;
if (ctx->key.dl_type == htons(ETH_TYPE_IP)) {
-bool hwol_bad_l3_csum = dp_packet_ip_checksum_bad(pkt);
+bool hwol_bad_l3_csum = dp_packet_ol_ip_checksum_bad(pkt);
if (hwol_bad_l3_csum) {
ok = false;
COVERAGE_INC(conntrack_l3csum_err);
} else {
-bool hwol_good_l3_csum = dp_packet_ip_checksum_valid(pkt)
+bool hwol_good_l3_csum = dp_packet_ol_ip_checksum_good(pkt)
|| dp_packet_ol_tx_ipv4(pkt);
/* Validate the checksum only when hwol is not supported. */
ok = extract_l3_ipv4(&ctx->key, l3, dp_packet_l3_size(pkt), NULL,
@@ -2107,9 +2107,9 @@ conn_key_extract(struct conntrack *ct, struct dp_packet
*pkt, ovs_be16 dl_type,
}
if (ok) {
-bool hwol_bad_l4_csum = dp_packet_l4_checksum_bad(pkt);
+bool hwol_bad_l4_csum = dp_packet_ol_l4_checksum_bad(pkt);
if (!hwol_bad_l4_csum) {
-bool hwol_good_l4_csum = dp_packet_l4_checksum_valid(pkt)
+bool hwol_good_l4_csum = dp_packet_ol_l4_checksum_good(pkt)
|| dp_packet_ol_tx_l4_checksum(pkt);
/* Validate the checksum only when hwol is not supported. */
if (extract_l4(&ctx->key, l4, dp_packet_l4_size(pkt),
diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index d390abbc3..9728565dc 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -33,7 +33,7 @@ dp_packet_init__(struct dp_packet *p, size_t allocated,
dp_packet_reset_offsets(p);
pkt_metadata_init(&p->md, 0);
dp_packet_reset_cutlen(p);
-dp_packet_reset_offload(p);
+dp_packet_ol_reset(p);
/* Initialize implementation-specific fields of dp_packet. */
dp_packet_init_specific(p);
/* By default assume the packet type to be Ethernet. */
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index bf09e2646..633b4ef38 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -935,7 +935,7 @@ dp_packet_rss_valid(const struct dp_packet *p)
}
static inline void
-dp_packet_reset_offload(struct dp_packet *p)
+dp_packet_ol_reset(struct dp_packet *p)
{
*dp_packet_ol_flags_ptr(p) &= ~DP_PACKET_OL_SUPPORTED_MASK;
}
@@ -1058,28 +1058,28 @@ dp_packet_ol_set_tcp_seg(struct dp_packet *a)
}
static inline bool
-dp_packet_ip_checksum_valid(const struct dp_packet *p)
+dp_packet_ol_ip_checksum_good(const struct dp_packet *p)
{
return (*dp_packet_ol_flags_ptr(p) & DP_PACKET_OL_RX_IP_CSUM_MASK) ==
DP_PACKET_OL_RX_IP_CSUM_GOOD;
}
static inline bool
-dp_packet_ip_checksum_bad(const struct dp_packet *p)
+dp_packet_ol_ip_checksum_bad(const struct dp_packet *p)
{
return (*dp_packet_ol_flags_ptr(p) & DP_PACKET_OL_RX_IP_CSUM_MASK) ==
DP_PACKET_OL_RX_IP_CSUM_BAD;
}
static inline bool
-dp_packet_l4_checksum_valid(const struct dp_packet *p)
+dp_packet_ol_l4_checksum_good(const struct dp_packet *p)
{
return (*dp_packet_ol_flags_ptr(p) & DP_PACKET_OL_RX_L4_CSUM_MASK) ==
DP_PACKET_OL_RX_L4_CSUM_GOOD;
}
static inline bool
-dp_packet_l4_checksum_bad(const struct dp_packet *p)
+dp_packet_ol_l4_checksum_bad(const struct dp_packet *p)
{
return (*dp_packet_ol_flags_ptr(p) & DP_PACKET_OL_RX_L4_CSUM_MASK) ==
DP_PACKET_OL_RX_L4_CSUM_BAD;
diff --git a/lib/ipf.c b/lib/ipf.c
index 0e875f509..4f635de11 100644
--- a/lib/ipf.c
+++ b/lib/ipf.c
@@ -574,7 +574,7 @@ ipf_list_state_transition(struct ipf *ipf, struct ipf_list
*ipf_list,
static bool
ipf_is_valid_v4_frag(struct ipf *ipf, struct dp_packet *pkt)
{
-if (OVS_UNLIKELY(dp_packet_ip_checksum_bad(pkt))) {
+if (OVS_UNLIKELY(dp_packet_ol_ip_checksum_bad(pkt))) {
COVERAGE_INC(ipf_l3csum_err);
goto invalid_pkt;
}
@@ -608,7 +608,7 @@ ipf_is_valid_v4_frag(struct ipf *ipf, struct dp_packet *pkt)
goto invalid_pkt;
}
-if (OVS_UNLIKELY(!dp_packet_ip_checksum_valid(pkt)
+if (OVS_UNLIKELY(!dp_packet_ol_ip_checksum_good(pkt)
&& !dp_packet_ol_tx_ipv4(pkt)
&& csum(l3, ip_hdr_len) != 0)) {
COVERAGE_INC(ipf_l3csum_err);
diff --git a/lib/netdev-n
[ovs-dev] [PATCH 3/7] Rename dp_packet_hwol to dp_packet_ol.
From: Flavio Leitner
The name correlates better with the flag names.
Signed-off-by: Flavio Leitner
Co-authored-by: Mike Pattrick
Signed-off-by: Mike Pattrick
---
lib/conntrack.c| 8
lib/dp-packet.h| 28 ++--
lib/ipf.c | 6 +++---
lib/netdev-dpdk.c | 20 ++--
lib/netdev-linux.c | 24
lib/netdev.c | 14 +++---
6 files changed, 50 insertions(+), 50 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 08da4ddf7..f9222f7db 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2095,7 +2095,7 @@ conn_key_extract(struct conntrack *ct, struct dp_packet
*pkt, ovs_be16 dl_type,
COVERAGE_INC(conntrack_l3csum_err);
} else {
bool hwol_good_l3_csum = dp_packet_ip_checksum_valid(pkt)
- || dp_packet_hwol_is_ipv4(pkt);
+ || dp_packet_ol_is_ipv4(pkt);
/* Validate the checksum only when hwol is not supported. */
ok = extract_l3_ipv4(&ctx->key, l3, dp_packet_l3_size(pkt), NULL,
!hwol_good_l3_csum);
@@ -2110,7 +2110,7 @@ conn_key_extract(struct conntrack *ct, struct dp_packet
*pkt, ovs_be16 dl_type,
bool hwol_bad_l4_csum = dp_packet_l4_checksum_bad(pkt);
if (!hwol_bad_l4_csum) {
bool hwol_good_l4_csum = dp_packet_l4_checksum_valid(pkt)
- || dp_packet_hwol_tx_l4_checksum(pkt);
+ || dp_packet_ol_tx_l4_checksum(pkt);
/* Validate the checksum only when hwol is not supported. */
if (extract_l4(&ctx->key, l4, dp_packet_l4_size(pkt),
&ctx->icmp_related, l3, !hwol_good_l4_csum,
@@ -3441,7 +3441,7 @@ handle_ftp_ctl(struct conntrack *ct, const struct
conn_lookup_ctx *ctx,
}
if (seq_skew) {
ip_len = ntohs(l3_hdr->ip_tot_len) + seq_skew;
-if (!dp_packet_hwol_is_ipv4(pkt)) {
+if (!dp_packet_ol_is_ipv4(pkt)) {
l3_hdr->ip_csum = recalc_csum16(l3_hdr->ip_csum,
l3_hdr->ip_tot_len,
htons(ip_len));
@@ -3463,7 +3463,7 @@ handle_ftp_ctl(struct conntrack *ct, const struct
conn_lookup_ctx *ctx,
}
th->tcp_csum = 0;
-if (!dp_packet_hwol_tx_l4_checksum(pkt)) {
+if (!dp_packet_ol_tx_l4_checksum(pkt)) {
if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
th->tcp_csum = packet_csum_upperlayer6(nh6, th, ctx->key.nw_proto,
dp_packet_l4_size(pkt));
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 3ba5b0665..968ec7534 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -960,35 +960,35 @@ dp_packet_set_flow_mark(struct dp_packet *p, uint32_t
mark)
/* Returns the L4 cksum offload bitmask. */
static inline uint64_t
-dp_packet_hwol_l4_mask(const struct dp_packet *b)
+dp_packet_ol_l4_mask(const struct dp_packet *b)
{
return *dp_packet_ol_flags_ptr(b) & DP_PACKET_OL_TX_L4_MASK;
}
/* Return true if the packet 'b' requested L4 checksum offload. */
static inline bool
-dp_packet_hwol_tx_l4_checksum(const struct dp_packet *b)
+dp_packet_ol_tx_l4_checksum(const struct dp_packet *b)
{
-return !!dp_packet_hwol_l4_mask(b);
+return !!dp_packet_ol_l4_mask(b);
}
/* Returns 'true' if packet 'b' is marked for TCP segmentation offloading. */
static inline bool
-dp_packet_hwol_is_tso(const struct dp_packet *b)
+dp_packet_ol_is_tso(const struct dp_packet *b)
{
return !!(*dp_packet_ol_flags_ptr(b) & DP_PACKET_OL_TX_TCP_SEG);
}
/* Returns 'true' if packet 'b' is marked for IPv4 checksum offloading. */
static inline bool
-dp_packet_hwol_is_ipv4(const struct dp_packet *b)
+dp_packet_ol_is_ipv4(const struct dp_packet *b)
{
return !!(*dp_packet_ol_flags_ptr(b) & DP_PACKET_OL_TX_IPV4);
}
/* Returns 'true' if packet 'b' is marked for TCP checksum offloading. */
static inline bool
-dp_packet_hwol_l4_is_tcp(const struct dp_packet *b)
+dp_packet_ol_l4_is_tcp(const struct dp_packet *b)
{
return (*dp_packet_ol_flags_ptr(b) & DP_PACKET_OL_TX_L4_MASK) ==
DP_PACKET_OL_TX_TCP_CSUM;
@@ -996,7 +996,7 @@ dp_packet_hwol_l4_is_tcp(const struct dp_packet *b)
/* Returns 'true' if packet 'b' is marked for UDP checksum offloading. */
static inline bool
-dp_packet_hwol_l4_is_udp(struct dp_packet *b)
+dp_packet_ol_l4_is_udp(struct dp_packet *b)
{
return (*dp_packet_ol_flags_ptr(b) & DP_PACKET_OL_TX_L4_MASK) ==
DP_PACKET_OL_TX_UDP_CSUM;
@@ -1004,7 +1004,7 @@ dp_packet_hwol_l4_is_udp(struct dp_packet *b)
/* Returns 'true' if packet 'b' is marked for SCTP checksum offloading. */
static inline bool
-dp_packet_hwol_l4_is_sctp(struct dp_packet *b)
+dp_packet_ol_l4
[ovs-dev] [PATCH 5/7] dp-packet: Rename dp_packet_ol_tcp_seg
From: Flavio Leitner
Rename to dp_packet_ol_tcp_seg, because that is less
redundant and allows other protocols.
Signed-off-by: Flavio Leitner
Co-authored-by: Mike Pattrick
Signed-off-by: Mike Pattrick
---
lib/dp-packet.h| 2 +-
lib/netdev-linux.c | 2 +-
lib/netdev.c | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index 1b17ef263..fdc0882ab 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -974,7 +974,7 @@ dp_packet_ol_tx_l4_checksum(const struct dp_packet *a)
/* Returns 'true' if packet 'a' is marked for TCP segmentation offloading. */
static inline bool
-dp_packet_ol_is_tso(const struct dp_packet *a)
+dp_packet_ol_tcp_seg(const struct dp_packet *a)
{
return !!(*dp_packet_ol_flags_ptr(a) & DP_PACKET_OL_TX_TCP_SEG);
}
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 490c751b6..47765032c 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -6682,7 +6682,7 @@ netdev_linux_prepend_vnet_hdr(struct dp_packet *p, int
mtu)
{
struct virtio_net_hdr *vnet = dp_packet_push_zeros(p, sizeof *vnet);
-if (dp_packet_ol_is_tso(p)) {
+if (dp_packet_ol_tcp_seg(p)) {
uint16_t hdr_len = ((char *) dp_packet_l4(p)
- (char *) dp_packet_eth(p)) + TCP_HEADER_LEN;
diff --git a/lib/netdev.c b/lib/netdev.c
index d087929e5..fb535ed7c 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -794,7 +794,7 @@ netdev_send_prepare_packet(const uint64_t netdev_flags,
{
uint64_t l4_mask;
-if (dp_packet_ol_is_tso(packet)
+if (dp_packet_ol_tcp_seg(packet)
&& !(netdev_flags & NETDEV_OFFLOAD_TX_TCP_TSO)) {
/* Fall back to GSO in software. */
VLOG_ERR_BUF(errormsg, "No TSO support");
@@ -960,7 +960,7 @@ netdev_push_header(const struct netdev *netdev,
size_t i, size = dp_packet_batch_size(batch);
DP_PACKET_BATCH_REFILL_FOR_EACH (i, size, packet, batch) {
-if (OVS_UNLIKELY(dp_packet_ol_is_tso(packet)
+if (OVS_UNLIKELY(dp_packet_ol_tcp_seg(packet)
|| dp_packet_ol_l4_mask(packet))) {
COVERAGE_INC(netdev_push_header_drops);
dp_packet_delete(packet);
--
2.27.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 6/7] dp-packet: Rename dp_packet_ol l4 functions.
From: Flavio Leitner
Rename to better represent their flags.
Signed-off-by: Flavio Leitner
Co-authored-by: Mike Pattrick
Signed-off-by: Mike Pattrick
---
lib/conntrack.c| 4 ++--
lib/dp-packet.h| 14 +++---
lib/ipf.c | 6 +++---
lib/netdev-linux.c | 14 +++---
lib/netdev.c | 16 +++-
5 files changed, 26 insertions(+), 28 deletions(-)
diff --git a/lib/conntrack.c b/lib/conntrack.c
index f9222f7db..1e67ee3f6 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -2095,7 +2095,7 @@ conn_key_extract(struct conntrack *ct, struct dp_packet
*pkt, ovs_be16 dl_type,
COVERAGE_INC(conntrack_l3csum_err);
} else {
bool hwol_good_l3_csum = dp_packet_ip_checksum_valid(pkt)
- || dp_packet_ol_is_ipv4(pkt);
+ || dp_packet_ol_tx_ipv4(pkt);
/* Validate the checksum only when hwol is not supported. */
ok = extract_l3_ipv4(&ctx->key, l3, dp_packet_l3_size(pkt), NULL,
!hwol_good_l3_csum);
@@ -3441,7 +3441,7 @@ handle_ftp_ctl(struct conntrack *ct, const struct
conn_lookup_ctx *ctx,
}
if (seq_skew) {
ip_len = ntohs(l3_hdr->ip_tot_len) + seq_skew;
-if (!dp_packet_ol_is_ipv4(pkt)) {
+if (!dp_packet_ol_tx_ipv4(pkt)) {
l3_hdr->ip_csum = recalc_csum16(l3_hdr->ip_csum,
l3_hdr->ip_tot_len,
htons(ip_len));
diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index fdc0882ab..bf09e2646 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -981,14 +981,14 @@ dp_packet_ol_tcp_seg(const struct dp_packet *a)
/* Returns 'true' if packet 'a' is marked for IPv4 checksum offloading. */
static inline bool
-dp_packet_ol_is_ipv4(const struct dp_packet *a)
+dp_packet_ol_tx_ipv4(const struct dp_packet *a)
{
return !!(*dp_packet_ol_flags_ptr(a) & DP_PACKET_OL_TX_IPV4);
}
/* Returns 'true' if packet 'a' is marked for TCP checksum offloading. */
static inline bool
-dp_packet_ol_l4_is_tcp(const struct dp_packet *a)
+dp_packet_ol_tx_tcp_csum(const struct dp_packet *a)
{
return (*dp_packet_ol_flags_ptr(a) & DP_PACKET_OL_TX_L4_MASK) ==
DP_PACKET_OL_TX_TCP_CSUM;
@@ -996,7 +996,7 @@ dp_packet_ol_l4_is_tcp(const struct dp_packet *a)
/* Returns 'true' if packet 'a' is marked for UDP checksum offloading. */
static inline bool
-dp_packet_ol_l4_is_udp(struct dp_packet *a)
+dp_packet_ol_tx_udp_csum(struct dp_packet *a)
{
return (*dp_packet_ol_flags_ptr(a) & DP_PACKET_OL_TX_L4_MASK) ==
DP_PACKET_OL_TX_UDP_CSUM;
@@ -1004,7 +1004,7 @@ dp_packet_ol_l4_is_udp(struct dp_packet *a)
/* Returns 'true' if packet 'a' is marked for SCTP checksum offloading. */
static inline bool
-dp_packet_ol_l4_is_sctp(struct dp_packet *a)
+dp_packet_ol_tx_sctp_csum(struct dp_packet *a)
{
return (*dp_packet_ol_flags_ptr(a) & DP_PACKET_OL_TX_L4_MASK) ==
DP_PACKET_OL_TX_SCTP_CSUM;
@@ -1027,7 +1027,7 @@ dp_packet_ol_set_tx_ipv6(struct dp_packet *a)
/* Mark packet 'a' for TCP checksum offloading. It implies that either
* the packet 'a' is marked for IPv4 or IPv6 checksum offloading. */
static inline void
-dp_packet_ol_set_csum_tcp(struct dp_packet *a)
+dp_packet_ol_set_tx_tcp_csum(struct dp_packet *a)
{
*dp_packet_ol_flags_ptr(a) |= DP_PACKET_OL_TX_TCP_CSUM;
}
@@ -1035,7 +1035,7 @@ dp_packet_ol_set_csum_tcp(struct dp_packet *a)
/* Mark packet 'a' for UDP checksum offloading. It implies that either
* the packet 'a' is marked for IPv4 or IPv6 checksum offloading. */
static inline void
-dp_packet_ol_set_csum_udp(struct dp_packet *a)
+dp_packet_ol_set_tx_udp_csum(struct dp_packet *a)
{
*dp_packet_ol_flags_ptr(a) |= DP_PACKET_OL_TX_UDP_CSUM;
}
@@ -1043,7 +1043,7 @@ dp_packet_ol_set_csum_udp(struct dp_packet *a)
/* Mark packet 'a' for SCTP checksum offloading. It implies that either
* the packet 'a' is marked for IPv4 or IPv6 checksum offloading. */
static inline void
-dp_packet_ol_set_csum_sctp(struct dp_packet *a)
+dp_packet_ol_set_tx_sctp_csum(struct dp_packet *a)
{
*dp_packet_ol_flags_ptr(a) |= DP_PACKET_OL_TX_SCTP_CSUM;
}
diff --git a/lib/ipf.c b/lib/ipf.c
index df9dd01dd..0e875f509 100644
--- a/lib/ipf.c
+++ b/lib/ipf.c
@@ -433,7 +433,7 @@ ipf_reassemble_v4_frags(struct ipf_list *ipf_list)
len += rest_len;
l3 = dp_packet_l3(pkt);
ovs_be16 new_ip_frag_off = l3->ip_frag_off & ~htons(IP_MORE_FRAGMENTS);
-if (!dp_packet_ol_is_ipv4(pkt)) {
+if (!dp_packet_ol_tx_ipv4(pkt)) {
l3->ip_csum = recalc_csum16(l3->ip_csum, l3->ip_frag_off,
new_ip_frag_off);
l3->ip_csum = recalc_csum16(l3->ip_csum, l3->ip_tot_len, htons(len));
@@ -609,7 +609,7 @@ ipf_is_valid_v4_frag(struct
[ovs-dev] [PATCH 4/7] dp-packet: Use p for packet and b for batch.
From: Flavio Leitner
Currently 'p' and 'b' and used for packets, so use
a convention that struct dp_packet is 'p' and
struct dp_packet_batch is 'b'.
Some comments needed new formatting to not pass the
80 column.
Some variables were using 'p' or 'b' were renamed
as well.
There should be no functional change with this patch.
Signed-off-by: Flavio Leitner
Co-authored-by: Mike Pattrick
Signed-off-by: Mike Pattrick
---
lib/dp-packet.c| 345 +++
lib/dp-packet.h| 504 ++---
lib/netdev-dummy.c | 8 +-
lib/netdev-linux.c | 56 ++---
4 files changed, 457 insertions(+), 456 deletions(-)
diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index 35c72542a..d390abbc3 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -25,58 +25,59 @@
#include "util.h"
static void
-dp_packet_init__(struct dp_packet *b, size_t allocated, enum dp_packet_source
source)
-{
-dp_packet_set_allocated(b, allocated);
-b->source = source;
-dp_packet_reset_offsets(b);
-pkt_metadata_init(&b->md, 0);
-dp_packet_reset_cutlen(b);
-dp_packet_reset_offload(b);
+dp_packet_init__(struct dp_packet *p, size_t allocated,
+ enum dp_packet_source source)
+{
+dp_packet_set_allocated(p, allocated);
+p->source = source;
+dp_packet_reset_offsets(p);
+pkt_metadata_init(&p->md, 0);
+dp_packet_reset_cutlen(p);
+dp_packet_reset_offload(p);
/* Initialize implementation-specific fields of dp_packet. */
-dp_packet_init_specific(b);
+dp_packet_init_specific(p);
/* By default assume the packet type to be Ethernet. */
-b->packet_type = htonl(PT_ETH);
+p->packet_type = htonl(PT_ETH);
}
static void
-dp_packet_use__(struct dp_packet *b, void *base, size_t allocated,
+dp_packet_use__(struct dp_packet *p, void *base, size_t allocated,
enum dp_packet_source source)
{
-dp_packet_set_base(b, base);
-dp_packet_set_data(b, base);
-dp_packet_set_size(b, 0);
+dp_packet_set_base(p, base);
+dp_packet_set_data(p, base);
+dp_packet_set_size(p, 0);
-dp_packet_init__(b, allocated, source);
+dp_packet_init__(p, allocated, source);
}
-/* Initializes 'b' as an empty dp_packet that contains the 'allocated' bytes of
+/* Initializes 'p' as an empty dp_packet that contains the 'allocated' bytes of
* memory starting at 'base'. 'base' should be the first byte of a region
- * obtained from malloc(). It will be freed (with free()) if 'b' is resized or
+ * obtained from malloc(). It will be freed (with free()) if 'p' is resized or
* freed. */
void
-dp_packet_use(struct dp_packet *b, void *base, size_t allocated)
+dp_packet_use(struct dp_packet *p, void *base, size_t allocated)
{
-dp_packet_use__(b, base, allocated, DPBUF_MALLOC);
+dp_packet_use__(p, base, allocated, DPBUF_MALLOC);
}
#if HAVE_AF_XDP
-/* Initialize 'b' as an empty dp_packet that contains
+/* Initialize 'p' as an empty dp_packet that contains
* memory starting at AF_XDP umem base.
*/
void
-dp_packet_use_afxdp(struct dp_packet *b, void *data, size_t allocated,
+dp_packet_use_afxdp(struct dp_packet *p, void *data, size_t allocated,
size_t headroom)
{
-dp_packet_set_base(b, (char *)data - headroom);
-dp_packet_set_data(b, data);
-dp_packet_set_size(b, 0);
+dp_packet_set_base(p, (char *) data - headroom);
+dp_packet_set_data(p, data);
+dp_packet_set_size(p, 0);
-dp_packet_init__(b, allocated, DPBUF_AFXDP);
+dp_packet_init__(p, allocated, DPBUF_AFXDP);
}
#endif
-/* Initializes 'b' as an empty dp_packet that contains the 'allocated' bytes of
+/* Initializes 'p' as an empty dp_packet that contains the 'allocated' bytes of
* memory starting at 'base'. 'base' should point to a buffer on the stack.
* (Nothing actually relies on 'base' being allocated on the stack. It could
* be static or malloc()'d memory. But stack space is the most common use
@@ -91,56 +92,56 @@ dp_packet_use_afxdp(struct dp_packet *b, void *data, size_t
allocated,
* on an dp_packet initialized by this function, so that if it expanded into
the
* heap, that memory is freed. */
void
-dp_packet_use_stub(struct dp_packet *b, void *base, size_t allocated)
+dp_packet_use_stub(struct dp_packet *p, void *base, size_t allocated)
{
-dp_packet_use__(b, base, allocated, DPBUF_STUB);
+dp_packet_use__(p, base, allocated, DPBUF_STUB);
}
-/* Initializes 'b' as an dp_packet whose data starts at 'data' and continues
for
- * 'size' bytes. This is appropriate for an dp_packet that will be used to
- * inspect existing data, without moving it around or reallocating it, and
+/* Initializes 'p' as an dp_packet whose data starts at 'data' and continues
+ * for 'size' bytes. This is appropriate for an dp_packet that will be used
+ * to inspect existing data, without moving it around or reallocating it, and
* generally without modifying it at all.
*
* A
[ovs-dev] [PATCH 0/7] dp-packet: Refactor offload related names
This patch refactors several function, variable, and definition names for consistency and brevity. cksum is changed to csum, p is the default name for a dp_packet, b is the default name for a dp_packet_batch, ol instead of hwol, and some checksum offload related functions are renamed to indicate direction. Flavio Leitner (7): Rename flags with CKSUM to CSUM. Prefix netdev offload flags with NETDEV_OFFLOAD_. Rename dp_packet_hwol to dp_packet_ol. dp-packet: Use p for packet and b for batch. dp-packet: Rename dp_packet_ol_tcp_seg dp-packet: Rename dp_packet_ol l4 functions. dp-packet: Add _ol_ to functions using OL flags. lib/conntrack.c | 16 +- lib/dp-packet.c | 336 +++--- lib/dp-packet.h | 580 +++--- lib/ipf.c | 10 +- lib/netdev-dpdk.c | 40 +-- lib/netdev-dummy.c| 8 +- lib/netdev-linux.c| 64 ++--- lib/netdev-native-tnl.c | 4 +- lib/netdev-offload-dpdk.c | 2 +- lib/netdev-provider.h | 10 +- lib/netdev.c | 32 +-- lib/packets.c | 2 +- 12 files changed, 551 insertions(+), 553 deletions(-) -- 2.27.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v2 0/3] Use masked ct_mark in a backwards compatible way.
On 6/3/22 17:08, Mark Michelson wrote: > Acked-by: Mark Michelson > > On 6/3/22 09:25, Dumitru Ceara wrote: >> Explicit backports for these patches (except 3/3 which should only be >> applied >> to main) are available at: > > I'm curious why patch 3 shouldn't be applied to branch-22.06. > It should, you're right. :) >> - branch-22.06: >> https://github.com/dceara/ovn/tree/bz2091565-ct-masked-mark-follow-up-22.06 >> >> - branch-22.03: >> https://github.com/dceara/ovn/tree/bz2091565-ct-masked-mark-follow-up-22.03 >> >> - branch-21.12: >> https://github.com/dceara/ovn/tree/bz2091565-ct-masked-mark-follow-up-21.12 >> >> >> V2: >> - Addressed comments from Mark and Han: >> - added a 3rd patch to flip the lb_hairpin_use_ct_mark default to >> 'true'. >> - changed I-P of the northd internal version into I-P of the SB_Global >> options. >> - consolidated all ovn-controller ct_mark masked access support under >> the "ct-no-masked-label" feature name. >> >> Dumitru Ceara (3): >> northd: ovn-controller: Use ct_mark.natted only when ct_lb_mark >> is used. >> northd: Use ct_mark.blocked and ecmp_reply_port only when all >> chassis support it. >> controller: Use ct_mark by default for load balancer hairpin >> flows. >> >> controller/chassis.c | 5 +- >> controller/ovn-controller.c | 2 +- >> include/ovn/features.h | 4 +- >> northd/northd.c | 172 +++- >> northd/northd.h | 2 +- >> ovn-sb.xml | 12 ++- >> tests/ovn-northd.at | 96 +++- >> 7 files changed, 222 insertions(+), 71 deletions(-) >> > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4] ofproto-dpif-xlate: No clone when tunnel push is last action
When OVS sees a tunnel push with a nested list next, it will not
clone the packet, as a clone is not needed. However, a clone action will
still be created with the tunnel push encapulated inside. There is no
need to create the clone action in this case, as extra parsing will need
to be performed, which is less efficient.
Signed-off-by: Rosemarie O'Riorden
---
v2:
- Fixes tests that are affected by the change
v3:
- Adds support for hardware offloading
v4:
- Adds negative check in test
- Fixes logic in function native_tunnel_output
- Stylistic and organizational improvements
lib/netdev-offload-dpdk.c | 38 +-
lib/netlink.c | 7 +++
lib/netlink.h | 1 +
ofproto/ofproto-dpif-xlate.c | 22 ++--
tests/nsh.at | 6 +--
tests/packet-type-aware.at| 24 -
tests/tunnel-push-pop-ipv6.at | 20 +++
tests/tunnel-push-pop.at | 98 ---
8 files changed, 152 insertions(+), 64 deletions(-)
diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 12d299603..44a2be9c3 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -2058,6 +2058,25 @@ parse_vlan_push_action(struct flow_actions *actions,
return 0;
}
+static void
+add_tunnel_push_action(struct flow_actions *actions,
+ const struct ovs_action_push_tnl *tnl_push)
+{
+ struct rte_flow_action_raw_encap *raw_encap;
+
+ if (tnl_push->tnl_type == OVS_VPORT_TYPE_VXLAN &&
+ !add_vxlan_encap_action(actions, tnl_push->header)) {
+ return;
+ }
+
+ raw_encap = xzalloc(sizeof *raw_encap);
+ raw_encap->data = (uint8_t *) tnl_push->header;
+ raw_encap->preserve = NULL;
+ raw_encap->size = tnl_push->header_len;
+
+ add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RAW_ENCAP, raw_encap);
+}
+
static int
parse_clone_actions(struct netdev *netdev,
struct flow_actions *actions,
@@ -2072,20 +2091,7 @@ parse_clone_actions(struct netdev *netdev,
if (clone_type == OVS_ACTION_ATTR_TUNNEL_PUSH) {
const struct ovs_action_push_tnl *tnl_push = nl_attr_get(ca);
-struct rte_flow_action_raw_encap *raw_encap;
-
-if (tnl_push->tnl_type == OVS_VPORT_TYPE_VXLAN &&
-!add_vxlan_encap_action(actions, tnl_push->header)) {
-continue;
-}
-
-raw_encap = xzalloc(sizeof *raw_encap);
-raw_encap->data = (uint8_t *) tnl_push->header;
-raw_encap->preserve = NULL;
-raw_encap->size = tnl_push->header_len;
-
-add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RAW_ENCAP,
-raw_encap);
+add_tunnel_push_action(actions, tnl_push);
} else if (clone_type == OVS_ACTION_ATTR_OUTPUT) {
if (add_output_action(netdev, actions, ca)) {
return -1;
@@ -2188,6 +2194,10 @@ parse_flow_actions(struct netdev *netdev,
}
} else if (nl_attr_type(nla) == OVS_ACTION_ATTR_POP_VLAN) {
add_flow_action(actions, RTE_FLOW_ACTION_TYPE_OF_POP_VLAN, NULL);
+} else if (nl_attr_type(nla) == OVS_ACTION_ATTR_TUNNEL_PUSH) {
+const struct ovs_action_push_tnl *tnl_push = nl_attr_get(nla);
+
+add_tunnel_push_action(actions, tnl_push);
} else if (nl_attr_type(nla) == OVS_ACTION_ATTR_CLONE &&
left <= NLA_ALIGN(nla->nla_len)) {
const struct nlattr *clone_actions = nl_attr_get(nla);
diff --git a/lib/netlink.c b/lib/netlink.c
index 8204025a5..46043ce97 100644
--- a/lib/netlink.c
+++ b/lib/netlink.c
@@ -570,6 +570,13 @@ nl_msg_put_nested(struct ofpbuf *msg,
nl_msg_end_nested(msg, offset);
}
+/* Reset message size to offset */
+void
+nl_msg_reset_size(struct ofpbuf *msg, size_t offset)
+{
+msg->size = offset;
+}
+
/* If 'buffer' begins with a valid "struct nlmsghdr", pulls the header and its
* payload off 'buffer', stores header and payload in 'msg->data' and
* 'msg->size', and returns a pointer to the header.
diff --git a/lib/netlink.h b/lib/netlink.h
index b97470743..e9050c31b 100644
--- a/lib/netlink.h
+++ b/lib/netlink.h
@@ -86,6 +86,7 @@ void nl_msg_cancel_nested(struct ofpbuf *, size_t offset);
bool nl_msg_end_non_empty_nested(struct ofpbuf *, size_t offset);
void nl_msg_put_nested(struct ofpbuf *, uint16_t type,
const void *data, size_t size);
+void nl_msg_reset_size(struct ofpbuf *, size_t offset);
/* Prepending attributes. */
void *nl_msg_push_unspec_uninit(struct ofpbuf *, uint16_t type, size_t);
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 9ea21edc4..a908160a5 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3739,7 +3739,11 @@ native_tunnel_output(struct xlate_ctx *ctx, const struct
xport *xport,
size_t clone_ofs = 0;
size_t push_action_size;
-cl
Re: [ovs-dev] [PATCH] ovs-tcpdump: Fix error when stopping ovs-tcpdump.
On Fri, May 20, 2022 at 5:55 AM Han Ding wrote: > > > Sometimes we need to dump packets on more than two interfaces in a bridge > at the same time. Then when we stop dumping in order, ovs-tcpdump print > traceback and fail to delete mirror interface for some interface. > > For example: > br-int has two interface tap1 and br-int. We use ovs-tcpdump dump tap1 first > and dump br-int next. Then stopping tap1 ovs-tcpdump first, and stopping > br-int second. When we stop ovs-tcpdump for br-int, the screen show the error > like this: > __main__.OVSDBException: Unable to delete Mirror m_br-int > > Signed-off-by: Han Ding I can confirm that this issue does occur, and this patch fixes it. Acked-by: Mike Pattrick Small nit but instead of closing and reopening the OVSDB object, maybe we should just atexit.register the call to ovsdb.close_idl. Cheers, M > --- > utilities/ovs-tcpdump.in | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/utilities/ovs-tcpdump.in b/utilities/ovs-tcpdump.in > index 82d1bedfa..6188be5c4 100755 > --- a/utilities/ovs-tcpdump.in > +++ b/utilities/ovs-tcpdump.in > @@ -164,6 +164,9 @@ class OVSDB(object): > schema.register_all() > self._idl_conn = idl.Idl(db_sock, schema) > OVSDB.wait_for_db_change(self._idl_conn) # Initial Sync with DB > + > +def close_idl(self): > +self._idl_conn.close() > > def _get_schema(self): > error, strm = Stream.open_block(Stream.open(self._db_sock)) > @@ -500,6 +503,8 @@ def main(): > pass > sys.exit(1) > > +ovsdb.close_idl() > + > pipes = _doexec(*([dump_cmd, '-i', mirror_interface] + tcpdargs)) > try: > while pipes.poll() is None: > @@ -512,6 +517,7 @@ def main(): > if pipes.poll() is None: > pipes.terminate() > > +ovsdb = OVSDB(db_sock) > ovsdb.destroy_mirror(interface, ovsdb.port_bridge(interface)) > ovsdb.destroy_port(mirror_interface, ovsdb.port_bridge(interface)) > if tap_created is True: > -- > 2.27.0 > > > > > ___ > 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] [PATCH ovn branch-21.12 1/2] Set release date for 21.12.2.
Signed-off-by: Mark Michelson --- NEWS | 3 ++- debian/changelog | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index de3330ab6..6ff9b8fdf 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,6 @@ -OVN v21.12.2 - xx xxx +OVN v21.12.2 - 03 Jun 2022 -- + - Bug fixes - When configured to log packets matching ACLs, log the direction (logical pipeline) too. - Replaced the usage of masked ct_label by ct_mark in most cases to work diff --git a/debian/changelog b/debian/changelog index 1a1c7364e..5bf39c320 100644 --- a/debian/changelog +++ b/debian/changelog @@ -2,7 +2,7 @@ OVN (21.12.2-1) unstable; urgency=low [ OVN team ] * New upstream version - -- OVN team Fri, 11 Mar 2022 13:22:24 -0500 + -- OVN team Fri, 03 Jun 2022 11:53:58 -0400 OVN (21.12.1-1) unstable; urgency=low [ OVN team ] -- 2.31.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-22.03 1/2] Set release date for 22.03.1.
Signed-off-by: Mark Michelson --- NEWS | 3 ++- debian/changelog | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 700a6830a..166470747 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,6 @@ -OVN v22.03.1 - xx xxx +OVN v22.03.1 - 03 Jun 2022 -- + - Bug fixes - Replaced the usage of masked ct_label by ct_mark in most cases to work better with hardware-offloading. diff --git a/debian/changelog b/debian/changelog index 18a1a042e..1b9ebca1b 100644 --- a/debian/changelog +++ b/debian/changelog @@ -2,7 +2,7 @@ OVN (22.03.1-1) unstable; urgency=low [ OVN team ] * New upstream version - -- OVN team Fri, 11 Mar 2022 13:22:32 -0500 + -- OVN team Fri, 03 Jun 2022 11:54:04 -0400 ovn (22.03.0-1) unstable; urgency=low -- 2.31.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-22.06 0/2] Release patches for v22.06.0.
Mark Michelson (2): Set release date for 22.06.0. Prepare for 22.06.1. NEWS | 5 - configure.ac | 2 +- debian/changelog | 8 +++- 3 files changed, 12 insertions(+), 3 deletions(-) -- 2.31.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-22.03 0/2] Release patches for v22.03.1.
Mark Michelson (2): Set release date for 22.03.1. Prepare for 22.03.2. NEWS | 6 +- configure.ac | 2 +- debian/changelog | 8 +++- 3 files changed, 13 insertions(+), 3 deletions(-) -- 2.31.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-22.06 1/2] Set release date for 22.06.0.
Signed-off-by: Mark Michelson --- NEWS | 2 +- debian/changelog | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index e27ae6c39..e335f64c2 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -OVN v22.06.0 - XX XXX +OVN v22.06.0 - 03 Jun 2022 -- - Support IGMP and MLD snooping on transit logical switches that connect different OVN Interconnection availability zones. diff --git a/debian/changelog b/debian/changelog index 27bb40232..caef68452 100644 --- a/debian/changelog +++ b/debian/changelog @@ -2,7 +2,7 @@ ovn (22.06.0-1) unstable; urgency=low * New upstream version - -- OVN team Thu, 19 May 2022 15:11:38 -0400 + -- OVN team Fri, 03 Jun 2022 11:54:08 -0400 ovn (22.03.0-1) unstable; urgency=low -- 2.31.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-22.06 2/2] Prepare for 22.06.1.
Signed-off-by: Mark Michelson --- NEWS | 3 +++ configure.ac | 2 +- debian/changelog | 6 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index e335f64c2..cc58306d7 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +OVN v22.06.1 - xx xxx +-- + OVN v22.06.0 - 03 Jun 2022 -- - Support IGMP and MLD snooping on transit logical switches that connect diff --git a/configure.ac b/configure.ac index b649441bc..739e0295e 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.63) -AC_INIT(ovn, 22.06.0, b...@openvswitch.org) +AC_INIT(ovn, 22.06.1, b...@openvswitch.org) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) AC_CONFIG_HEADERS([config.h]) diff --git a/debian/changelog b/debian/changelog index caef68452..1c2de53bd 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +OVN (22.06.1-1) unstable; urgency=low + [ OVN team ] + * New upstream version + + -- OVN team Fri, 03 Jun 2022 11:54:08 -0400 + ovn (22.06.0-1) unstable; urgency=low * New upstream version -- 2.31.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-21.12 0/2] Release patches for v21.12.2.
Mark Michelson (2): Set release date for 21.12.2. Prepare for 21.12.3. NEWS | 6 +- configure.ac | 2 +- debian/changelog | 8 +++- 3 files changed, 13 insertions(+), 3 deletions(-) -- 2.31.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-21.12 2/2] Prepare for 21.12.3.
Signed-off-by: Mark Michelson --- NEWS | 3 +++ configure.ac | 2 +- debian/changelog | 6 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 6ff9b8fdf..31e08b015 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +OVN v21.12.3 - xx xxx +-- + OVN v21.12.2 - 03 Jun 2022 -- - Bug fixes diff --git a/configure.ac b/configure.ac index c4bf08db7..f37afb7bb 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.63) -AC_INIT(ovn, 21.12.2, b...@openvswitch.org) +AC_INIT(ovn, 21.12.3, b...@openvswitch.org) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) AC_CONFIG_HEADERS([config.h]) diff --git a/debian/changelog b/debian/changelog index 5bf39c320..abf5ab264 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +OVN (21.12.3-1) unstable; urgency=low + [ OVN team ] + * New upstream version + + -- OVN team Fri, 03 Jun 2022 11:53:58 -0400 + OVN (21.12.2-1) unstable; urgency=low [ OVN team ] * New upstream version -- 2.31.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn branch-22.03 2/2] Prepare for 22.03.2.
Signed-off-by: Mark Michelson --- NEWS | 3 +++ configure.ac | 2 +- debian/changelog | 6 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 166470747..3e1c6fb11 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +OVN v22.03.2 - xx xxx +-- + OVN v22.03.1 - 03 Jun 2022 -- - Bug fixes diff --git a/configure.ac b/configure.ac index 70f86e1f5..f1d03528f 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.63) -AC_INIT(ovn, 22.03.1, b...@openvswitch.org) +AC_INIT(ovn, 22.03.2, b...@openvswitch.org) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) AC_CONFIG_HEADERS([config.h]) diff --git a/debian/changelog b/debian/changelog index 1b9ebca1b..7abf3c332 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +OVN (22.03.2-1) unstable; urgency=low + [ OVN team ] + * New upstream version + + -- OVN team Fri, 03 Jun 2022 11:54:04 -0400 + OVN (22.03.1-1) unstable; urgency=low [ OVN team ] * New upstream version -- 2.31.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn branch-21.12 1/2] Set release date for 21.12.2.
Bleep bloop. Greetings Mark Michelson, 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. build: PYTHONPATH=/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/python":"$PYTHONPATH PYTHONDONTWRITEBYTECODE=yes /usr/bin/python3 /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/ovsdb/ovsdb-idlc.in annotate ./ovn-ic-sb.ovsschema ./lib/ovn-ic-sb-idl.ann > lib/ovn-ic-sb-idl.ovsidl.tmp && \ mv lib/ovn-ic-sb-idl.ovsidl.tmp lib/ovn-ic-sb-idl.ovsidl PYTHONPATH=/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/python":"$PYTHONPATH PYTHONDONTWRITEBYTECODE=yes /usr/bin/python3 /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/ovsdb/ovsdb-idlc.in c-idl-source lib/ovn-ic-sb-idl.ovsidl > lib/ovn-ic-sb-idl.c.tmp && mv lib/ovn-ic-sb-idl.c.tmp lib/ovn-ic-sb-idl.c PYTHONPATH=/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/python":"$PYTHONPATH PYTHONDONTWRITEBYTECODE=yes /usr/bin/python3 /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/ovsdb/ovsdb-idlc.in c-idl-header lib/ovn-ic-sb-idl.ovsidl > lib/ovn-ic-sb-idl.h.tmp && mv lib/ovn-ic-sb-idl.h.tmp lib/ovn-ic-sb-idl.h make all-am make[1]: Entering directory `/var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace' depbase=`echo lib/acl-log.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\ /bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./ovn -I ./include -I ./lib -I ./lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/acl-log.lo -MD -MP -MF $depbase.Tpo -c -o li b/acl-log.lo lib/acl-log.c &&\ mv -f $depbase.Tpo $depbase.Plo libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./ovn -I ./include -I ./lib -I ./lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/acl-log.lo -MD -MP -MF lib/.deps/acl-log.Tpo -c lib/acl-log.c -o lib/acl-log. o depbase=`echo lib/actions.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\ /bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./ovn -I ./include -I ./lib -I ./lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/include -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/actions.lo -MD -MP -MF $depbase.Tpo -c -o li b/actions.lo lib/actions.c &&\ mv -f $depbase.Tpo $depbase.Plo libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./ovn -I ./include -I ./lib -I ./lib -I /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/ovs/include -I /var/lib/jenkins/jobs/0day_robot_upstre
Re: [ovs-dev] [PATCH v3] dpif-netdev: Allow cross-NUMA polling on selected ports
Hi Anurag,
Thanks for submitting this. Some initial comments on the code below.
On 03/06/2022 05:25, Anurag Agarwal wrote:
From: Jan Scheurich
Today dpif-netdev considers PMD threads on a non-local NUMA node for automatic
assignment of the rxqs of a port only if there are no local,non-isolated PMDs.
On typical servers with both physical ports on one NUMA node, this often
leaves the PMDs on the other NUMA node under-utilized, wasting CPU resources.
The alternative, to manually pin the rxqs to PMDs on remote NUMA nodes, also
has drawbacks as it limits OVS' ability to auto load-balance the rxqs.
This patch introduces a new interface configuration option to allow ports to
be automatically polled by PMDs on any NUMA node:
ovs-vsctl set interface other_config:cross-numa-polling=true
The group assignment algorithm now has the ability to select lowest loaded PMD
on any NUMA, and not just the local NUMA on which the rxq of the port resides
If this option is not present or set to false, legacy behaviour applies.
Co-authored-by: Anurag Agarwal
Signed-off-by: Jan Scheurich
Signed-off-by: Anurag Agarwal
---
It would be good if you could include some links below the cut line here
or in a cover-letter to the previous discussion on this feature and the
possible side-effect, so reviewers can be aware of that.
Also, fine so far as it was minor revisions sent close together, but for
future revisions, please add a note about what has changed. That will
help reviewers to know what they need to focus on in the new revision.
Documentation/topics/dpdk/pmd.rst | 23 ++
lib/dpif-netdev.c | 123 ++
tests/pmd.at | 33
vswitchd/vswitch.xml | 20 +
4 files changed, 166 insertions(+), 33 deletions(-)
diff --git a/Documentation/topics/dpdk/pmd.rst
b/Documentation/topics/dpdk/pmd.rst
index b259cc8b3..387f962d1 100644
--- a/Documentation/topics/dpdk/pmd.rst
+++ b/Documentation/topics/dpdk/pmd.rst
@@ -99,6 +99,25 @@ core cycles for each Rx queue::
$ ovs-appctl dpif-netdev/pmd-rxq-show
+Normally, Rx queues are assigned to PMD threads automatically. By default
+OVS only assigns Rx queues to PMD threads executing on the same NUMA
+node in order to avoid unnecessary latency for accessing packet buffers
+across the NUMA boundary. Typically this overhead is higher for vhostuser
+ports than for physical ports due to the packet copy that is done for all
+rx packets.
+
I don't think it needs double space for the start of each sentence, but
that could be because I'm comparing with other parts of the
documentation that I wrote incorrectly without that o_O
+On NUMA servers with physical ports only on one NUMA node, the NUMA-local
+polling policy can lead to an under-utilization of the PMD threads on the
+remote NUMA node. For the overall OVS performance it may in such cases be
+beneficial to utilize the spare capacity and allow polling of a physical
+port's rxqs across NUMA nodes despite the overhead involved.
+The policy can be set per port with the following configuration option::
+
+$ ovs-vsctl set Interface \
+other_config:cross-numa-polling=true|false
+
+The default value is false.
+
.. note::
A history of one minute is recorded and shown for each Rx queue to allow for
@@ -115,6 +134,10 @@ core cycles for each Rx queue::
A ``overhead`` statistics is shown per PMD: it represents the number of
cycles inherently consumed by the OVS PMD processing loop.
+.. versionchanged:: 2.18.0
+
+ Added the interface parameter ``other_config:cross-numa-polling``
+
Rx queue to PMD assignment takes place whenever there are configuration
changes
or can be triggered by using::
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index ff57b3961..ace5c1920 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -467,6 +467,7 @@ struct dp_netdev_port {
char *type; /* Port type as requested by user. */
char *rxq_affinity_list;/* Requested affinity of rx queues. */
enum txq_req_mode txq_requested_mode;
+bool cross_numa_polling;/* If true cross polling will be enabled */
Full stop to be consistent with other comments e.g.'...enabled.'
};
static bool dp_netdev_flow_ref(struct dp_netdev_flow *);
@@ -2101,6 +2102,7 @@ port_create(const char *devname, const char *type,
port->sf = NULL;
port->emc_enabled = true;
port->need_reconfigure = true;
+port->cross_numa_polling = false;
ovs_mutex_init(&port->txq_used_mutex);
*portp = port;
@@ -5013,6 +5015,7 @@ dpif_netdev_port_set_config(struct dpif *dpif, odp_port_t
port_no,
bool emc_enabled = smap_get_bool(cfg, "emc-enable", true);
const char *tx_steering_mode = smap_get(cfg, "tx-steering");
enum txq_req_mode txq_mode;
+bool cross_numa_polling = smap_get_bool(cfg, "cross-numa-polling", false);
ovs_rwlock_wrl
Re: [ovs-dev] [PATCH] python: use setuptools instead of distutils
On Wed, May 11, 2022 at 2:14 PM Timothy Redaelli wrote: > > On Python 3.12, distutils will be removed and it's currently (3.10+) > deprecated (see PEP 632). > > Since the suggested and simplest replacement is setuptools, this commit > replaces distutils to use setuptools instead. > > Signed-off-by: Timothy Redaelli > --- > python/setup.py | 9 - > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/python/setup.py b/python/setup.py > index cfe01763f..7c7418bd9 100644 > --- a/python/setup.py > +++ b/python/setup.py > @@ -12,9 +12,8 @@ > > import sys > > -from distutils.command.build_ext import build_ext > -from distutils.errors import CCompilerError, DistutilsExecError, \ > -DistutilsPlatformError > +from setuptools.command.build_ext import build_ext > +from setuptools.errors import CCompilerError, ExecError, PlatformError I tried applying this patch on RHEL 8.5 with Python 3.6.8 and setuptools 39.2.0, and got the following error: Processing /home/test/ovs_mod/python Complete output from command python setup.py egg_info: Traceback (most recent call last): File "", line 1, in File "/tmp/pip-v08_3iao-build/setup.py", line 16, in from setuptools.errors import CCompilerError, ExecError, PlatformError ModuleNotFoundError: No module named 'setuptools.errors' You could use this sort of motif to improve backwards compatibility: try: from A import B except ImportError: from C import D as B Cheers, M > > import setuptools > > @@ -37,7 +36,7 @@ except IOError: >file=sys.stderr) > sys.exit(-1) > > -ext_errors = (CCompilerError, DistutilsExecError, DistutilsPlatformError) > +ext_errors = (CCompilerError, ExecError, PlatformError) > if sys.platform == 'win32': > ext_errors += (IOError, ValueError) > > @@ -53,7 +52,7 @@ class try_build_ext(build_ext): > def run(self): > try: > build_ext.run(self) > -except DistutilsPlatformError: > +except PlatformError: > raise BuildFailed() > > def build_extension(self, ext): > -- > 2.36.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
Re: [ovs-dev] [PATCH] dpcls: revert subtable-lookup-prio-get name change
On 5/27/22 14:45, Van Haaren, Harry wrote:
>> -Original Message-
>> From: Ilya Maximets
>> Sent: Friday, May 27, 2022 1:10 PM
>> To: Van Haaren, Harry ; Eelco Chaudron
>>
>> Cc: i.maxim...@ovn.org; ovs-dev@openvswitch.org; Stokes, Ian
>> ; Amber, Kumar
>> Subject: Re: [PATCH] dpcls: revert subtable-lookup-prio-get name change
>>
>> On 5/25/22 18:15, Van Haaren, Harry wrote:
-Original Message-
From: Ilya Maximets
Sent: Wednesday, May 25, 2022 4:33 PM
To: Eelco Chaudron ; Van Haaren, Harry
Cc: i.maxim...@ovn.org; ovs-dev@openvswitch.org; Stokes, Ian
; Amber, Kumar
Subject: Re: [PATCH] dpcls: revert subtable-lookup-prio-get name change
On 5/25/22 16:32, Eelco Chaudron wrote:
>
>
> On 25 May 2022, at 16:10, Harry van Haaren wrote:
>
>> This commit reverts the name-change that was done (prio->info).
>> The change breaks a user visible ovs-appctl command, resulting in
>> breakage of tools/scripts/user-expectation outside of the OVS repo.
>>
>> This commit changes the documentation, command string, and unit tests
>> back to the expected "prio" string, as expected in OVS 2.17 and earlier.
>
> Can’t remember all the details, but I guess Ilya’s concern was that the
> command
name does not reflect the actual output.
>
> So what about hiding the “subtable-lookup-prio-get” command, and making it
>> such
that it still works for backward compatibility?
> I think this can easily be done by adding the command with a NULL usage
> string.
>
> I guess all that needs to be added is another command, something like, but
>> please
test it:
>
> +unixctl_command_register("dpif-netdev/subtable-lookup-prio-get",
> NULL,
> + 0, 0, dpif_netdev_subtable_lookup_get,
> + NULL);
>
> Harry/Ilya what do you think?
>
>
>
>>> There is value in maintaining command-name stability -> see above example.
>>> It shouldn't matter if things are supposed to be used specific ways in
>>> general,
>>> reality is that these commands are put into scripts, and renaming it will
>>> break.
>>>
>>> I don't know what else to say, renaming/breaking commands has no value in my
>> eyes, and only
>>> makes using the command difficult.
>>>
>>> This patch should be merged ASAP, to return the command to the known command
>> string,
>>> and fix the current breakage of the user's scripts/command-history.
>>
>> Again, what's wrong with the backward compatible alias?
>
> For bug fix patches, or resolving issues I tend to prefer solution that
> offers the lowest-risk,
> and least amount of unknown/new changes. I think this patch achieves that.
> The backwards
> compatible alias is a "new/different" solution, and hence not my preferred
> option.
The lowest risk is to just revert the 738c76a503f4.
I don't see any serious risk in having an alias, it's not a rocket
science. And it's not like we have to fix this ASAP, we have time
to develop a good solution. Eelco is basically already provided
all the code needed, the code just needs a round of simple testing.
> I'll leave it to you as OVS maintainer; there's a fix patch here I'm
> confident in, and can be merged.
> If there is a preference to develop and merge an alternative solution, that's
> Ok with me too,
If someone else wants to pick the change up and submit a new version,
that's fine. We may revert the whole 738c76a503f4 as an alternative
and go with the dp-extra-info approach instead, that is also fine,
even though the format in the 738c76a503f4 suits the purpose a bit
better.
> but I won't commit to spending time on it as there is a fix here already.
The current version of the patch has been reviewed and changes have
been requested. This is the standard process.
The current version of the change doesn't make sense to me:
NACKed-by: Ilya Maximets
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-linux: skip some internal kernel stats gathering
On 6/3/22 16:47, Jon Kohler wrote:
>
>
>> On Jun 2, 2022, at 5:50 PM, Ilya Maximets wrote:
>>
>> On 6/2/22 23:13, Jon Kohler wrote:
>>> Any takers? I’m hoping I’ve got the right mailing list, as I did see
>>> the thread get generated on the mailing list website?
>>
>> Hey, Jon.
>>
>> Yes, it's the right mailing list, though ovs-dev and dev are
>> the same thing, so you don't need to send to both.
>>
>> For the subject, you sent a patch just about a week ago.
>> Unfortunately, patches are not moving that fast. People
>> are busy with working on issues and doing reviews of other
>> patches, so please, be patient.
>>
>> Just from the patches received last month we still have
>> about 60 under review right now. You can see them in the
>> patchwork here:
>>
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_project_openvswitch_list_&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=NGPRGGo37mQiSXgHKm5rCQ&m=8oqFH3XK_nAzOxdHmUuHbWgSEPemwCFQKUdGExfEPEx3NfR3ioDpK5CZHwMdEyf_&s=Osr7hGzYKATNpHa9IvtWygokIFsGsmT4GrnDuUQ3FzY&e=
>>
>>
>> And as you can see we have much older patches too. So, it
>> will take some time until someone will get to yours.
>>
>> Best regards, Ilya Maximets.
>
> Thanks for the feedback and pointers. My apologies, I wasn’t trying to be
> ansty :) this is my first patch to this mailing list and wanted to make sure
> it
> was going to the right place.
No problem. :)
>
> Ok no problems, I can fully empathize with patch review backlogs :) No
> specific rush on this one. Nice CPU win whenever someone can get
> to it, but would definitely rather have quality over speed to make sure
> we haven’t missed anything! :)
Thanks for working on improvements! I hope, someone will get
to this patch in the near future as it looks interesting indeed.
>
> Cheers,
> Jon
>
>>
>>>
On May 31, 2022, at 10:03 AM, Jon Kohler wrote:
> On May 26, 2022, at 9:11 PM, Jon Kohler wrote:
>
> For netdev_linux_update_via_netlink(), hint to the kernel that
> we do not need it to gather netlink internal stats when we want
> to update the netlink flags, as those stats are not rendered
> within OVS.
>
> Background:
> ovs-vswitchd can spend quite a bit of time blocked by the kernel
> during netlink calls, especially systems with many cores. This
> time is dominated by the kernel-side internal stats gathering
> mechanism in netlink, specifically:
> inet6_fill_link_af
> inet6_fill_ifla6_attrs
>__snmp6_fill_stats64
>
> In Linux 4.4+, there exists a hint for netlink requests to not
> trigger the ipv6 stats gathering mechanism, which greatly reduces
> the amount of time that ovs-vswitchd is on CPU.
>
> Testing and Results:
> Tested booting 320 VM's and measuring OVS utilization with perf
> record, then visualized into a flamegraph using a patched version
> of ovs 2.14.2. Calls under bridge_run() seem to get hit the worst
> by this issue.
>
> Before bridge_run() == 11.3% of samples
> After bridge_run() == 3.4% of samples
>
> Note that there are at least two observed netlink calls under
> bridge_run that are still kernel stats heavy after this patch:
>
> Call 1:
> bridge_run -> netdev_run -> route_table_run -> route_table_reset ->
> ovs_router_insert -> ovs_router_insert__ -> get_src_addr ->
>netdev_ger_addr_list -> netdev_linux_get_addr_list -> getifaddrs
>
> Since the actual netlink call is coming from getifaddrs() in glibc,
> fixing would likely involve either duplicating glibc code in ovs
> source or patch glibc.
>
> Call 2:
> bridge_run -> iface_refresh_stats -> netdev_get_stats ->
> netdev_linux_get_stats -> get_stats_via_netlink
>
> This does use netlink based stats; however, it isn't immediately
> clear if just dropping the stats from inet6_fill_link_af would
> impact anything or not. Given this call is more intermittent, its
> of lesser concern.
>
> Signed-off-by: Jon Kohler
> Acked-by: Greg Smith
Gentle bump
>>>
>>>
>>>
> ---
> lib/netdev-linux.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 2766b3f2bf..f0246d3b2b 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -247,6 +247,12 @@ enum {
> VALID_NUMA_ID = 1 << 8,
> };
>
> +/* Linux 4.4 introduced the ability to skip the internal stats gathering
> + * that netlink does via an external filter mask that can be passed into
> + * a netlink request.
> + */
> +#define RTEXT_FILTER_SKIP_STATS (1 << 3)
> +
> /* Use one for the packet buffer and another for the aux buffer to receive
> * TSO packets. */
> #define IOV_STD_SIZE 1
> @@ -6418,6 +6424,9 @@ netdev_linux_update_via_netlink(struct netdev_linux
> *netdev)
> if (
Re: [ovs-dev] [PATCH v4 4/5] system-offloads-traffic: Properly initialize offload before testing.
On Fri, Jun 3, 2022 at 4:58 AM Eelco Chaudron wrote: > > This patch will properly initialize offload, as it requires the > setting to be enabled before starting ovs-vswitchd (or do a > restart once configured). > > Signed-off-by: Eelco Chaudron > --- > v4: > - Use the existing dbinit-aux-args argument, rather than > creating a new pre-vswitchd command option. > v3: > - No changes. > v2: > - Unified all the OVS_TRAFFIC_VSWITCHD_START macro's > > tests/ofproto-macros.at |3 ++- > tests/system-kmod-macros.at |4 ++-- > tests/system-offloads-traffic.at |9 +++-- > tests/system-tso-macros.at |4 ++-- > tests/system-userspace-macros.at |4 ++-- > 5 files changed, 11 insertions(+), 13 deletions(-) Looks good! Acked-by: Mike Pattrick Cheers, M > > diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at > index b18f0fbc1..af956bdcb 100644 > --- a/tests/ofproto-macros.at > +++ b/tests/ofproto-macros.at > @@ -153,7 +153,7 @@ m4_divert_pop([PREPARE_TESTS]) > > m4_define([TESTABLE_LOG], [-vPATTERN:ANY:'%c|%p|%m']) > > -# _OVS_VSWITCHD_START([vswitchd-aux-args]) > +# _OVS_VSWITCHD_START([vswitchd-aux-args] [dbinit-aux-args]) > # > # Creates an empty database and starts ovsdb-server. > # Starts ovs-vswitchd, with additional arguments 'vswitchd-aux-args'. > @@ -189,6 +189,7 @@ m4_define([_OVS_VSWITCHD_START], > /ofproto|INFO|datapath ID changed to fedcba9876543210/d > /dpdk|INFO|DPDK Disabled - Use other_config:dpdk-init to enable/d > /netlink_socket|INFO|netlink: could not enable listening to all nsid/d > +/netdev_offload|INFO|netdev: Flow API Enabled/d > /probe tc:/d > /setting extended ack support failed/d > /tc: Using policy/d']]) > diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at > index 86d633ac4..b8a6b67c8 100644 > --- a/tests/system-kmod-macros.at > +++ b/tests/system-kmod-macros.at > @@ -4,7 +4,7 @@ > # appropriate type and properties > m4_define([_ADD_BR], [[add-br $1 -- set Bridge $1 > protocols=OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15 > fail-mode=secure ]]) > > -# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [=override]) > +# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], > [dbinit-aux-args]]) > # > # Creates a database and starts ovsdb-server, starts ovs-vswitchd > # connected to that database, calls ovs-vsctl to create a bridge named > @@ -24,7 +24,7 @@ m4_define([OVS_TRAFFIC_VSWITCHD_START], >]) > on_exit 'ovs-dpctl del-dp ovs-system' > on_exit 'ovs-appctl dpctl/flush-conntrack' > - _OVS_VSWITCHD_START([]) > + _OVS_VSWITCHD_START([], [$3]) > dnl Add bridges, ports, etc. > AT_CHECK([ovs-vsctl -- _ADD_BR([br0]) -- $1 m4_if([$2], [], [], [| > uuidfilt])], [0], [$2]) > ]) > diff --git a/tests/system-offloads-traffic.at > b/tests/system-offloads-traffic.at > index 80bc1dd5c..f7373664c 100644 > --- a/tests/system-offloads-traffic.at > +++ b/tests/system-offloads-traffic.at > @@ -39,9 +39,8 @@ AT_CLEANUP > > > AT_SETUP([offloads - ping between two ports - offloads enabled]) > -OVS_TRAFFIC_VSWITCHD_START() > +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . > other_config:hw-offload=true]) > > -AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true]) > AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) > > ADD_NAMESPACES(at_ns0, at_ns1) > @@ -97,8 +96,7 @@ AT_CLEANUP > AT_SETUP([offloads - set ingress_policing_rate and ingress_policing_burst - > offloads enabled]) > AT_KEYWORDS([ingress_policing]) > AT_SKIP_IF([test $HAVE_TC = "no"]) > -OVS_TRAFFIC_VSWITCHD_START() > -AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true]) > +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . > other_config:hw-offload=true]) > AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) > ADD_NAMESPACES(at_ns0) > ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") > @@ -146,8 +144,7 @@ AT_CLEANUP > AT_SETUP([offloads - set ingress_policing_kpkts_rate and > ingress_policing_kpkts_burst - offloads enabled]) > AT_KEYWORDS([ingress_policing_kpkts]) > AT_SKIP_IF([test $SUPPORT_TC_INGRESS_PPS = "no"]) > -OVS_TRAFFIC_VSWITCHD_START() > -AT_CHECK([ovs-vsctl set Open_vSwitch . other_config:hw-offload=true]) > +OVS_TRAFFIC_VSWITCHD_START([], [], [-- set Open_vSwitch . > other_config:hw-offload=true]) > AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) > ADD_NAMESPACES(at_ns0) > ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") > diff --git a/tests/system-tso-macros.at b/tests/system-tso-macros.at > index 1a8004761..1881c28fb 100644 > --- a/tests/system-tso-macros.at > +++ b/tests/system-tso-macros.at > @@ -4,7 +4,7 @@ > # appropriate type and properties > m4_define([_ADD_BR], [[add-br $1 -- set Bridge $1 datapath_type="netdev" > protocols=OpenFlow10,OpenFlow11,OpenFlow12,OpenFlow13,OpenFlow14,OpenFlow15 > fail-mode=secure ]]) > > -# OVS_TRAFFIC_VSWITCHD_START([vsctl-args], [vsctl-output], [=override]) > +# OV
Re: [ovs-dev] [PATCH 1/1] packets: Re-calculate IPv6 checksum only for last frags upon modify.
On Sun, May 29, 2022 at 10:21 AM Salem Sol via dev
wrote:
>
> In case of modifying an IPv6 packet src/dst address the checksum should be
> recalculated only for the last frag. Currently it's done for all frags,
> leading to incorrect reassembled packet checksum.
> Fix it by adding a new flag to recalculate the checksum only for last frag.
>
> Fixes: bc7a5acdff08 ("datapath: add ipv6 'set' action")
> Signed-off-by: Salem Sol
> ---
> lib/packets.c | 22 --
> tests/system-traffic.at | 35 +++
> 2 files changed, 55 insertions(+), 2 deletions(-)
> https://github.com/salemsol/ovs/actions/runs/2404335827
>
> diff --git a/lib/packets.c b/lib/packets.c
> index d0fba8176..70b0e6ad2 100644
> --- a/lib/packets.c
> +++ b/lib/packets.c
> @@ -1148,6 +1148,20 @@ packet_set_ipv4_addr(struct dp_packet *packet,
> put_16aligned_be32(addr, new_addr);
> }
>
> +static bool
> +packet_is_last_ipv6_frag(struct dp_packet *packet)
> +{
> +const struct ovs_16aligned_ip6_frag *frag_hdr;
> +const struct ovs_16aligned_ip6_hdr *nh;
> +uint8_t *data = dp_packet_l3(packet);
> +
> +nh = ALIGNED_CAST(struct ovs_16aligned_ip6_hdr *, data);
Nit: is the above line needed?
Otherwise, looks good to me.
Acked-by: Mike Pattrick
Cheers,
M
> +data += sizeof *nh;
> +frag_hdr = ALIGNED_CAST(struct ovs_16aligned_ip6_frag *, data);
> +return (frag_hdr->ip6f_offlg & IP6F_OFF_MASK) &&
> + !(frag_hdr->ip6f_offlg & IP6F_MORE_FRAG);
> +}
> +
> /* Returns true, if packet contains at least one routing header where
> * segements_left > 0.
> *
> @@ -1334,17 +1348,21 @@ packet_set_ipv6(struct dp_packet *packet, const
> struct in6_addr *src,
> {
> struct ovs_16aligned_ip6_hdr *nh = dp_packet_l3(packet);
> uint8_t proto = 0;
> +bool recalc_csum;
> bool rh_present;
>
> rh_present = packet_rh_present(packet, &proto);
> +recalc_csum = nh->ip6_nxt == IPPROTO_FRAGMENT ?
> +packet_is_last_ipv6_frag(packet) : true;
>
> if (memcmp(&nh->ip6_src, src, sizeof(ovs_be32[4]))) {
> -packet_set_ipv6_addr(packet, proto, nh->ip6_src.be32, src, true);
> +packet_set_ipv6_addr(packet, proto, nh->ip6_src.be32,
> + src, recalc_csum);
> }
>
> if (memcmp(&nh->ip6_dst, dst, sizeof(ovs_be32[4]))) {
> packet_set_ipv6_addr(packet, proto, nh->ip6_dst.be32, dst,
> - !rh_present);
> + !rh_present && recalc_csum);
> }
>
> packet_set_ipv6_tc(&nh->ip6_flow, key_tc);
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 239105e89..16ba42d84 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -192,6 +192,41 @@ NS_CHECK_EXEC([at_ns0], [ping6 -s 3200 -q -c 3 -i 0.3 -w
> 2 fc00:1::2 | FORMAT_PI
> OVS_TRAFFIC_VSWITCHD_STOP
> AT_CLEANUP
>
> +AT_SETUP([datapath - ping6 between two ports with header modify])
> +OVS_TRAFFIC_VSWITCHD_START()
> +
> +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
> +
> +ADD_NAMESPACES(at_ns0, at_ns1)
> +
> +ADD_VETH(p0, at_ns0, br0, "fc00::1/96", e4:11:22:33:44:55)
> +ADD_VETH(p1, at_ns1, br0, "fc00::2/96", e4:11:22:33:44:54)
> +NS_CHECK_EXEC([at_ns0], [ip -6 neigh add fc00::3 lladdr e4:11:22:33:44:54
> dev p0])
> +NS_CHECK_EXEC([at_ns0], [ip -6 neigh add fc00::2 lladdr e4:11:22:33:44:54
> dev p0])
> +NS_CHECK_EXEC([at_ns0], [ip -6 neigh add fc00::1 lladdr e4:11:22:33:44:55
> dev p0])
> +
> +dnl Linux seems to take a little time to get its IPv6 stack in order. Without
> +dnl waiting, we get occasional failures due to the following error:
> +dnl "connect: Cannot assign requested address"
> +OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2])
> +
> +AT_CHECK([ovs-ofctl del-flows -OOpenFlow15 br0], [], [stdout], [stderr])
> +AT_CHECK([ovs-ofctl add-flow -OOpenFlow15 br0
> in_port=ovs-p0,ipv6,ipv6_dst=fc00::3,ipv6_src=fc00::1,actions=set_field:fc00::2-\>ipv6_dst,ovs-p1],
> [], [stdout], [stderr])
> +AT_CHECK([ovs-ofctl add-flow -OOpenFlow15 br0
> in_port=ovs-p1,ipv6,ipv6_dst=fc00::1,ipv6_src=fc00::2,actions=set_field:fc00::3-\>ipv6_src,ovs-p0],
> [], [stdout], [stderr])
> +
> +NS_CHECK_EXEC([at_ns0], [ping6 -q -c 3 -i 0.3 -w 2 fc00::3 | FORMAT_PING],
> [0], [dnl
> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> +])
> +NS_CHECK_EXEC([at_ns0], [ping6 -s 1600 -q -c 3 -i 0.3 -w 2 fc00::3 |
> FORMAT_PING], [0], [dnl
> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> +])
> +NS_CHECK_EXEC([at_ns0], [ping6 -s 3200 -q -c 3 -i 0.3 -w 2 fc00::3 |
> FORMAT_PING], [0], [dnl
> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> +])
> +
> +OVS_TRAFFIC_VSWITCHD_STOP
> +AT_CLEANUP
> +
> AT_SETUP([datapath - ping over bond])
> OVS_TRAFFIC_VSWITCHD_START()
>
> --
> 2.21.0
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listin
Re: [ovs-dev] [PATCH v2] ovs-ofctl: '--bundle' option can be used with OpenFlow 1.3
On Mon, May 9, 2022 at 5:21 AM yangchang wrote: > > From the commit 25070e045e, bundle option has be used with OpenFlow 1.3 Nit, s/has/can/ But the content of the patch looks correct. Acked-by: Mike Pattrick Cheers, M > > Signed-off-by: yangchang > --- > utilities/ovs-ofctl.8.in | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in > index e903ca1..35f4dea 100644 > --- a/utilities/ovs-ofctl.8.in > +++ b/utilities/ovs-ofctl.8.in > @@ -1312,7 +1312,7 @@ well as cookie values and table IDs if they are zero. > Do not execute read/write commands. > . > .IP "\fB\-\-bundle\fR" > -Execute flow mods as an OpenFlow 1.4 atomic bundle transaction. > +Execute flow mods as an OpenFlow 1.3 atomic bundle transaction. > .RS > .IP \(bu > Within a bundle, all flow mods are processed in the order they appear > @@ -1324,15 +1324,15 @@ the transaction, or after all the flow mods in the > bundle have been > successfully applied. > .IP \(bu > The beginning and the end of the flow table modification commands in a > -bundle are delimited with OpenFlow 1.4 bundle control messages, which > +bundle are delimited with OpenFlow 1.3 bundle control messages, which > makes it possible to stream the included commands without explicit > OpenFlow barriers, which are otherwise used after each flow table > modification command. This may make large modifications execute > faster as a bundle. > .IP \(bu > -Bundles require OpenFlow 1.4 or higher. An explicit \fB-O > -OpenFlow14\fR option is not needed, but you may need to enable > -OpenFlow 1.4 support for OVS by setting the OVSDB \fIprotocols\fR > +Bundles require OpenFlow 1.3 or higher. An explicit \fB-O > +OpenFlow13\fR option is not needed, but you may need to enable > +OpenFlow 1.3 support for OVS by setting the OVSDB \fIprotocols\fR > column in the \fIbridge\fR table. > .RE > . > -- > 1.8.3.1 > > > > yangch...@chinatelecom.cn > > ___ > 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 branch-22.03 0/2] Release patches for v22.03.1.
On Fri, Jun 3, 2022 at 11:55 AM Mark Michelson wrote: > > > Mark Michelson (2): > Set release date for 22.03.1. > Prepare for 22.03.2. > > NEWS | 6 +- > configure.ac | 2 +- > debian/changelog | 8 +++- > 3 files changed, 13 insertions(+), 3 deletions(-) For both the patches: Acked-by: Numan Siddique Numan > > -- > 2.31.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
Re: [ovs-dev] [PATCH ovn branch-22.03 1/2] Set release date for 22.03.1.
On Fri, Jun 3, 2022 at 11:55 AM Mark Michelson wrote: > > Signed-off-by: Mark Michelson Acked-by: Numan Siddique Numan > --- > NEWS | 3 ++- > debian/changelog | 2 +- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/NEWS b/NEWS > index 700a6830a..166470747 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,5 +1,6 @@ > -OVN v22.03.1 - xx xxx > +OVN v22.03.1 - 03 Jun 2022 > -- > + - Bug fixes >- Replaced the usage of masked ct_label by ct_mark in most cases to work > better with hardware-offloading. > > diff --git a/debian/changelog b/debian/changelog > index 18a1a042e..1b9ebca1b 100644 > --- a/debian/changelog > +++ b/debian/changelog > @@ -2,7 +2,7 @@ OVN (22.03.1-1) unstable; urgency=low > [ OVN team ] > * New upstream version > > - -- OVN team Fri, 11 Mar 2022 13:22:32 -0500 > + -- OVN team Fri, 03 Jun 2022 11:54:04 -0400 > > ovn (22.03.0-1) unstable; urgency=low > > -- > 2.31.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
Re: [ovs-dev] [PATCH ovn branch-22.03 2/2] Prepare for 22.03.2.
On Fri, Jun 3, 2022 at 11:55 AM Mark Michelson wrote: > > Signed-off-by: Mark Michelson Acked-by: Numan Siddique Numan > --- > NEWS | 3 +++ > configure.ac | 2 +- > debian/changelog | 6 ++ > 3 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/NEWS b/NEWS > index 166470747..3e1c6fb11 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,3 +1,6 @@ > +OVN v22.03.2 - xx xxx > +-- > + > OVN v22.03.1 - 03 Jun 2022 > -- > - Bug fixes > diff --git a/configure.ac b/configure.ac > index 70f86e1f5..f1d03528f 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -13,7 +13,7 @@ > # limitations under the License. > > AC_PREREQ(2.63) > -AC_INIT(ovn, 22.03.1, b...@openvswitch.org) > +AC_INIT(ovn, 22.03.2, b...@openvswitch.org) > AC_CONFIG_MACRO_DIR([m4]) > AC_CONFIG_AUX_DIR([build-aux]) > AC_CONFIG_HEADERS([config.h]) > diff --git a/debian/changelog b/debian/changelog > index 1b9ebca1b..7abf3c332 100644 > --- a/debian/changelog > +++ b/debian/changelog > @@ -1,3 +1,9 @@ > +OVN (22.03.2-1) unstable; urgency=low > + [ OVN team ] > + * New upstream version > + > + -- OVN team Fri, 03 Jun 2022 11:54:04 -0400 > + > OVN (22.03.1-1) unstable; urgency=low > [ OVN team ] > * New upstream version > -- > 2.31.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
Re: [ovs-dev] [PATCH ovn branch-22.06 1/2] Set release date for 22.06.0.
On Fri, Jun 3, 2022 at 11:55 AM Mark Michelson wrote: > > Signed-off-by: Mark Michelson Acked-by: Numan Siddique Numan > --- > NEWS | 2 +- > debian/changelog | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/NEWS b/NEWS > index e27ae6c39..e335f64c2 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,4 +1,4 @@ > -OVN v22.06.0 - XX XXX > +OVN v22.06.0 - 03 Jun 2022 > -- >- Support IGMP and MLD snooping on transit logical switches that connect > different OVN Interconnection availability zones. > diff --git a/debian/changelog b/debian/changelog > index 27bb40232..caef68452 100644 > --- a/debian/changelog > +++ b/debian/changelog > @@ -2,7 +2,7 @@ ovn (22.06.0-1) unstable; urgency=low > > * New upstream version > > - -- OVN team Thu, 19 May 2022 15:11:38 -0400 > + -- OVN team Fri, 03 Jun 2022 11:54:08 -0400 > > ovn (22.03.0-1) unstable; urgency=low > > -- > 2.31.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
Re: [ovs-dev] [PATCH ovn branch-21.12 1/2] Set release date for 21.12.2.
On Fri, Jun 3, 2022 at 11:55 AM Mark Michelson wrote: > > Signed-off-by: Mark Michelson Acked-by: Numan Siddique Numan > --- > NEWS | 3 ++- > debian/changelog | 2 +- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/NEWS b/NEWS > index de3330ab6..6ff9b8fdf 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,5 +1,6 @@ > -OVN v21.12.2 - xx xxx > +OVN v21.12.2 - 03 Jun 2022 > -- > + - Bug fixes >- When configured to log packets matching ACLs, log the direction (logical > pipeline) too. >- Replaced the usage of masked ct_label by ct_mark in most cases to work > diff --git a/debian/changelog b/debian/changelog > index 1a1c7364e..5bf39c320 100644 > --- a/debian/changelog > +++ b/debian/changelog > @@ -2,7 +2,7 @@ OVN (21.12.2-1) unstable; urgency=low > [ OVN team ] > * New upstream version > > - -- OVN team Fri, 11 Mar 2022 13:22:24 -0500 > + -- OVN team Fri, 03 Jun 2022 11:53:58 -0400 > > OVN (21.12.1-1) unstable; urgency=low > [ OVN team ] > -- > 2.31.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
Re: [ovs-dev] [PATCH ovn branch-21.12 2/2] Prepare for 21.12.3.
On Fri, Jun 3, 2022 at 11:56 AM Mark Michelson wrote: > > Signed-off-by: Mark Michelson Acked-by: Numan Siddique Numan > --- > NEWS | 3 +++ > configure.ac | 2 +- > debian/changelog | 6 ++ > 3 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/NEWS b/NEWS > index 6ff9b8fdf..31e08b015 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,3 +1,6 @@ > +OVN v21.12.3 - xx xxx > +-- > + > OVN v21.12.2 - 03 Jun 2022 > -- > - Bug fixes > diff --git a/configure.ac b/configure.ac > index c4bf08db7..f37afb7bb 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -13,7 +13,7 @@ > # limitations under the License. > > AC_PREREQ(2.63) > -AC_INIT(ovn, 21.12.2, b...@openvswitch.org) > +AC_INIT(ovn, 21.12.3, b...@openvswitch.org) > AC_CONFIG_MACRO_DIR([m4]) > AC_CONFIG_AUX_DIR([build-aux]) > AC_CONFIG_HEADERS([config.h]) > diff --git a/debian/changelog b/debian/changelog > index 5bf39c320..abf5ab264 100644 > --- a/debian/changelog > +++ b/debian/changelog > @@ -1,3 +1,9 @@ > +OVN (21.12.3-1) unstable; urgency=low > + [ OVN team ] > + * New upstream version > + > + -- OVN team Fri, 03 Jun 2022 11:53:58 -0400 > + > OVN (21.12.2-1) unstable; urgency=low > [ OVN team ] > * New upstream version > -- > 2.31.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
Re: [ovs-dev] [PATCH ovn branch-22.06 2/2] Prepare for 22.06.1.
On Fri, Jun 3, 2022 at 11:55 AM Mark Michelson wrote: > > Signed-off-by: Mark Michelson Acked-by: Numan Siddique Numan > --- > NEWS | 3 +++ > configure.ac | 2 +- > debian/changelog | 6 ++ > 3 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/NEWS b/NEWS > index e335f64c2..cc58306d7 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,3 +1,6 @@ > +OVN v22.06.1 - xx xxx > +-- > + > OVN v22.06.0 - 03 Jun 2022 > -- >- Support IGMP and MLD snooping on transit logical switches that connect > diff --git a/configure.ac b/configure.ac > index b649441bc..739e0295e 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -13,7 +13,7 @@ > # limitations under the License. > > AC_PREREQ(2.63) > -AC_INIT(ovn, 22.06.0, b...@openvswitch.org) > +AC_INIT(ovn, 22.06.1, b...@openvswitch.org) > AC_CONFIG_MACRO_DIR([m4]) > AC_CONFIG_AUX_DIR([build-aux]) > AC_CONFIG_HEADERS([config.h]) > diff --git a/debian/changelog b/debian/changelog > index caef68452..1c2de53bd 100644 > --- a/debian/changelog > +++ b/debian/changelog > @@ -1,3 +1,9 @@ > +OVN (22.06.1-1) unstable; urgency=low > + [ OVN team ] > + * New upstream version > + > + -- OVN team Fri, 03 Jun 2022 11:54:08 -0400 > + > ovn (22.06.0-1) unstable; urgency=low > > * New upstream version > -- > 2.31.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
Re: [ovs-dev] [PATCH ovn v2 0/3] Use masked ct_mark in a backwards compatible way.
I merged each of the individual patch series to their appropriate branches. On 6/3/22 09:25, Dumitru Ceara wrote: Explicit backports for these patches (except 3/3 which should only be applied to main) are available at: - branch-22.06: https://github.com/dceara/ovn/tree/bz2091565-ct-masked-mark-follow-up-22.06 - branch-22.03: https://github.com/dceara/ovn/tree/bz2091565-ct-masked-mark-follow-up-22.03 - branch-21.12: https://github.com/dceara/ovn/tree/bz2091565-ct-masked-mark-follow-up-21.12 V2: - Addressed comments from Mark and Han: - added a 3rd patch to flip the lb_hairpin_use_ct_mark default to 'true'. - changed I-P of the northd internal version into I-P of the SB_Global options. - consolidated all ovn-controller ct_mark masked access support under the "ct-no-masked-label" feature name. Dumitru Ceara (3): northd: ovn-controller: Use ct_mark.natted only when ct_lb_mark is used. northd: Use ct_mark.blocked and ecmp_reply_port only when all chassis support it. controller: Use ct_mark by default for load balancer hairpin flows. controller/chassis.c| 5 +- controller/ovn-controller.c | 2 +- include/ovn/features.h | 4 +- northd/northd.c | 172 +++- northd/northd.h | 2 +- ovn-sb.xml | 12 ++- tests/ovn-northd.at | 96 +++- 7 files changed, 222 insertions(+), 71 deletions(-) ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn branch-22.03 2/2] Prepare for 22.03.2.
I merged these changes and tagged v22.03.1. On 6/3/22 15:25, Numan Siddique wrote: On Fri, Jun 3, 2022 at 11:55 AM Mark Michelson wrote: Signed-off-by: Mark Michelson Acked-by: Numan Siddique Numan --- NEWS | 3 +++ configure.ac | 2 +- debian/changelog | 6 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 166470747..3e1c6fb11 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +OVN v22.03.2 - xx xxx +-- + OVN v22.03.1 - 03 Jun 2022 -- - Bug fixes diff --git a/configure.ac b/configure.ac index 70f86e1f5..f1d03528f 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.63) -AC_INIT(ovn, 22.03.1, b...@openvswitch.org) +AC_INIT(ovn, 22.03.2, b...@openvswitch.org) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) AC_CONFIG_HEADERS([config.h]) diff --git a/debian/changelog b/debian/changelog index 1b9ebca1b..7abf3c332 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +OVN (22.03.2-1) unstable; urgency=low + [ OVN team ] + * New upstream version + + -- OVN team Fri, 03 Jun 2022 11:54:04 -0400 + OVN (22.03.1-1) unstable; urgency=low [ OVN team ] * New upstream version -- 2.31.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
Re: [ovs-dev] [PATCH ovn branch-21.12 2/2] Prepare for 21.12.3.
I merged these changes and tagged v21.12.2 On 6/3/22 15:27, Numan Siddique wrote: On Fri, Jun 3, 2022 at 11:56 AM Mark Michelson wrote: Signed-off-by: Mark Michelson Acked-by: Numan Siddique Numan --- NEWS | 3 +++ configure.ac | 2 +- debian/changelog | 6 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 6ff9b8fdf..31e08b015 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +OVN v21.12.3 - xx xxx +-- + OVN v21.12.2 - 03 Jun 2022 -- - Bug fixes diff --git a/configure.ac b/configure.ac index c4bf08db7..f37afb7bb 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.63) -AC_INIT(ovn, 21.12.2, b...@openvswitch.org) +AC_INIT(ovn, 21.12.3, b...@openvswitch.org) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) AC_CONFIG_HEADERS([config.h]) diff --git a/debian/changelog b/debian/changelog index 5bf39c320..abf5ab264 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +OVN (21.12.3-1) unstable; urgency=low + [ OVN team ] + * New upstream version + + -- OVN team Fri, 03 Jun 2022 11:53:58 -0400 + OVN (21.12.2-1) unstable; urgency=low [ OVN team ] * New upstream version -- 2.31.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
Re: [ovs-dev] [PATCH ovn branch-22.06 2/2] Prepare for 22.06.1.
I merged these changes and tagged v22.06.0 On 6/3/22 15:26, Numan Siddique wrote: On Fri, Jun 3, 2022 at 11:55 AM Mark Michelson wrote: Signed-off-by: Mark Michelson Acked-by: Numan Siddique Numan --- NEWS | 3 +++ configure.ac | 2 +- debian/changelog | 6 ++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index e335f64c2..cc58306d7 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +OVN v22.06.1 - xx xxx +-- + OVN v22.06.0 - 03 Jun 2022 -- - Support IGMP and MLD snooping on transit logical switches that connect diff --git a/configure.ac b/configure.ac index b649441bc..739e0295e 100644 --- a/configure.ac +++ b/configure.ac @@ -13,7 +13,7 @@ # limitations under the License. AC_PREREQ(2.63) -AC_INIT(ovn, 22.06.0, b...@openvswitch.org) +AC_INIT(ovn, 22.06.1, b...@openvswitch.org) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR([build-aux]) AC_CONFIG_HEADERS([config.h]) diff --git a/debian/changelog b/debian/changelog index caef68452..1c2de53bd 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +OVN (22.06.1-1) unstable; urgency=low + [ OVN team ] + * New upstream version + + -- OVN team Fri, 03 Jun 2022 11:54:08 -0400 + ovn (22.06.0-1) unstable; urgency=low * New upstream version -- 2.31.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
[ovs-dev] [PATCH v13 ovn] Implement RARP activation strategy for ports
When options:activation-strategy is set to "rarp" for LSP, when used in
combination with multiple chassis names listed in
options:requested-chassis, additional chassis will install special flows
that would block all ingress and egress traffic for the port until a
special activation event happens.
For "rarp" strategy, an observation of a RARP packet sent from the port
on the additional chassis is such an event. When it occurs, a special
flow passes control to a controller() action handler that eventually
removes the installed blocking flows and also marks the port as
options:additional-chassis-activated in southbound db.
This feature is useful in live migration scenarios where it's not
advisable to unlock the destination port location prematurily to avoid
duplicate packets originating from the port.
Signed-off-by: Ihar Hrachyshka
---
v13: use resubmit() action to reinject RARP into pipeline.
v13: lock / unlock pinctrl_mutex in functions invoked from main thread.
v13: db_is_port_activated->lport_is_activated_by_activation_strategy.
---
NEWS| 2 +
controller/lport.c | 22 +++
controller/lport.h | 3 +
controller/ovn-controller.c | 87 +
controller/physical.c | 94 ++
controller/pinctrl.c| 145 +-
controller/pinctrl.h| 13 ++
include/ovn/actions.h | 3 +
northd/northd.c | 10 +
northd/ovn-northd.c | 5 +-
ovn-nb.xml | 11 ++
ovn-sb.xml | 15 ++
tests/ovn.at| 365
13 files changed, 772 insertions(+), 3 deletions(-)
diff --git a/NEWS b/NEWS
index 2ee283a56..7c54670ed 100644
--- a/NEWS
+++ b/NEWS
@@ -29,6 +29,8 @@ OVN v22.06.0 - XX XXX
- Added support for setting the Next server IP in the DHCP header
using the private DHCP option - 253 in native OVN DHCPv4 responder.
- Support list of chassis for Logical_Switch_Port:options:requested-chassis.
+ - Support Logical_Switch_Port:options:activation-strategy for live migration
+scenarios.
OVN v22.03.0 - 11 Mar 2022
--
diff --git a/controller/lport.c b/controller/lport.c
index bf55d83f2..add7e91aa 100644
--- a/controller/lport.c
+++ b/controller/lport.c
@@ -197,3 +197,25 @@ get_peer_lport(const struct sbrec_port_binding *pb,
peer_name);
return (peer && peer->datapath) ? peer : NULL;
}
+
+bool
+lport_is_activated_by_activation_strategy(const struct sbrec_port_binding *pb,
+ const struct sbrec_chassis *chassis)
+{
+const char *activated_chassis = smap_get(&pb->options,
+ "additional-chassis-activated");
+if (activated_chassis) {
+char *save_ptr;
+char *tokstr = xstrdup(activated_chassis);
+for (const char *chassis_name = strtok_r(tokstr, ",", &save_ptr);
+ chassis_name != NULL;
+ chassis_name = strtok_r(NULL, ",", &save_ptr)) {
+if (!strcmp(chassis_name, chassis->name)) {
+free(tokstr);
+return true;
+}
+}
+free(tokstr);
+}
+return false;
+}
diff --git a/controller/lport.h b/controller/lport.h
index 115881655..644c67255 100644
--- a/controller/lport.h
+++ b/controller/lport.h
@@ -70,4 +70,7 @@ const struct sbrec_port_binding *lport_get_peer(
const struct sbrec_port_binding *lport_get_l3gw_peer(
const struct sbrec_port_binding *,
struct ovsdb_idl_index *sbrec_port_binding_by_name);
+bool
+lport_is_activated_by_activation_strategy(const struct sbrec_port_binding *pb,
+ const struct sbrec_chassis *chassis);
#endif /* controller/lport.h */
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index b597c0e37..a37dfcb78 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -1047,6 +1047,50 @@ en_ofctrl_is_connected_run(struct engine_node *node,
void *data)
engine_set_node_state(node, EN_UNCHANGED);
}
+struct ed_type_activated_ports {
+struct ovs_list *activated_ports;
+};
+
+static void *
+en_activated_ports_init(struct engine_node *node OVS_UNUSED,
+struct engine_arg *arg OVS_UNUSED)
+{
+struct ed_type_activated_ports *data = xzalloc(sizeof *data);
+data->activated_ports = NULL;
+return data;
+}
+
+static void
+en_activated_ports_cleanup(void *data_)
+{
+struct ed_type_activated_ports *data = data_;
+
+struct activated_port *pp;
+if (!data->activated_ports) {
+return;
+}
+
+LIST_FOR_EACH_POP (pp, list, data->activated_ports) {
+free(pp);
+}
+free(data->activated_ports);
+data->activated_ports = NULL;
+}
+
+static void
+en_activated_ports_run(struct engine_node *node, void *data_)
+{
+struct ed_type_activated_ports *data = data_;
+
+en_activated_ports_cleanu
Re: [ovs-dev] [PATCH v12 ovn] Implement RARP activation strategy for ports
Thanks Numan.
All comments handled in v13 I just sent.
Ihar
On Thu, Jun 2, 2022 at 5:06 PM Numan Siddique wrote:
>
> On Wed, Jun 1, 2022 at 7:00 PM Ihar Hrachyshka wrote:
> >
> > This is the remaining piece in multi-chassis series. This new version
> > has no flow modification done in pinctrl thread. Instead, I-P engine
> > is notified of any newly activated ports, which then triggers pflow
> > update for lports.
> >
> > Ihar
> >
> > On Wed, Jun 1, 2022 at 6:57 PM Ihar Hrachyshka wrote:
> > >
> > > When options:activation-strategy is set to "rarp" for LSP, when used in
> > > combination with multiple chassis names listed in
> > > options:requested-chassis, additional chassis will install special flows
> > > that would block all ingress and egress traffic for the port until a
> > > special activation event happens.
> > >
> > > For "rarp" strategy, an observation of a RARP packet sent from the port
> > > on the additional chassis is such an event. When it occurs, a special
> > > flow passes control to a controller() action handler that eventually
> > > removes the installed blocking flows and also marks the port as
> > > options:additional-chassis-activated in southbound db.
> > >
> > > This feature is useful in live migration scenarios where it's not
> > > advisable to unlock the destination port location prematurily to avoid
> > > duplicate packets originating from the port.
> > >
> > > Signed-off-by: Ihar Hrachyshka
>
> Hi Ihar,
>
> Thanks for addressing the comments in v12.
>
> I've got a few comments. Please see below.
>
>
> > > ---
> > > NEWS| 2 +
> > > controller/ovn-controller.c | 87 +
> > > controller/physical.c | 92 +
> > > controller/pinctrl.c| 195 ++-
> > > controller/pinctrl.h| 13 ++
> > > include/ovn/actions.h | 3 +
> > > northd/northd.c | 10 +
> > > northd/ovn-northd.c | 5 +-
> > > ovn-nb.xml | 11 ++
> > > ovn-sb.xml | 15 ++
> > > tests/ovn.at| 365
> > > 11 files changed, 795 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/NEWS b/NEWS
> > > index 2ee283a56..7c54670ed 100644
> > > --- a/NEWS
> > > +++ b/NEWS
> > > @@ -29,6 +29,8 @@ OVN v22.06.0 - XX XXX
> > >- Added support for setting the Next server IP in the DHCP header
> > > using the private DHCP option - 253 in native OVN DHCPv4 responder.
> > >- Support list of chassis for
> > > Logical_Switch_Port:options:requested-chassis.
> > > + - Support Logical_Switch_Port:options:activation-strategy for live
> > > migration
> > > +scenarios.
> > >
> > > OVN v22.03.0 - 11 Mar 2022
> > > --
> > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > > index b597c0e37..3c2f76359 100644
> > > --- a/controller/ovn-controller.c
> > > +++ b/controller/ovn-controller.c
> > > @@ -1047,6 +1047,50 @@ en_ofctrl_is_connected_run(struct engine_node
> > > *node, void *data)
> > > engine_set_node_state(node, EN_UNCHANGED);
> > > }
> > >
> > > +struct ed_type_activated_ports {
> > > +struct ovs_list *activated_ports;
> > > +};
> > > +
> > > +static void *
> > > +en_activated_ports_init(struct engine_node *node OVS_UNUSED,
> > > +struct engine_arg *arg OVS_UNUSED)
> > > +{
> > > +struct ed_type_activated_ports *data = xzalloc(sizeof *data);
> > > +data->activated_ports = get_activated_ports();
>
> I think calling get_activated_ports() is not required.
>
> en_activated_ports_run() is anyway doing this job.
>
>
> > > +return data;
> > > +}
> > > +
> > > +static void
> > > +en_activated_ports_cleanup(void *data_)
> > > +{
> > > +struct ed_type_activated_ports *data = data_;
> > > +
> > > +struct activated_port *pp;
> > > +if (!data->activated_ports) {
> > > +return;
> > > +}
> > > +
> > > +LIST_FOR_EACH_POP (pp, list, data->activated_ports) {
> > > +free(pp);
> > > +}
> > > +free(data->activated_ports);
> > > +data->activated_ports = NULL;
> > > +}
> > > +
> > > +static void
> > > +en_activated_ports_run(struct engine_node *node, void *data_)
> > > +{
> > > +struct ed_type_activated_ports *data = data_;
> > > +
> > > +en_activated_ports_cleanup(data);
> > > +data->activated_ports = get_activated_ports();
> > > +if (data->activated_ports) {
> > > +engine_set_node_state(node, EN_UNCHANGED);
> > > +} else {
> > > +engine_set_node_state(node, EN_UPDATED);
> > > +}
> > > +}
> > > +
> > > /* This engine node is to wrap the OVS_interface input and maintain a
> > > copy of
> > > * the old version of data for the column external_ids.
> > > *
> > > @@ -1421,6 +1465,44 @@ en_runtime_data_run(struct engine_node *node, void
> > > *data)
> > > engine_set_node_state(node, EN_UPDATED);
> > > }
> > >
> > > +static bool
> > > +runtime_data_acti
[ovs-dev] [PATCH ovn] Lock pinctrl_mutex for pinctrl_wait
The function is called from main thread, and wait_* subprocedures access
data structures that are managed by pinctrl thread, so make sure the
access to them is guarded.
Signed-off-by: Ihar Hrachyshka
---
controller/pinctrl.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 428863293..9a1a0faa1 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -4030,12 +4030,14 @@ prepare_ipv6_ras(const struct shash
*local_active_ports_ras,
void
pinctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn)
{
+ovs_mutex_lock(&pinctrl_mutex);
wait_put_mac_bindings(ovnsb_idl_txn);
wait_controller_event(ovnsb_idl_txn);
wait_put_vport_bindings(ovnsb_idl_txn);
int64_t new_seq = seq_read(pinctrl_main_seq);
seq_wait(pinctrl_main_seq, new_seq);
wait_put_fdbs(ovnsb_idl_txn);
+ovs_mutex_unlock(&pinctrl_mutex);
}
/* Called by ovn-controller. */
--
2.34.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn 2/2] northd.c: Add option to enable MAC learning on localnet
On Wed, Jun 1, 2022 at 8:28 AM Ales Musil wrote:
>
> The localnet is excluded from MAC learning for scale
> reason. However there might be a valid workflow
> when yo uwant to enable the learning and benefit
> for that for HW offload. Add option called
> 'localnet_learn_fdb' to LSP, which will enable/disable
> the learning. Setting it as disabled by default.
>
> Reported-at: https://bugzilla.redhat.com/2070529
> Signed-off-by: Ales Musil
Thanks for adding the support.
A few minor comments below.
Numan
> ---
> northd/northd.c | 10 --
> ovn-nb.xml | 7 +++
> ovn-sb.xml | 1 +
> tests/ovn-northd.at | 37 +
> 4 files changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 16ea7a6aa..b3077714e 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -5408,8 +5408,14 @@ build_lswitch_learn_fdb_op(
> struct ovn_port *op, struct hmap *lflows,
> struct ds *actions, struct ds *match)
> {
> -if (op->nbsp && !op->n_ps_addrs && !strcmp(op->nbsp->type, "") &&
> -op->has_unknown) {
> +if (!op->nbsp) {
> +return;
> +}
> +
> +bool localnet_learn_fdb = smap_get_bool(&op->nbsp->options,
> +"localnet_learn_fdb", false);
I'd suggest to add a helper function for the above i.e to check if
learn fdb is enabled or not for localnet ports
and modify the below 'if' as
if (!op->n_ps_addrs && op->has_unknown && (!strcmp(op->nbsp->type, "") ||
(lsp_is_localnet(op->nbsp) &&
localnet_lsp_has_learn_(op->nbsp->options)))
{
///
}
This would avoid unnecessary smap_get_bool() call for other lsp types.
I think it would be good if you can add a new test in ovn.at or
enhance existing OVN FDB (MAC learning) test cases in ovn.at
to cover this scenario.
You can create a vif in the localnet bridge and inject some packets
from there so that the packet enters the br-int via the patch port
and the test can check if the mac learning is working as expected or not.
Thanks
Numan
> +if (!op->n_ps_addrs && op->has_unknown && (!strcmp(op->nbsp->type, "") ||
> +(localnet_learn_fdb && lsp_is_localnet(op->nbsp {
> ds_clear(match);
> ds_clear(actions);
> ds_put_format(match, "inport == %s", op->json_key);
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 3e3e142b3..9df6b1aab 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -976,6 +976,13 @@
>headers. Supported values: 802.11q (default), 802.11ad.
>
>
> + +type='{"type": "boolean"}'>
> + Optional. Allows localnet port to learn MACs and store them in FDB
> + table if set to true. The default value is
> + false.
> +
> +
>
>
>
> diff --git a/ovn-sb.xml b/ovn-sb.xml
> index 4c35dda36..3d92ba88f 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -4602,6 +4602,7 @@ tcp.flags = RST;
>
>
>This table is primarily used to learn the MACs observed on a VIF
> + (or a localnet port with 'localnet_learn_fdb' enabled)
>which belongs to a Logical_Switch_Port record in
>OVN_Northbound whose port security is disabled
>and 'unknown' address set. If port security is disabled on a
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 5bd0935e7..89eeb0894 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -7418,3 +7418,40 @@ AT_CHECK([cat sw0flows | grep -e port_sec | sort | sed
> 's/table=./table=?/' ], [
>
> AT_CLEANUP
> ])
> +
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([Localnet MAC learning option])
> +ovn_start
> +
> +AT_CHECK([ovn-nbctl ls-add ls0])
> +
> +AT_CHECK([ovn-nbctl lsp-add ls0 ln_port])
> +AT_CHECK([ovn-nbctl lsp-set-addresses ln_port unknown])
> +AT_CHECK([ovn-nbctl lsp-set-type ln_port localnet])
> +AT_CHECK([ovn-nbctl lsp-set-options ln_port network_name=phys])
> +AT_CHECK([ovn-nbctl --wait=sb sync])
> +
> +# Check MAC learning flows with 'localnet_learn_fdb' default (false)
> +AT_CHECK([ovn-sbctl dump-flows ls0 | grep -e 'ls_in_\(put\|lookup\)_fdb' |
> sort | sed 's/table=./table=?/'], [0], [dnl
> + table=? (ls_in_lookup_fdb ), priority=0, match=(1), action=(next;)
> + table=? (ls_in_put_fdb ), priority=0, match=(1), action=(next;)
> +])
> +
> +# Enable 'localnet_learn_fdb' and check the flows
> +AT_CHECK([ovn-nbctl --wait=sb lsp-set-options ln_port
> localnet_learn_fdb=true])
> +AT_CHECK([ovn-sbctl dump-flows ls0 | grep -e 'ls_in_\(put\|lookup\)_fdb' |
> sort | sed 's/table=./table=?/'], [0], [dnl
> + table=? (ls_in_lookup_fdb ), priority=0, match=(1), action=(next;)
> + table=? (ls_in_lookup_fdb ), priority=100 , match=(inport ==
> "ln_port"), action=(reg0[[11]] = lookup_fdb(inport, eth.src); next;)
> + table=? (ls_in_put_fdb ), priority=0, match=(1), action=(next;)
> + table=? (ls_in_put_fdb ), priority=100 , match=(inp
