Re: [ovs-dev] [PATCH v2] netdev-offload: make netdev-offload-tc work with flow-restore-wait
On Fri, Apr 22, 2022 at 1:41 AM Eelco Chaudron wrote:
>
>
>
> On 15 Apr 2022, at 13:25, wenx05124...@163.com wrote:
>
> > From: wenxu
> >
> > The netdev-offload in tc mode can't work with flow-restore-wait.
> > When the vswitchd restart with flow-restore-wait, the tc qdisc
> > will be delete in netdev_set_flow_api_enabled. The netdev flow
> > api can be enabled after the flow-restore-wait flag removing.
> >
> > Signed-off-by: wenxu
Hi, I found this patch useful, but it seems inactive for a long time. I
hope we can revive and update it.
Regardless of the issues pointed out by Eelco, this patch works well for
traffic not going through tunnels, but for tunnelled traffic, e.g. geneve
traffic, I found that even with the patch, when OVS starts, the ingress
qdisc for the genev_sys_6081 device is deleted, so the traffic is still
broken even with flow-restore-wait set. I didn't find yet in the code where
it could be deleted. Any hint/insight would be appreciated.
> > ---
> > lib/netdev-linux.c | 16
> > vswitchd/bridge.c | 18 ++
> > 2 files changed, 18 insertions(+), 16 deletions(-)
> >
> > diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> > index 9d12502..b6315e3 100644
> > --- a/lib/netdev-linux.c
> > +++ b/lib/netdev-linux.c
> > @@ -2784,17 +2784,17 @@ netdev_linux_set_policing(struct netdev
*netdev_, uint32_t kbits_rate,
> > goto out;
> > }
> >
> > -/* Remove any existing ingress qdisc. */
> > -error = tc_add_del_qdisc(ifindex, false, 0, TC_INGRESS);
> > -if (error) {
> > -VLOG_WARN_RL(, "%s: removing policing failed: %s",
> > - netdev_name, ovs_strerror(error));
> > -goto out;
> > -}
> > -
> > if (kbits_rate || kpkts_rate) {
> > const char *cls_name = "matchall";
> >
> > +/* Remove any existing ingress qdisc. */
> > +error = tc_add_del_qdisc(ifindex, false, 0, TC_INGRESS);
> > +if (error) {
> > +VLOG_WARN_RL(, "%s: removing policing failed: %s",
> > + netdev_name, ovs_strerror(error));
> > +goto out;
> > +}
>
> Are we sure we are not breaking a corner case here where something might
already have been configured, and we are not cleaning it up?
>
> I.e. what if we configured some rate limiting, and now we unconfigure it,
the values are all zero, and it will not be removed?
>
Agree. This is incorrect. I think it is better to use the approach from v1,
to avoid configuring qos when flow-restore-wait=true, hopefully good enough
for the short period before flow restore is done.
> > +
> > error = tc_add_del_qdisc(ifindex, true, 0, TC_INGRESS);
> > if (error) {
> > VLOG_WARN_RL(, "%s: adding policing qdisc failed: %s",
> > diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> > index e328d8e..d8843a7 100644
> > --- a/vswitchd/bridge.c
> > +++ b/vswitchd/bridge.c
> > @@ -3288,8 +3288,17 @@ bridge_run(void)
> > }
> > cfg = ovsrec_open_vswitch_first(idl);
> >
> > +/* Once the value of flow-restore-wait is false, we no longer
should
> > + * check its value from the database. */
> > +if (cfg && ofproto_get_flow_restore_wait()) {
> > +ofproto_set_flow_restore_wait(smap_get_bool(>other_config,
> > +"flow-restore-wait", false));
> > +}
> > +
> > if (cfg) {
> > -netdev_set_flow_api_enabled(>other_config);
> > +if (!ofproto_get_flow_restore_wait()) {
> > +netdev_set_flow_api_enabled(>other_config);
>
> See my previous comment here, this needs more input:
>
> However, the main problem with this patch might be the following
statement in the documentation:
>
> “The default value is false. Changing this value requires
restarting the daemon”
>
> I’ve dealt with a case in the past that was failing because I enabled
offload but did not restart. Unfortunately, I can not remember the details,
I think it had something to do with feature detection. I’ve copied on some
more folks who might know more details about this requirement.
>
>
Based on my test, this seems to be fine, but I am not sure if I am just
lucky with my environment.
Thanks,
Han
>
> > +}
> > dpdk_init(>other_config);
> > userspace_tso_init(>other_config);
> > }
> > @@ -3300,13 +3309,6 @@ bridge_run(void)
> > * returns immediately. */
> > bridge_init_ofproto(cfg);
> >
> > -/* Once the value of flow-restore-wait is false, we no longer
should
> > - * check its value from the database. */
> > -if (cfg && ofproto_get_flow_restore_wait()) {
> > -ofproto_set_flow_restore_wait(smap_get_bool(>other_config,
> > -"flow-restore-wait", false));
> > -}
> > -
> > bridge_run__();
> >
> > /* Re-configure SSL. We do this on every trip through the main
loop,
>
> ___
> dev mailing list
>
Re: [ovs-dev] [PATCH] ofproto-dpif: Fix vxlan with different name del/add failed.
Thanks Tao for fixing this. I think the title can be more generic because
this problem and fix applies to all tunnel types rather than just VXLAN.
On Tue, Mar 12, 2024 at 7:04 AM Tao Liu wrote:
>
> Reproduce:
> ovs-vsctl add-port br-int p0 \
> -- set interface p0 type=vxlan options:remote_ip=10.10.10.1
>
> sleep 2
>
> ovs-vsctl --if-exists del-port p0 \
> -- add-port br-int p1 \
> -- set interface p1 type=vxlan options:remote_ip=10.10.10.1
> ovs-vsctl: Error detected while setting up 'p1': could not add
> network device p1 to ofproto (File exists).
>
> vswitchd log:
> bridge|INFO|bridge br-int: added interface p0 on port 1106
> bridge|INFO|bridge br-int: deleted interface p0 on port 1106
> tunnel|WARN|p1: attempting to add tunnel port with same config as port
'p0' (::->10.10.10.1, key=0, legacy_l2, dp port=122)
> ofproto|WARN|br-int: could not add port p1 (File exists)
> bridge|WARN|could not add network device p1 to ofproto (File exists)
>
> CallTrace:
> bridge_reconfigure
> bridge_del_ports
> port_destroy
> iface_destroy__
> netdev_remove <-- netdev p0 removed
> bridge_delete_or_reconfigure_ports
> OFPROTO_PORT_FOR_EACH
> ofproto_port_dump_next
> port_dump_next
> port_query_by_name<-- netdev_shash do not contain p0
> ofproto_port_del<-- p0 do not del in ofproto
> bridge_add_ports
> bridge_add_ports__
> iface_create
> iface_do_create
> ofproto_port_add<-- p1 add failed
>
> Fixes: fe83f81df977 ("netdev: Remove netdev from global shash when the
user is changing interface configuration.")
> Signed-off-by: Tao Liu
> ---
> ofproto/ofproto-dpif.c | 13 +
> tests/tunnel.at| 12
> 2 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index f59d69c4d..0cac3050d 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -3905,14 +3905,19 @@ port_query_by_name(const struct ofproto
*ofproto_, const char *devname,
>
> if (sset_contains(>ghost_ports, devname)) {
> const char *type = netdev_get_type_from_name(devname);
> +const struct ofport *ofport =
> +shash_find_data(>up.port_by_name,
devname);
> +if (!type && ofport && ofport->netdev) {
> +type = netdev_get_type(ofport->netdev);
> +}
>
> /* We may be called before ofproto->up.port_by_name is populated
with
> * the appropriate ofport. For this reason, we must get the
name and
> - * type from the netdev layer directly. */
> + * type from the netdev layer directly.
> + * When a port deleted, the corresponding netdev is also removed
from
> + * netdev_shash. netdev_get_type_from_name returns NULL in such
case.
> + * We should try to get type from ofport->netdev. */
nit: this comment is better to be moved to the above where we are trying to
get the type from ofport.
Otherwise looks good to me:
Acked-by: Han Zhou
Tested-by: Han Zhou
> if (type) {
> -const struct ofport *ofport;
> -
> -ofport = shash_find_data(>up.port_by_name, devname);
> ofproto_port->ofp_port = ofport ? ofport->ofp_port :
OFPP_NONE;
> ofproto_port->name = xstrdup(devname);
> ofproto_port->type = xstrdup(type);
> diff --git a/tests/tunnel.at b/tests/tunnel.at
> index 71e7c2df4..9d539ee6f 100644
> --- a/tests/tunnel.at
> +++ b/tests/tunnel.at
> @@ -1269,6 +1269,18 @@ OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
> OVS_APP_EXIT_AND_WAIT([ovsdb-server])]
> AT_CLEANUP
>
> +AT_SETUP([tunnel - re-create port with different name])
> +OVS_VSWITCHD_START(
> + [add-port br0 p0 -- set int p0 type=vxlan
options:remote_ip=10.10.10.1])
> +
> +AT_CHECK([ovs-vsctl --if-exists del-port p0 -- \
> + add-port br0 p1 -- \
> + set int p1 type=vxlan options:remote_ip=10.10.10.1])
> +
> +OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])]
> +AT_CLEANUP
> +
> AT_SETUP([tunnel - SRV6 basic])
> OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=dummy \
> ofport_request=1 \
> --
> 2.31.1
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto-dpif-upcall: try lock for umap iteration during sweep
Bleep bloop. Greetings LIU Yulong, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: The subject summary should start with a capital. WARNING: The subject summary should end with a dot. Subject: ofproto-dpif-upcall: try lock for umap iteration during sweep Lines checked: 63, Warnings: 2, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ofproto-dpif-upcall: try lock for umap iteration during sweep
A potential race condition happened with the following 3 threads:
* PMD thread replaced the old_ukey and transitioned the state to
UKEY_DELETED.
* RCU thread is freeing the old_ukey mutex.
* While the revalidator thread is trying to lock the old_ukey mutex.
Then vswitchd process aborts at the revalidator thread try_lock of
ukey->mutex because of the NULL pointer.
This patch adds the try_lock for the ukeys' basket umap to avoid
the PMD and revalidator access to the same umap for replacing the
ukey and transitioning the ukey state.
More details can be found at:
[1] https://mail.openvswitch.org/pipermail/ovs-discuss/2024-March/052973.html
[2] https://mail.openvswitch.org/pipermail/ovs-discuss/2024-February/052949.html
Signed-off-by: LIU Yulong
---
ofproto/ofproto-dpif-upcall.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 9a5c5c29c..ef13f820a 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -2974,6 +2974,10 @@ revalidator_sweep__(struct revalidator *revalidator,
bool purge)
struct umap *umap = >ukeys[i];
size_t n_ops = 0;
+if (ovs_mutex_trylock(>mutex)) {
+continue;
+}
+
CMAP_FOR_EACH(ukey, cmap_node, >cmap) {
enum ukey_state ukey_state;
@@ -3013,9 +3017,7 @@ revalidator_sweep__(struct revalidator *revalidator, bool
purge)
if (ukey_state == UKEY_EVICTED) {
/* The common flow deletion case involves deletion of the flow
* during the dump phase and ukey deletion here. */
-ovs_mutex_lock(>mutex);
ukey_delete(umap, ukey);
-ovs_mutex_unlock(>mutex);
}
if (n_ops == REVALIDATE_MAX_BATCH) {
@@ -3025,6 +3027,7 @@ revalidator_sweep__(struct revalidator *revalidator, bool
purge)
n_ops = 0;
}
}
+ovs_mutex_unlock(>mutex);
if (n_ops) {
push_ukey_ops(udpif, umap, ops, n_ops);
--
2.27.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ovs-monitor-ipsec: LibreSwan autodetect paths.
In v4.0, LibreSwan changed a default paths that had been hardcoded in
ovs-monitor-ipsec, breaking some uses of this script. This patch adds
support for both old and newer versions by auto detecting the location
of these paths from LibreSwan shell script environment variables.
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1975039
Reported-by: Qijun Ding
Fixes: d6afbc00d5b3 ("ipsec: Allow custom file locations.")
Signed-off-by: Mike Pattrick
---
ipsec/ovs-monitor-ipsec.in | 31 +++
1 file changed, 27 insertions(+), 4 deletions(-)
diff --git a/ipsec/ovs-monitor-ipsec.in b/ipsec/ovs-monitor-ipsec.in
index 7945162f9..6c28f30f4 100755
--- a/ipsec/ovs-monitor-ipsec.in
+++ b/ipsec/ovs-monitor-ipsec.in
@@ -456,15 +456,38 @@ conn prevent_unencrypted_vxlan
CERT_PREFIX = "ovs_cert_"
CERTKEY_PREFIX = "ovs_certkey_"
+def collect_environment(self):
+"""Extract important paths from ipsec file."""
+env = {
+"IPSEC_CONF": "/etc/ipsec.conf",
+"IPSEC_NSSDIR": "/etc/ipsec.d",
+"IPSEC_RUNDIR": "/run/pluto"
+}
+try:
+with open(self.IPSEC) as fh:
+e_list = re.findall("^([A-Z_]+)=.*:-(.*)}",
+fh.read(),
+re.MULTILINE)
+except:
+return env
+
+for k, v in e_list:
+env[k] = v
+
+return env
+
def __init__(self, libreswan_root_prefix, args):
-ipsec_conf = args.ipsec_conf if args.ipsec_conf else "/etc/ipsec.conf"
-ipsec_d = args.ipsec_d if args.ipsec_d else "/etc/ipsec.d"
+self.IPSEC = libreswan_root_prefix + "/usr/sbin/ipsec"
+
+env = self.collect_environment()
+
+ipsec_conf = args.ipsec_conf if args.ipsec_conf else env["IPSEC_CONF"]
+ipsec_d = args.ipsec_d if args.ipsec_d else env["IPSEC_NSSDIR"]
ipsec_secrets = (args.ipsec_secrets if args.ipsec_secrets
else "/etc/ipsec.secrets")
ipsec_ctl = (args.ipsec_ctl if args.ipsec_ctl
-else "/run/pluto/pluto.ctl")
+else os.path.join(env["IPSEC_RUNDIR"], "pluto.ctl"))
-self.IPSEC = libreswan_root_prefix + "/usr/sbin/ipsec"
self.IPSEC_CONF = libreswan_root_prefix + ipsec_conf
self.IPSEC_SECRETS = libreswan_root_prefix + ipsec_secrets
self.IPSEC_D = "sql:" + libreswan_root_prefix + ipsec_d
--
2.39.3
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH OVN] ovn-nb: Add documentation for disable_arp_nd_rsp option
On Fri, Feb 16, 2024 at 10:54 PM Naveen Yerramneni
wrote:
>
> Signed-off-by: Naveen Yerramneni
Thanks. Applied to main.
Numan
> ---
> ovn-nb.xml | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index e0b983ed6..b652046a7 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -1268,6 +1268,13 @@
>unknown ports connected to the same Logical Switch.
>
>
> + +type='{"type": "boolean"}'>
> + If set to true, ARP/ND responder flows are not
> installed
> + for the IP addresses configured on this logical port.
> + Default: false.
> +
> +
>
>
> If set, OVN will attempt to perform plugging of this VIF. In
> order
> --
> 2.36.6
>
> ___
> 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 3/3] netdev-dpdk: Fix tunnel type check during Tx offload preparation.
Tunnel types are not flags, but 4-bit fields, so checking them with
a simple binary 'and' is incorrect and may produce false-positive
matches.
While the current implementation is unlikely to cause any issues today,
since both RTE_MBUF_F_TX_TUNNEL_VXLAN and RTE_MBUF_F_TX_TUNNEL_GENEVE
only have 1 bit set, it is risky to have this code and it may lead
to problems if we add support for other tunnel types in the future.
Use proper field checks instead. Also adding a warning for unexpected
tunnel types in case something goes wrong.
Fixes: 084c8087292c ("userspace: Support VXLAN and GENEVE TSO.")
Signed-off-by: Ilya Maximets
---
lib/netdev-dpdk.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 1ae2ef398..29a6bf032 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2601,8 +2601,9 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev,
struct rte_mbuf *mbuf)
/* If packet is vxlan or geneve tunnel packet, calculate outer
* l2 len and outer l3 len. Inner l2/l3/l4 len are calculated
* before. */
-if (mbuf->ol_flags &
-(RTE_MBUF_F_TX_TUNNEL_GENEVE | RTE_MBUF_F_TX_TUNNEL_VXLAN)) {
+const uint64_t tunnel_type = mbuf->ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK;
+if (tunnel_type == RTE_MBUF_F_TX_TUNNEL_GENEVE ||
+tunnel_type == RTE_MBUF_F_TX_TUNNEL_VXLAN) {
mbuf->outer_l2_len = (char *) dp_packet_l3(pkt) -
(char *) dp_packet_eth(pkt);
mbuf->outer_l3_len = (char *) dp_packet_l4(pkt) -
@@ -2616,6 +2617,12 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev,
struct rte_mbuf *mbuf)
mbuf->ol_flags &= ~(RTE_MBUF_F_TX_IPV4 |
RTE_MBUF_F_TX_IPV6);
}
+} else if (OVS_UNLIKELY(tunnel_type)) {
+VLOG_WARN_RL(, "%s: Unexpected tunnel type: %#"PRIx64,
+ netdev_get_name(>up), tunnel_type);
+netdev_dpdk_mbuf_dump(netdev_get_name(>up),
+ "Packet with unexpected tunnel type", mbuf);
+return false;
} else {
mbuf->l2_len = (char *) dp_packet_l3(pkt) -
(char *) dp_packet_eth(pkt);
@@ -2641,8 +2648,7 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev,
struct rte_mbuf *mbuf)
return false;
}
-if (mbuf->ol_flags & (RTE_MBUF_F_TX_TUNNEL_GENEVE |
-RTE_MBUF_F_TX_TUNNEL_VXLAN)) {
+if (tunnel_type) {
mbuf->tso_segsz = dev->mtu - mbuf->l2_len - mbuf->l3_len -
mbuf->l4_len - mbuf->outer_l3_len;
} else {
--
2.43.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 2/3] netdev-dpdk: Fix TCP check during Tx offload preparation.
RTE_MBUF_F_TX_TCP_CKSUM is not a flag, but a 2-bit field, so checking
it with a simple binary 'and' is incorrect. For example, this check
will succeed for a packet with UDP checksum requested as well.
Fix the check to avoid wrongly initializing tso_segz and potentially
accessing UDP header via TCP structure pointer.
The IPv4 checksum flag has to be set for any L4 checksum request,
regardless of the type, so moving this check out of the TCP condition.
Fixes: 8b5fe2dc6080 ("userspace: Add Generic Segmentation Offloading.")
Signed-off-by: Ilya Maximets
---
lib/netdev-dpdk.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 270d3e11c..1ae2ef398 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2634,7 +2634,7 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev,
struct rte_mbuf *mbuf)
}
}
-if (mbuf->ol_flags & RTE_MBUF_F_TX_TCP_CKSUM) {
+if ((mbuf->ol_flags & RTE_MBUF_F_TX_L4_MASK) == RTE_MBUF_F_TX_TCP_CKSUM) {
if (!th) {
VLOG_WARN_RL(, "%s: TCP offloading without L4 header"
" pkt len: %"PRIu32"", dev->up.name, mbuf->pkt_len);
@@ -2661,11 +2661,14 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev,
struct rte_mbuf *mbuf)
return false;
}
}
+}
-if (mbuf->ol_flags & RTE_MBUF_F_TX_IPV4) {
-mbuf->ol_flags |= RTE_MBUF_F_TX_IP_CKSUM;
-}
+/* If L4 checksum is requested, IPv4 should be requested as well. */
+if (mbuf->ol_flags & RTE_MBUF_F_TX_L4_MASK
+&& mbuf->ol_flags & RTE_MBUF_F_TX_IPV4) {
+mbuf->ol_flags |= RTE_MBUF_F_TX_IP_CKSUM;
}
+
return true;
}
--
2.43.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 0/3] netdev-dpdk: More Tx offlaod fixes.
Stuff discovered while working on: https://github.com/openvswitch/ovs-issues/issues/321 Ilya Maximets (3): netdev-dpdk: Clear inner packet marks if no inner offloads requested. netdev-dpdk: Fix TCP check during Tx offload preparation. netdev-dpdk: Fix tunnel type check during Tx offload preparation. lib/netdev-dpdk.c | 34 ++ 1 file changed, 26 insertions(+), 8 deletions(-) -- 2.43.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 1/3] netdev-dpdk: Clear inner packet marks if no inner offloads requested.
In some cases only outer offloads may be requested for a tunneled
packet. In this case there is no need to mark the type of an
inner packet. Clean these flags up to avoid potential confusion
of DPDK drivers.
Fixes: 084c8087292c ("userspace: Support VXLAN and GENEVE TSO.")
Signed-off-by: Ilya Maximets
---
lib/netdev-dpdk.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 8c52accff..270d3e11c 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2607,6 +2607,15 @@ netdev_dpdk_prep_hwol_packet(struct netdev_dpdk *dev,
struct rte_mbuf *mbuf)
(char *) dp_packet_eth(pkt);
mbuf->outer_l3_len = (char *) dp_packet_l4(pkt) -
(char *) dp_packet_l3(pkt);
+
+/* If neither inner checksums nor TSO is requested, inner marks
+ * should not be set. */
+if (!(mbuf->ol_flags & (RTE_MBUF_F_TX_IP_CKSUM |
+RTE_MBUF_F_TX_L4_MASK |
+RTE_MBUF_F_TX_TCP_SEG))) {
+mbuf->ol_flags &= ~(RTE_MBUF_F_TX_IPV4 |
+RTE_MBUF_F_TX_IPV6);
+}
} else {
mbuf->l2_len = (char *) dp_packet_l3(pkt) -
(char *) dp_packet_eth(pkt);
--
2.43.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [Patch ovn] docs: Remove ref. to "ovn-sbctl --no-wait".
Couple places in the documentation reference "--wait" and "--no-wait" options for "ovn-sbctl" but it doesn't support these options [0]. Trying, for example, "ovn-sbctl --no-wait init" exits with: ovn-sbctl: --no-wait not supported [0] https://github.com/ovn-org/ovn/blob/63b35e2f6789f7843363c8f7a92430402bf989f9/utilities/ovn-sbctl.c#L127 Signed-off-by: Martin Kalcok --- Documentation/intro/install/general.rst | 2 +- utilities/ovn-sbctl.8.xml | 8 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Documentation/intro/install/general.rst b/Documentation/intro/install/general.rst index ab6209482..6efb3a701 100644 --- a/Documentation/intro/install/general.rst +++ b/Documentation/intro/install/general.rst @@ -428,7 +428,7 @@ the first time after you create the databases with ovsdb-tool, though running it at any time is harmless:: $ ovn-nbctl --no-wait init -$ ovn-sbctl --no-wait init +$ ovn-sbctl init Start ``ovn-northd``, telling it to connect to the OVN db servers same Unix domain socket:: diff --git a/utilities/ovn-sbctl.8.xml b/utilities/ovn-sbctl.8.xml index 81ab4131d..32035d051 100644 --- a/utilities/ovn-sbctl.8.xml +++ b/utilities/ovn-sbctl.8.xml @@ -123,10 +123,10 @@ Instructs the daemon process to run one or more ovn-sbctl commands described above and reply with the results of running these -commands. Accepts the --no-wait, --wait, ---timeout, --dry-run, --oneline, -and the options described under Table Formatting Options -in addition to the the command-specific options. +commands. Accepts the --timeout, --dry-run, +--oneline, and the options described under +Table Formatting Options in addition to the the +command-specific options. exit -- 2.40.1 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] netdev-dpdk: Clean up all marker flags if no offloads requested.
On 3/12/24 14:36, Mike Pattrick wrote:
> On Mon, Mar 11, 2024 at 2:31 PM Ilya Maximets wrote:
>>
>> Some drivers (primarily, Intel ones) do not expect any marking flags
>> being set if no offloads are requested. If these flags are present,
>> driver will fail Tx preparation or behave abnormally.
>>
>> For example, ixgbe driver will refuse to process the packet with
>> only RTE_MBUF_F_TX_TUNNEL_GENEVE and RTE_MBUF_F_TX_OUTER_IPV4 set.
>> This pretty much breaks Geneve tunnels on these cards.
>>
>> An extra check is added to make sure we don't have any unexpected
>> Tx offload flags set.
>>
>> Fixes: 084c8087292c ("userspace: Support VXLAN and GENEVE TSO.")
>> Reported-at: https://github.com/openvswitch/ovs-issues/issues/321
>> Signed-off-by: Ilya Maximets
>
> Acked-by: Mike Pattrick
>
Thanks! Applied and backported to 3.3.
Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] conntrack: Fix SNAT with exhaustion system test.
Recent kernels introduced a mechanism that allows to evict colliding entries in a closing state whereas they were previously considered as parts of a non-recoverable clash. This new behavior makes "conntrack - SNAT with port range with exhaustion test" fail, as it relies on the previous assumptions. Fix it by creating and not advancing the first entry in SYN_SENT to avoid early eviction. Suggested-by: Ilya Maximets Reported-at: https://issues.redhat.com/browse/FDP-486 Signed-off-by: Paolo Valerio --- tests/system-traffic.at | 21 ++--- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 2d12d558e..04559f5e8 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -6388,7 +6388,6 @@ OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP AT_SETUP([conntrack - SNAT with port range with exhaustion]) -OVS_CHECK_GITHUB_ACTION() CHECK_CONNTRACK() CHECK_CONNTRACK_NAT() OVS_TRAFFIC_VSWITCHD_START() @@ -6398,11 +6397,11 @@ ADD_NAMESPACES(at_ns0, at_ns1) ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") NS_CHECK_EXEC([at_ns0], [ip link set dev p0 address 80:88:88:88:88:88]) ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24") +NS_CHECK_EXEC([at_ns1], [ip link set dev p1 address 80:89:89:89:89:89]) dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic from ns1->ns0. AT_DATA([flows.txt], [dnl -in_port=1,tcp,action=ct(commit,zone=1,nat(src=10.1.1.240:34568,random)),2 -in_port=2,ct_state=-trk,tcp,tp_dst=34567,action=ct(table=0,zone=1,nat) +in_port=1,tcp,action=ct(commit,zone=1,nat(src=10.1.1.240:34568)),2 in_port=2,ct_state=-trk,tcp,tp_dst=34568,action=ct(table=0,zone=1,nat) in_port=2,ct_state=+trk,ct_zone=1,tcp,action=1 dnl @@ -6426,17 +6425,25 @@ AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) dnl HTTP requests from p0->p1 should work fine. OVS_START_L7([at_ns1], [http]) -NS_CHECK_EXEC([at_ns0], [wget 10.1.1.2 -t 1 -T 1 --retry-connrefused -v -o wget0.log]) + +dnl Send a valid SYN to make conntrack pick it up. +dnl The source port used is 123 to prevent unwanted reuse in the next HTTP request. +AT_CHECK([ovs-ofctl packet-out br0 "packet=8089898989898088080045280001400664cb0a0101010a010102007b0050500220007913 actions=ct(commit,zone=1,nat(src=10.1.1.240:34568))"]) + +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | uniq], [0], [dnl +tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=10.1.1.2,dst=10.1.1.240,sport=,dport=),zone=1,protoinfo=(state=) +]) NS_CHECK_EXEC([at_ns0], [wget 10.1.1.2 -t 1 -T 1 --retry-connrefused -v -o wget0.log], [4]) -AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | sed -e 's/dst=10.1.1.2[[45]][[0-9]]/dst=10.1.1.2XX/' | uniq], [0], [dnl -tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=10.1.1.2,dst=10.1.1.2XX,sport=,dport=),zone=1,protoinfo=(state=) +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2) | uniq], [0], [dnl +tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=,dport=),reply=(src=10.1.1.2,dst=10.1.1.240,sport=,dport=),zone=1,protoinfo=(state=) ]) OVS_TRAFFIC_VSWITCHD_STOP(["dnl /Unable to NAT due to tuple space exhaustion - if DoS attack, use firewalling and\/or zone partitioning./d -/Dropped .* log messages in last .* seconds \(most recently, .* seconds ago\) due to excessive rate/d"]) +/Dropped .* log messages in last .* seconds \(most recently, .* seconds ago\) due to excessive rate/d +/|WARN|.* execute ct.* failed/d"]) AT_CLEANUP AT_SETUP([conntrack - more complex SNAT]) -- 2.44.0 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 12/12] documentation: Document ovs-flowviz.
Bleep bloop. Greetings Adrian Moreno, I am a robot and I have tried out your
patch.
Thanks for your contribution.
I encountered some error that I wasn't expecting. See the details below.
checkpatch:
WARNING: Line has trailing whitespace
#206 FILE: Documentation/ref/ovs-flowviz.8.rst:126:
- json
WARNING: Line is 81 characters long (recommended limit is 79)
#544 FILE: Documentation/ref/ovs-flowviz.8.rst:464:
[! | not ] {key}[[.subkey]...] [OPERATOR] {value})] [LOGICAL OPERATOR]
...
WARNING: Line is 80 characters long (recommended limit is 79)
#558 FILE: Documentation/ref/ovs-flowviz.8.rst:478:
To compare against a match or info field, use the field directly, e.g:
WARNING: Line is 80 characters long (recommended limit is 79)
#565 FILE: Documentation/ref/ovs-flowviz.8.rst:485:
Actions values might be dictionaries, use subkeys to access individual
WARNING: Line is 110 characters long (recommended limit is 79)
#601 FILE: Documentation/ref/ovs-flowviz.8.rst:521:
$ ovs-flowviz -i flows.txt --style "light" --highlight "n_packets > 0 and
drop" openflow html > flows.html
WARNING: Line is 141 characters long (recommended limit is 79)
#697 FILE: Documentation/topics/flow-visualization.rst:80:
cookie=0xf76b4b20, duration=765.107s, table=0, n_packets=0, n_bytes=0,
priority=180,vlan_tci=0x/0x1000 actions=conjunction(100,2/2)
WARNING: Line is 328 characters long (recommended limit is 79)
#698 FILE: Documentation/topics/flow-visualization.rst:81:
cookie=0xf76b4b20, duration=765.107s, table=0, n_packets=0, n_bytes=0,
priority=180,conj_id=100,in_port="patch-br-int-to",vlan_tci=0x/0x1000
actions=load:0xa->NXM_NX_REG13[],load:0xc->NXM_NX_REG11[],load:0xb->NXM_NX_REG12[],load:0xb->OXM_OF_METADATA[],load:0x1->NXM_NX_REG14[],mod_dl_src:02:42:ac:12:00:03,resubmit(,8)
WARNING: Line is 286 characters long (recommended limit is 79)
#699 FILE: Documentation/topics/flow-visualization.rst:82:
cookie=0x0, duration=765.388s, table=0, n_packets=0, n_bytes=0,
priority=100,in_port="ovn-6bb3b3-0"
actions=move:NXM_NX_TUN_ID[0..23]->OXM_OF_METADATA[0..23],move:NXM_NX_TUN_METADATA0[16..30]->NXM_NX_REG14[0..14],move:NXM_NX_TUN_METADATA0[0..15]->NXM_NX_REG15[0..15],resubmit(,40)
WARNING: Line is 286 characters long (recommended limit is 79)
#700 FILE: Documentation/topics/flow-visualization.rst:83:
cookie=0x0, duration=765.388s, table=0, n_packets=0, n_bytes=0,
priority=100,in_port="ovn-a6ff98-0"
actions=move:NXM_NX_TUN_ID[0..23]->OXM_OF_METADATA[0..23],move:NXM_NX_TUN_METADATA0[16..30]->NXM_NX_REG14[0..14],move:NXM_NX_TUN_METADATA0[0..15]->NXM_NX_REG15[0..15],resubmit(,40)
WARNING: Line is 262 characters long (recommended limit is 79)
#701 FILE: Documentation/topics/flow-visualization.rst:84:
cookie=0xf2ca6195, duration=765.107s, table=0, n_packets=6, n_bytes=636,
priority=100,in_port="ovn-k8s-mp0"
actions=load:0x1->NXM_NX_REG13[],load:0x2->NXM_NX_REG11[],load:0x7->NXM_NX_REG12[],load:0x4->OXM_OF_METADATA[],load:0x2->NXM_NX_REG14[],resubmit(,8)
WARNING: Line is 266 characters long (recommended limit is 79)
#702 FILE: Documentation/topics/flow-visualization.rst:85:
cookie=0x236e941d, duration=408.874s, table=0, n_packets=11, n_bytes=846,
priority=100,in_port=aceac9829941d11
actions=load:0x11->NXM_NX_REG13[],load:0x2->NXM_NX_REG11[],load:0x7->NXM_NX_REG12[],load:0x4->OXM_OF_METADATA[],load:0x3->NXM_NX_REG14[],resubmit(,8)
WARNING: Line is 268 characters long (recommended limit is 79)
#703 FILE: Documentation/topics/flow-visualization.rst:86:
cookie=0x3facf689, duration=405.581s, table=0, n_packets=11, n_bytes=846,
priority=100,in_port="363ba22029cd92b"
actions=load:0x12->NXM_NX_REG13[],load:0x2->NXM_NX_REG11[],load:0x7->NXM_NX_REG12[],load:0x4->OXM_OF_METADATA[],load:0x4->NXM_NX_REG14[],resubmit(,8)
WARNING: Line is 268 characters long (recommended limit is 79)
#704 FILE: Documentation/topics/flow-visualization.rst:87:
cookie=0xe7c8c4bb, duration=405.570s, table=0, n_packets=11, n_bytes=846,
priority=100,in_port="6a62cde0d50ef44"
actions=load:0x13->NXM_NX_REG13[],load:0x2->NXM_NX_REG11[],load:0x7->NXM_NX_REG12[],load:0x4->OXM_OF_METADATA[],load:0x5->NXM_NX_REG14[],resubmit(,8)
WARNING: Line is 266 characters long (recommended limit is 79)
#705 FILE: Documentation/topics/flow-visualization.rst:88:
cookie=0x99a0ffc1, duration=59.391s, table=0, n_packets=8, n_bytes=636,
priority=100,in_port="5ff3bfaaa4eb622"
actions=load:0x14->NXM_NX_REG13[],load:0x2->NXM_NX_REG11[],load:0x7->NXM_NX_REG12[],load:0x4->OXM_OF_METADATA[],load:0x6->NXM_NX_REG14[],resubmit(,8)
WARNING: Line is 266 characters long (recommended limit is 79)
#706 FILE: Documentation/topics/flow-visualization.rst:89:
cookie=0xe1b5c263, duration=59.365s, table=0, n_packets=8, n_bytes=636,
priority=100,in_port="8d9e0bc76347e59"
Re: [ovs-dev] [Patch ovn v2 1/2] actions: Enable specifying zone for ct_commit.
On Tue, Mar 12, 2024 at 9:17 PM Martin Kalcok
wrote:
> Following up on the comments from v1.
>
> @amusil You were right that the struct in actions.h was not necessary
> then. However I also noticed that I forgot to modify `format_CT_COMMIT_V1`
> function and for that I think the struct is necessary. I need to
> distinguish whether the `ct_commit` action was called with dnat, snat, or
> without any argument to format it properly. If you still don't like it, I
> can try to figure out how to do it without the struct, but I couldn't
> figure out an obvious solution, so I left it in there in this version.
>
> Regarding the action formatting unit tests, I have two
> assumptions/questions:
> 1. There's no way to distinguish router/switch datapaths in these tests. I
> saw that both `ct_commit_nat(dnat)` and `ct_commit_nat(snat)` [0] expect to
> encode into the same zone, although they'd output different zones if they
> were used in LR datapath.
> 2. When action formats into identical string as was its input (e.g.
> "ct_commit(snat)" -> "ct_commit(snat)"), the test should not use "format
> as" check, otherwise it fails. (This one took me a while to figure out, as
> it was not obvious from the testlog why it was failing)
>
> Are these correct?
>
> @numans I though a lot about your suggestions for different action names,
> but I think that current "ct_commit(snat/dnat)" fits semantically the most.
> Brand new actions would share pretty much all of the code with current
> "ct_commit_v1" handling. So to address your comments regarding the backward
> compatibility, I added new feature flag, following Ales' approach in [1]. I
> believe that this should avoid problems of backward incompatibility in
> cases when northd is upgraded but controller is not.
>
>
Sorry to re-iterate on this @numans, I just hope I didn't misunderstood
your original comments on the v1. The way I took it is that you are OK with
using `ct_commit(dnat/snat)` and repurposing the implementation of
`ct_commit_v1`, as long as it doesn't break backwards compatibility. Or do
you think completely new action names with separate implementations are
still needed? I just don't wan't to give impression that I'm ignoring your
suggestions from v1.
> @0-day Robot: I forgot to run checkpatch.py, my bad. However the only
> problem is 81 char line in ovn-sb.xml and there are already many lines that
> go over this limit. Should I create v3 if this turns out to be the only
> modification needed?
>
> [0]
> https://github.com/ovn-org/ovn/blob/b92ad9e0b408a202273d69ba573f2538e53c6e48/tests/ovn.at#L1500-L1511
> [1]
> https://github.com/ovn-org/ovn/commit/43f741c2f029a68a11436e5b14c5bbda6e207dd3#diff-ca917e7415d06776f8ee2baf6102a866c5c31f998e4df93ff8eaa246b65e1da2
>
> On Tue, Mar 12, 2024 at 8:45 PM Martin Kalcok
> wrote:
>
>> Action `ct_commit` currently does not allow specifying zone into
>> which connection is committed. For example, in LR datapath, the
>> `ct_commit`
>> will always use the DNAT zone.
>>
>> This change adds option to use `ct_commit(snat)` or `ct_commit(dnat)` to
>> explicitly specify the zone into which the connection will be committed.
>> It also comes with new feature flag OVN_FEATURE_CT_COMMIT_TO_ZONE to avoid
>> incompatibility between northd and controller in case when controller does
>> not suport these actions.
>>
>> Original behavior of `ct_commit` without the arguments remains unchanged.
>>
>> Signed-off-by: Martin Kalcok
>> ---
>> controller/chassis.c | 8
>> include/ovn/actions.h | 9 +
>> include/ovn/features.h| 1 +
>> lib/actions.c | 29 -
>> northd/en-global-config.c | 10 ++
>> northd/en-global-config.h | 1 +
>> ovn-sb.xml| 10 ++
>> tests/ovn.at | 7 +++
>> 8 files changed, 74 insertions(+), 1 deletion(-)
>>
>> diff --git a/controller/chassis.c b/controller/chassis.c
>> index ad75df288..9bb2eba95 100644
>> --- a/controller/chassis.c
>> +++ b/controller/chassis.c
>> @@ -371,6 +371,7 @@ chassis_build_other_config(const struct
>> ovs_chassis_cfg *ovs_cfg,
>> smap_replace(config, OVN_FEATURE_FDB_TIMESTAMP, "true");
>> smap_replace(config, OVN_FEATURE_LS_DPG_COLUMN, "true");
>> smap_replace(config, OVN_FEATURE_CT_COMMIT_NAT_V2, "true");
>> +smap_replace(config, OVN_FEATURE_CT_COMMIT_TO_ZONE, "true");
>> }
>>
>> /*
>> @@ -516,6 +517,12 @@ chassis_other_config_changed(const struct
>> ovs_chassis_cfg *ovs_cfg,
>> return true;
>> }
>>
>> +if (!smap_get_bool(_rec->other_config,
>> + OVN_FEATURE_CT_COMMIT_TO_ZONE,
>> + false)) {
>> +return true;
>> +}
>> +
>> return false;
>> }
>>
>> @@ -648,6 +655,7 @@ update_supported_sset(struct sset *supported)
>> sset_add(supported, OVN_FEATURE_FDB_TIMESTAMP);
>> sset_add(supported, OVN_FEATURE_LS_DPG_COLUMN);
>> sset_add(supported, OVN_FEATURE_CT_COMMIT_NAT_V2);
>>
Re: [ovs-dev] [Patch ovn v2 2/2] northd: Fix direct access to SNAT network on DR.
Regarding the failed unstable test in the CI, I suspect that this is not
related to the code change, I've had couple successful CI runs in my branch
[0].
Martin.
[0] https://github.com/mkalcok/ovn/actions/runs/8256539983
On Tue, Mar 12, 2024 at 8:45 PM Martin Kalcok
wrote:
> This change fixes bug that breaks ability of machines from external
> networks to communicate with machines in SNATed networks (specifically
> when using a Distributed router).
>
> Currently when a machine (S1) on an external network tries to talk
> over TCP with a machine (A1) in a network that has enabled SNAT, the
> connection is established successfully. However after the three-way
> handshake, any packets that come from the A1 machine will have their
> source address translated by the Distributed router, breaking the
> communication.
>
> Existing rule in `build_lrouter_out_snat_flow` that decides which
> packets should be SNATed already tries to avoid SNATing packets in
> reply direction with `(!ct.trk || !ct.rpl)`. However, previous stages
> in the distributed LR egress pipeline do not initiate the CT state.
>
> Additionally we need to commit new connections that originate from
> external networks into CT, so that the packets in the reply direction
> can be properly identified.
>
> Rationale:
>
> In my original RFC [0], there were questions about the motivation for
> fixing this issue. I'll try to summarize why I think this is a bug
> that should be fixed.
>
> 1. Current implementation for Distributed router already tries to
>avoid SNATing packets in the reply direction, it's just missing
>initialized CT states to make proper decisions.
>
> 2. This same scenario works with Gateway Router. I tested with
>following setup:
>
> foo -- R1 -- join - R3 -- alice
> |
> bar --R2
>
> R1 is a Distributed router with SNAT for foo. R2 is a Gateway
> router with SNAT for bar. R3 is a Gateway router with no SNAT.
> Using 'alice1' as a client I was able to talk over TCP with
> 'bar1' but connection with 'foo1' failed.
>
> 3. Regarding security and "leaking" of internal IPs. Reading through
>RFC 4787 [1], 5382 [2] and their update in 7857 [3], the
>specifications do not seem to mandate that SNAT implementations
>must filter incoming traffic destined directly to the internal
>network. Sections like "5. Filtering Behavior" in 4787 and
>"4.3 Externally Initiated Connections" in 5382 describe only
>behavior for traffic destined to external IP/Port associated
>with NAT on the device that performs NAT.
>
>Besides, with the current implementation, it's already possible
>to scan the internal network with pings and TCP syn scanning.
>
> 4. We do have customers/clouds that depend on this functionality.
>This is a scenario that used to work in Openstack with ML2/OVS
>and migrating those clouds to ML2/OVN would break it.
>
> [0]
> https://mail.openvswitch.org/pipermail/ovs-dev/2024-February/411670.html
> [1]https://datatracker.ietf.org/doc/html/rfc4787
> [2]https://datatracker.ietf.org/doc/html/rfc5382
> [3]https://datatracker.ietf.org/doc/html/rfc7857
>
> Signed-off-by: Martin Kalcok
> ---
> northd/northd.c | 68
> northd/ovn-northd.8.xml | 29 +
> tests/ovn-northd.at | 33
> tests/system-ovn.at | 69 +
> 4 files changed, 180 insertions(+), 19 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 2c3560ce2..25af52d5a 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -14438,20 +14438,27 @@ build_lrouter_out_is_dnat_local(struct
> lflow_table *lflows,
>
> static void
> build_lrouter_out_snat_match(struct lflow_table *lflows,
> - const struct ovn_datapath *od,
> - const struct nbrec_nat *nat, struct ds
> *match,
> - bool distributed_nat, int cidr_bits, bool
> is_v6,
> - struct ovn_port *l3dgw_port,
> - struct lflow_ref *lflow_ref)
> + const struct ovn_datapath *od,
> + const struct nbrec_nat *nat,
> + struct ds *match,
> + bool distributed_nat, int
> cidr_bits,
> + bool is_v6,
> + struct ovn_port *l3dgw_port,
> + struct lflow_ref *lflow_ref,
> + bool is_reverse)
> {
> ds_clear(match);
>
> -ds_put_format(match, "ip && ip%c.src == %s", is_v6 ? '6' : '4',
> +ds_put_format(match, "ip && ip%c.%s == %s",
> + is_v6 ? '6' : '4',
> + is_reverse ? "dst" : "src",
>
[ovs-dev] [PATCH v2 12/12] documentation: Document ovs-flowviz.
Add a man page for ovs-flowviz as well as a topic page with some more
detailed examples.
Signed-off-by: Adrian Moreno
---
Documentation/automake.mk | 4 +-
Documentation/conf.py | 2 +
Documentation/ref/index.rst | 1 +
Documentation/ref/ovs-flowviz.8.rst | 531
Documentation/topics/flow-visualization.rst | 271 ++
Documentation/topics/index.rst | 1 +
6 files changed, 809 insertions(+), 1 deletion(-)
create mode 100644 Documentation/ref/ovs-flowviz.8.rst
create mode 100644 Documentation/topics/flow-visualization.rst
diff --git a/Documentation/automake.mk b/Documentation/automake.mk
index 47d2e336a..539870aa2 100644
--- a/Documentation/automake.mk
+++ b/Documentation/automake.mk
@@ -45,7 +45,7 @@ DOC_SOURCE = \
Documentation/topics/fuzzing/ovs-fuzzing-infrastructure.rst \
Documentation/topics/fuzzing/ovs-fuzzers.rst \
Documentation/topics/fuzzing/security-analysis-of-ovs-fuzzers.rst \
- Documentation/topics/testing.rst \
+ Documentation/topics/flow-visualization.rst \
Documentation/topics/integration.rst \
Documentation/topics/language-bindings.rst \
Documentation/topics/networking-namespaces.rst \
@@ -55,6 +55,7 @@ DOC_SOURCE = \
Documentation/topics/ovsdb-replication.rst \
Documentation/topics/porting.rst \
Documentation/topics/record-replay.rst \
+ Documentation/topics/testing.rst \
Documentation/topics/tracing.rst \
Documentation/topics/usdt-probes.rst \
Documentation/topics/userspace-checksum-offloading.rst \
@@ -162,6 +163,7 @@ RST_MANPAGES = \
ovs-actions.7.rst \
ovs-appctl.8.rst \
ovs-ctl.8.rst \
+ ovs-flowviz.8.rst \
ovs-l3ping.8.rst \
ovs-parse-backtrace.8.rst \
ovs-pki.8.rst \
diff --git a/Documentation/conf.py b/Documentation/conf.py
index 085ca2cd6..e41cf6031 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -120,6 +120,8 @@ _man_pages = [
u'utility for configuring running Open vSwitch daemons'),
('ovs-ctl.8',
u'OVS startup helper script'),
+('ovs-flowviz.8',
+ u'utility for visualizing OpenFlow and datapath flows'),
('ovs-l3ping.8',
u'check network deployment for L3 tunneling problems'),
('ovs-parse-backtrace.8',
diff --git a/Documentation/ref/index.rst b/Documentation/ref/index.rst
index 03ada932f..7f2fe6177 100644
--- a/Documentation/ref/index.rst
+++ b/Documentation/ref/index.rst
@@ -42,6 +42,7 @@ time:
ovs-actions.7
ovs-appctl.8
ovs-ctl.8
+ ovs-flowviz.8
ovs-l3ping.8
ovs-pki.8
ovs-sim.1
diff --git a/Documentation/ref/ovs-flowviz.8.rst
b/Documentation/ref/ovs-flowviz.8.rst
new file mode 100644
index 0..da1135918
--- /dev/null
+++ b/Documentation/ref/ovs-flowviz.8.rst
@@ -0,0 +1,531 @@
+..
+ Licensed under the Apache License, Version 2.0 (the "License"); you may
+ not use this file except in compliance with the License. You may obtain
+ a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ License for the specific language governing permissions and limitations
+ under the License.
+
+ Convention for heading levels in Open vSwitch documentation:
+
+ === Heading 0 (reserved for the title in a document)
+ --- Heading 1
+ ~~~ Heading 2
+ +++ Heading 3
+ ''' Heading 4
+
+ Avoid deeper levels because they do not render well.
+
+===
+ovs-flowviz
+===
+
+Synopsis
+
+
+``ovs-flowviz``
+[``[-i | --input] <[alias,]file>``]
+[``[-c | --config] ``]
+[``[-f | --filter] ``]
+[``[-h | --highlight] ``]
+[``--style
[ovs-dev] [PATCH v2 11/12] python: ovs: flowviz: Support html dark style.
In order to support dark style in html outputs, allow the config file to
express the background color and set it in a top style object.
Acked-by: Eelco Chaudron
Signed-off-by: Adrian Moreno
---
python/ovs/flowviz/html_format.py | 4 +++-
python/ovs/flowviz/odp/html.py | 30 -
python/ovs/flowviz/ofp/html.py | 28 ++-
python/ovs/flowviz/ovs-flowviz.conf | 20 +++
4 files changed, 71 insertions(+), 11 deletions(-)
diff --git a/python/ovs/flowviz/html_format.py
b/python/ovs/flowviz/html_format.py
index ebfa65c34..3f3550da5 100644
--- a/python/ovs/flowviz/html_format.py
+++ b/python/ovs/flowviz/html_format.py
@@ -48,7 +48,9 @@ class HTMLBuffer(FlowBuffer):
style = ' style="color:{}"'.format(color) if color else ""
self._text += "".format(style)
if href:
-self._text += "".format(href)
+self._text += " ".format(
+href, 'style="color:{}"'.format(color) if color else ""
+)
self._text += string
if href:
self._text += ""
diff --git a/python/ovs/flowviz/odp/html.py b/python/ovs/flowviz/odp/html.py
index 4aa08dc70..b2855bf40 100644
--- a/python/ovs/flowviz/odp/html.py
+++ b/python/ovs/flowviz/odp/html.py
@@ -55,10 +55,18 @@ class HTMLTree(FlowTree):
flows(dict[int, list[DPFlow]): Optional; initial flows
"""
+html_body_style = """
+
+body {{
+background-color: {bg};
+color: {fg};
+}}
+"""
+
html_header = """
.flow{
-background-color:white;
+background-color:inherit;
display: inline-block;
text-align: left;
font-family: monospace;
@@ -177,9 +185,9 @@ class HTMLTree(FlowTree):
append()
"""
-def __init__(self, parent_name, flow=None, opts=None):
+def __init__(self, parent_name, flow=None, fmt=None, opts=None):
self._parent_name = parent_name
-self._formatter = HTMLFormatter(opts)
+self._formatter = fmt
self._opts = opts
super(HTMLTree.HTMLTreeElem, self).__init__(flow)
@@ -232,13 +240,14 @@ class HTMLTree(FlowTree):
def __init__(self, name, opts, flows=None):
self.opts = opts
self.name = name
+self._formatter = HTMLFormatter(opts)
super(HTMLTree, self).__init__(
flows, self.HTMLTreeElem("", flow=None, opts=self.opts)
)
def _new_elem(self, flow, _):
"""Override _new_elem to provide HTMLTreeElems."""
-return self.HTMLTreeElem(self.name, flow, self.opts)
+return self.HTMLTreeElem(self.name, flow, self._formatter, self.opts)
def render(self):
"""Render the Tree in HTML.
@@ -247,10 +256,21 @@ class HTMLTree(FlowTree):
an html string representing the element
"""
name = self.name.replace(" ", "_")
+bg = (
+self._formatter.style.get("background").color
+if self._formatter.style.get("background")
+else "white"
+)
+fg = (
+self._formatter.style.get("default").color
+if self._formatter.style.get("default")
+else "black"
+)
html_text = """
""" # noqa: E501
-html_obj = self.html_header + html_text.format(name=name)
+html_obj = self.html_body_style.format(bg=bg, fg=fg)
+html_obj += self.html_header + html_text.format(name=name)
html_obj += "".format(name=name)
(html_elem, _) = self.root.render()
diff --git a/python/ovs/flowviz/ofp/html.py b/python/ovs/flowviz/ofp/html.py
index a66f5fe8e..b00ca58f0 100644
--- a/python/ovs/flowviz/ofp/html.py
+++ b/python/ovs/flowviz/ofp/html.py
@@ -24,6 +24,7 @@ class HTMLProcessor(OpenFlowFactory, FileProcessor):
def __init__(self, opts):
super().__init__(opts)
+self.formatter = HTMLFormatter(self.opts)
self.data = dict()
def start_file(self, name, filename):
@@ -39,21 +40,38 @@ class HTMLProcessor(OpenFlowFactory, FileProcessor):
self.tables[table].append(flow)
def html(self):
-html_obj = ""
+bg = (
+self.formatter.style.get("background").color
+if self.formatter.style.get("background")
+else "white"
+)
+fg = (
+self.formatter.style.get("default").color
+if self.formatter.style.get("default")
+else "black"
+)
+html_obj = """
+