[ovs-dev] [PATCH v2] conntrack: support default timeout policy get/set cmd for netdev datapath
From: wenxu
Now, the default timeout policy for netdev datapath is hard codeing. In
some case show or modify is needed.
Add command for get/set default timeout policy. Using like this:
ovs-appctl dpctl/ct-get-default-timeout-policy [dp]
ovs-appctl dpctl/ct-set-default-timeout-policy [dp] policies
Signed-off-by: wenxu
---
NEWS| 4 +++
lib/conntrack-tp.c | 9 ++
lib/conntrack-tp.h | 2 ++
lib/ct-dpif.c | 63 +
lib/ct-dpif.h | 8 +
lib/dpctl.c | 84 +
tests/system-traffic.at | 6
7 files changed, 176 insertions(+)
diff --git a/NEWS b/NEWS
index 434ee57..c6a2eda 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,10 @@ Post-v2.16.0
- ovs-dpctl and 'ovs-appctl dpctl/':
* New commands 'cache-get-size' and 'cache-set-size' that allows to
get or configure linux kernel datapath cache sizes.
+ - ovs-appctl dpctl/:
+ * New commands 'ct-set-default-timeout-policy' and
+ 'ct-set-default-timeout-policy' that allows to get or configure
+ netdev datapath ct default timeout policy.
v2.16.0 - 16 Aug 2021
diff --git a/lib/conntrack-tp.c b/lib/conntrack-tp.c
index a586d3a..726b854 100644
--- a/lib/conntrack-tp.c
+++ b/lib/conntrack-tp.c
@@ -230,6 +230,15 @@ tm_to_ct_dpif_tp(enum ct_timeout tm)
return CT_DPIF_TP_ATTR_MAX;
}
+void
+timeout_policy_dump(const struct ct_dpif_timeout_policy *tp, struct ds *ds)
+{
+for (unsigned i = 0; i < N_CT_TM; i++) {
+ds_put_format(ds, "\n\t%s = %"PRIu32, ct_timeout_str[i],
+ tp->attrs[tm_to_ct_dpif_tp(i)]);
+}
+}
+
static void
conn_update_expiration__(struct conntrack *ct, struct conn *conn,
enum ct_timeout tm, long long now,
diff --git a/lib/conntrack-tp.h b/lib/conntrack-tp.h
index 4d411d1..bd22242 100644
--- a/lib/conntrack-tp.h
+++ b/lib/conntrack-tp.h
@@ -22,6 +22,8 @@ enum ct_timeout;
void timeout_policy_init(struct conntrack *ct);
int timeout_policy_update(struct conntrack *ct, struct timeout_policy *tp);
int timeout_policy_delete(struct conntrack *ct, uint32_t tp_id);
+void timeout_policy_dump(const struct ct_dpif_timeout_policy *tp,
+ struct ds *ds);
struct timeout_policy *timeout_policy_get(struct conntrack *ct, int32_t tp_id);
void conn_init_expiration(struct conntrack *ct, struct conn *conn,
enum ct_timeout tm, long long now);
diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
index cfc2315..6e36da2 100644
--- a/lib/ct-dpif.c
+++ b/lib/ct-dpif.c
@@ -20,6 +20,8 @@
#include
#include "ct-dpif.h"
+#include "conntrack-private.h"
+#include "conntrack-tp.h"
#include "openvswitch/ofp-parse.h"
#include "openvswitch/vlog.h"
@@ -180,6 +182,24 @@ ct_dpif_get_tcp_seq_chk(struct dpif *dpif, bool *enabled)
}
int
+ct_dpif_set_default_timeout_policy(struct dpif *dpif,
+ const struct ct_dpif_timeout_policy *tp)
+{
+return (dpif->dpif_class->ct_set_timeout_policy
+? dpif->dpif_class->ct_set_timeout_policy(dpif, tp)
+: EOPNOTSUPP);
+}
+
+int
+ct_dpif_get_default_timeout_policy(struct dpif *dpif,
+ struct ct_dpif_timeout_policy *tp)
+{
+return (dpif->dpif_class->ct_get_timeout_policy
+? dpif->dpif_class->ct_get_timeout_policy(dpif, DEFAULT_TP_ID, tp)
+: EOPNOTSUPP);
+}
+
+int
ct_dpif_set_limits(struct dpif *dpif, const uint32_t *default_limit,
const struct ovs_list *zone_limits)
{
@@ -710,6 +730,42 @@ ct_dpif_free_zone_limits(struct ovs_list *zone_limits)
}
}
+
+/* Parses a specification of a timeout policy from 's' into '*tp'
+ * . Returns true on success. Otherwise, returns false and
+ * and puts the error message in 'ds'. */
+bool
+ct_dpif_parse_timeout_policy_tuple(const char *s, struct ds *ds,
+ struct ct_dpif_timeout_policy *tp)
+{
+char *pos, *key, *value, *copy, *err;
+
+pos = copy = xstrdup(s);
+while (ofputil_parse_key_value(&pos, &key, &value)) {
+uint32_t tmp;
+
+if (!*value) {
+ds_put_format(ds, "field %s missing value", key);
+goto error;
+}
+
+err = str_to_u32(value, &tmp);
+if (err) {
+ free(err);
+ goto error_with_msg;
+}
+
+ct_dpif_set_timeout_policy_attr_by_name(tp, key, tmp);
+}
+free(copy);
+return true;
+
+error_with_msg:
+ds_put_format(ds, "failed to parse field %s", key);
+error:
+free(copy);
+return false;
+}
/* Parses a specification of a conntrack zone limit from 's' into '*pzone'
* and '*plimit'. Returns true on success. Otherwise, returns false and
* and puts the error message in 'ds'. */
@@ -792,6 +848,13 @@ static const char *const ct_dpif_tp_attr_string[] = {
#undef CT_DPIF_TP_ICMP_ATTR
};
+void
+ct_dpif_format_t
[ovs-dev] [PATCH v2 1/1] datapath-windows: Reset flow key after Ipv4 fragments are reassembled
Thank you for the fix! Applied on master and backported until branch-2.13. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] conntrack: support default timeout policy get/set cmd for netdev datapath
Bleep bloop. Greetings wenxu, 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:
mv tests/ovsdb-cluster-testsuite.tmp tests/ovsdb-cluster-testsuite
\
{ sed -n -e '/%AUTHORS%/q' -e p < ./debian/copyright.in; \
sed '34,/^$/d' ./AUTHORS.rst | \
sed -n -e '/^$/q' -e 's/^/ /p'; \
sed -e '34,/%AUTHORS%/d' ./debian/copyright.in; \
} > debian/copyright
(printf '\043 Generated automatically -- do not modify!-*-
buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.16.90,g') <
./rhel/openvswitch-dkms.spec.in > openvswitch-dkms.spec.tmp || exit 1; if cmp
-s openvswitch-dkms.spec.tmp rhel/openvswitch-dkms.spec; then touch
rhel/openvswitch-dkms.spec; rm openvswitch-dkms.spec.tmp; else mv
openvswitch-dkms.spec.tmp rhel/openvswitch-dkms.spec; fi
(printf '\043 Generated automatically -- do not modify!-*-
buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.16.90,g') <
./rhel/kmod-openvswitch-rhel6.spec.in > kmod-openvswitch-rhel6.spec.tmp || exit
1; if cmp -s kmod-openvswitch-rhel6.spec.tmp rhel/kmod-openvswitch-rhel6.spec;
then touch rhel/kmod-openvswitch-rhel6.spec; rm
kmod-openvswitch-rhel6.spec.tmp; else mv kmod-openvswitch-rhel6.spec.tmp
rhel/kmod-openvswitch-rhel6.spec; fi
(printf '\043 Generated automatically -- do not modify!-*-
buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.16.90,g') <
./rhel/openvswitch-kmod-fedora.spec.in > openvswitch-kmod-fedora.spec.tmp ||
exit 1; if cmp -s openvswitch-kmod-fedora.spec.tmp
rhel/openvswitch-kmod-fedora.spec; then touch
rhel/openvswitch-kmod-fedora.spec; rm openvswitch-kmod-fedora.spec.tmp; else mv
openvswitch-kmod-fedora.spec.tmp rhel/openvswitch-kmod-fedora.spec; fi
(printf '\043 Generated automatically -- do not modify!-*-
buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.16.90,g') <
./rhel/openvswitch.spec.in > openvswitch.spec.tmp || exit 1; if cmp -s
openvswitch.spec.tmp rhel/openvswitch.spec; then touch rhel/openvswitch.spec;
rm openvswitch.spec.tmp; else mv openvswitch.spec.tmp rhel/openvswitch.spec; fi
(printf '\043 Generated automatically -- do not modify!-*-
buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.16.90,g') <
./rhel/openvswitch-fedora.spec.in > openvswitch-fedora.spec.tmp || exit 1; if
cmp -s openvswitch-fedora.spec.tmp rhel/openvswitch-fedora.spec; then touch
rhel/openvswitch-fedora.spec; rm openvswitch-fedora.spec.tmp; else mv
openvswitch-fedora.spec.tmp rhel/openvswitch-fedora.spec; fi
(printf '\043 Generated automatically -- do not modify!-*-
buffer-read-only: t -*-\n' && sed -e 's,[@]VERSION[@],2.16.90,g') \
< ./xenserver/openvswitch-xen.spec.in > openvswitch-xen.spec.tmp ||
exit 1; \
if cmp -s openvswitch-xen.spec.tmp xenserver/openvswitch-xen.spec; then touch
xenserver/openvswitch-xen.spec; rm openvswitch-xen.spec.tmp; else mv
openvswitch-xen.spec.tmp xenserver/openvswitch-xen.spec; fi
make[3]: Entering directory
`/var/lib/jenkins/jobs/0day_robot_upstream_build_from_pw/workspace/datapath'
make[3]: Leaving directory
`/var/lib/jenkins/jobs/0day_robot_upstream_build_from_pw/workspace/datapath'
tests/system-traffic.at
See above for files that use tabs for indentation.
Please use spaces instead.
make[2]: *** [check-tabs] Error 1
make[2]: Leaving directory
`/var/lib/jenkins/jobs/0day_robot_upstream_build_from_pw/workspace'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory
`/var/lib/jenkins/jobs/0day_robot_upstream_build_from_pw/workspace'
make: *** [all] Error 2
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
Re: [ovs-dev] Fwd: [PATCH] build: check libibverbs deps before linking with dpdk
Hello,
On Sun, Nov 14, 2021 at 10:27 AM Harold Huang wrote:
> David Marchand 于2021年11月12日周五 下午7:01写道：
> > OVS requests ibverbs only if mlx drivers are present in DPDK build (+
> > option RTE_IBVERBS_LINK_DLOPEN iirc).
> >
> > For your issue, things that come to mind are an outdated build
> > directory, maybe following a pull from OVS sources, or maybe you had
> > ibverbs installed in the past on this system, and now it has been
> > removed.
> >
> > Did you compile OVS from a scratch/clean environment?
>
> Hi, David,
>I also started a clean centos 8.2 vm environment and init the vm
> environment with the following command:
> $dnf -y install 'dnf-command(config-manager)' && dnf config-manager
> --set-enabled PowerTools && dnf install -y gcc make numactl-devel
> meson autoconf automake libtool unbound unbound-devel
>
> I could also build dpdk and link ovs successfully. But after I
> installed libpcap-devel and rebuild dpdk, I could not link ovs
> anymore. And the error message is the same as what I posted before. I
> found libpcap.pc also has a dependency about libibverbs:
> #
> # pkg-config file for libpcap.
> #
> # These variables come from the configure script, so includedir and
> # libdir may be defined in terms of prefix and exec_prefix, so the
> # latter must be defined as well.
> #
> prefix="/usr"
> exec_prefix="/usr"
> includedir="/usr/include"
> libdir="/usr/lib64"
>
> Name: libpcap
> Description: Platform-independent network traffic capture library
> Version: 1.9.1
> Libs: -L${libdir} -lpcap
> Libs.private: -libverbs
> Cflags: -I${includedir}
I can reproduce the issue now.
As you describe, I installed libpcap-devel, rebuilt and installed DPDK, then:
[dpdk@centos8 build]$
PKG_CONFIG_PATH=/home/dpdk/dpdk/builddir/install/lib64/pkgconfig
../configure --with-dpdk
...
checking for struct tcf_t.firstuse... yes
checking whether dpdk is enabled... yes
checking for DPDK... yes
checking for faulty pkg-config version... no
checking for rte_config.h... yes
checking whether RTE_LIBRTE_VHOST_NUMA is declared... yes
checking for library containing get_mempolicy... -lnuma
checking whether RTE_EAL_NUMA_AWARE_HUGEPAGES is declared... yes
checking for library containing get_mempolicy... (cached) -lnuma
checking whether RTE_NET_PCAP is declared... yes
checking for library containing pcap_dump_close... -lpcap
checking whether RTE_NET_AF_XDP is declared... no
checking whether RTE_LIBRTE_VHOST_NUMA is declared... (cached) yes
checking whether RTE_NET_MLX5 is declared... no
checking whether RTE_NET_MLX4 is declared... no
checking for library containing dlopen... -ldl
checking whether linking with dpdk works... no
configure: error: Could not find DPDK library in default search path,
update PKG_CONFIG_PATH for pkg-config to find the .pc file in
non-standard location
(fwiw, linking against dpdk dynamically works).
The problem is either on dpdk side or (I would lean to) Centos
packaging, not OVS.
It can be reproduced with DPDK examples:
[dpdk@centos8 build]$ cd ~/dpdk/builddir/install/share/dpdk/examples/helloworld/
[dpdk@centos8 helloworld]$
PKG_CONFIG_PATH=/home/dpdk/dpdk/builddir/install/lib64/pkgconfig make
static
cc -O3 -I/home/dpdk/dpdk/builddir/install/include -include
rte_config.h -march=native -mno-avx512f -DALLOW_EXPERIMENTAL_API
main.c -o build/helloworld-static -Wl,--whole-archive
-L/home/dpdk/dpdk/builddir/install/lib64 -l:librte_common_cpt.a
-l:librte_common_dpaax.a -l:librte_common_iavf.a
-l:librte_common_octeontx.a -l:librte_common_octeontx2.a
-l:librte_common_sfc_efx.a -l:librte_bus_dpaa.a -l:librte_bus_fslmc.a
-l:librte_bus_ifpga.a -l:librte_bus_pci.a -l:librte_bus_vdev.a
-l:librte_bus_vmbus.a -l:librte_common_qat.a
-l:librte_mempool_bucket.a -l:librte_mempool_dpaa.a
-l:librte_mempool_dpaa2.a -l:librte_mempool_octeontx.a
-l:librte_mempool_octeontx2.a -l:librte_mempool_ring.a
-l:librte_mempool_stack.a -l:librte_net_af_packet.a
-l:librte_net_ark.a -l:librte_net_atlantic.a -l:librte_net_avp.a
-l:librte_net_axgbe.a -l:librte_net_bond.a -l:librte_net_bnx2x.a
-l:librte_net_bnxt.a -l:librte_net_cxgbe.a -l:librte_net_dpaa.a
-l:librte_net_dpaa2.a -l:librte_net_e1000.a -l:librte_net_ena.a
-l:librte_net_enetc.a -l:librte_net_enic.a -l:librte_net_failsafe.a
-l:librte_net_fm10k.a -l:librte_net_i40e.a -l:librte_net_hinic.a
-l:librte_net_hns3.a -l:librte_net_iavf.a -l:librte_net_ice.a
-l:librte_net_igc.a -l:librte_net_ixgbe.a -l:librte_net_kni.a
-l:librte_net_liquidio.a -l:librte_net_memif.a -l:librte_net_netvsc.a
-l:librte_net_nfp.a -l:librte_net_null.a -l:librte_net_octeontx.a
-l:librte_net_octeontx2.a -l:librte_net_pcap.a -l:librte_net_pfe.a
-l:librte_net_qede.a -l:librte_net_ring.a -l:librte_net_sfc.a
-l:librte_net_softnic.a -l:librte_net_tap.a -l:librte_net_thunderx.a
-l:librte_net_txgbe.a -l:librte_net_vdev_netvsc.a
-l:librte_net_vhost.a -l:librte_net_virtio.a -l:librte_net_vmxnet3.a
-l:librte_raw_dpaa2_cmdif.a -l:librte_raw_dpaa2_qdma.a
-l:librte_raw_ioat.a -l:librte_raw_ntb.a -l:librte_raw_octeontx2_d
Re: [ovs-dev] [PATCH v1] checkpatch: Detect "trojan source" attack
On Wed, Nov 10, 2021, at 15:31, Mike Pattrick wrote:
> On Wed, Nov 10, 2021 at 6:30 AM Gaëtan Rivet wrote:
>>
>> On Tue, Nov 2, 2021, at 19:43, Mike Pattrick wrote:
>> > Recently there has been a lot of press about the "trojan source" attack,
>> > where Unicode characters are used to obfuscate the true functionality of
>> > code. This attack didn't effect OVS, but adding the check here will help
>> > guard against it sneaking in later.
>> >
>> > Signed-off-by: Mike Pattrick
>>
>> Hi,
>>
>> What did you base the selection of characters to blacklist on?
>
> I believe this list was sourced from https://unicode.org/reports/tr9/
>
Sure, I'm just thinking about zero-width chars, that are used
to subtly introduce off-by-ones. The bidir check seems incomplete.
>> Reading issues open on other languages, I haven't found a good comprehensive
>> set of characters that would need to be blacklisted. I'm not sure it is a
>> sufficient
>> approach: getting creative and circumventing this kind of blacklist would be
>> a sport.
>>
>> Instead, shouldn't we take the reverse approach and whitelist single-byte
>> chars?
>> (warn on multi-byte unicode sequence). It would be sufficient for the vast
>> majority
>> of C sources (and scripts).
>
> I've been going back and forth on that idea. I'm afraid of making a
> change that seems exclusive to people with non-latin characters in
> their name. There are a few pre-canned lists of homoglyphs, maybe I
> could add those to the blacklist?
>
I understand, but the check should only execute on {.c,.h,.in} files, not on the
commit header itself.
If restricted to sources, I think no name would appear. Comments and doc are
written
in English.
--
Gaetan Rivet
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 1/2] stream-ssl: Fix handling of default ciphers/protocols
On Mon, Sep 13, 2021 at 4:23 AM Frode Nordahl
wrote:
>
> On Sat, Sep 11, 2021 at 10:23 PM Flavio Leitner wrote:
> >
> > On Fri, Sep 10, 2021 at 06:20:45PM +0200, Frode Nordahl wrote:
> > > On Thu, Sep 9, 2021 at 9:53 PM Flavio Leitner wrote:
> > > >
> > > >
> > > > Hi Frode,
> > > >
> > > > Thanks for your patch.
> > > > Please see my comments below.
> > >
> > > Flavio, thank you for taking the time to review.
> > >
> > > > On Wed, Aug 25, 2021 at 01:05:13PM +0200, Frode Nordahl wrote:
> > > > > Contrary to what is stated in the documentation, when SSL
> > > > > ciphers or protocols options are omitted the default values
> > > > > will not be set. The SSL library default settings will be used
> > > > > instead.
> > > > >
> > > > > Fix handling of default ciphers and protocols so that we actually
> > > > > enforce what is listed as defaults.
> > > > >
> > > > > Fixes: e18a1d086133 ("Add support for specifying SSL connection
> > > > > parameters to ovsdb")
> > > > > Signed-off-by: Frode Nordahl
> > > > > ---
> > > > > lib/stream-ssl.c | 30 ++
> > > > > 1 file changed, 22 insertions(+), 8 deletions(-)
> > > > >
> > > > > diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
> > > > > index 0ea3f2c08..6b4cf6970 100644
> > > > > --- a/lib/stream-ssl.c
> > > > > +++ b/lib/stream-ssl.c
> > > > > @@ -162,8 +162,10 @@ struct ssl_config_file {
> > > > > static struct ssl_config_file private_key;
> > > > > static struct ssl_config_file certificate;
> > > > > static struct ssl_config_file ca_cert;
> > > > > -static char *ssl_protocols = "TLSv1,TLSv1.1,TLSv1.2";
> > > > > -static char *ssl_ciphers = "HIGH:!aNULL:!MD5";
> > > > > +static char *default_ssl_protocols = "TLSv1,TLSv1.1,TLSv1.2";
> > > > > +static char *default_ssl_ciphers = "HIGH:!aNULL:!MD5";
> > > > > +static char *ssl_protocols = NULL;
> > > > > +static char *ssl_ciphers = NULL;
> > > > >
> > > > > /* Ordinarily, the SSL client and server verify each other's
> > > > > certificates using
> > > > > * a CA certificate. Setting this to false disables this behavior.
> > > > > (This is a
> > > > > @@ -1225,14 +1227,19 @@ stream_ssl_set_key_and_cert(const char
> > > > > *private_key_file,
> > > > > void
> > > > > stream_ssl_set_ciphers(const char *arg)
> > > > > {
> > > > > -if (ssl_init() || !arg || !strcmp(ssl_ciphers, arg)) {
> > > >
> > > > The ssl_init() calls at least one time do_ssl_init() which then
> > > > calls SSL_CTX_set_cipher_list(ctx, "HIGH:!aNULL:!MD5").
> > > > Those are the defaults in the man-page and not from the library.
> > > >
> > > > The do_ssl_init() also does:
> > > >method = CONST_CAST(SSL_METHOD *, SSLv23_method());
> > > >
> > > > That should return SSLv3, TLSv1, TLSv1.1 and TLS1.2.
> > > >
> > > >ctx = SSL_CTX_new(method);
> > > >SSL_CTX_set_options(ctx, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
> > > >
> > > > And there it excludes those SSL v2 and v3.
> > > >
> > > > Therefore, the default would be "TLSv1,TLSv1.1,TLSv1.2" which is
> > > > the same in the man-page.
> > > >
> > > > Did I miss something?
> > >
> > > Thank you for pointing out that, I did not realize we manipulated
> > > these options multiple places.
> > >
> > > I do need to rephrase the commit message, but there is still a problem
> > > here. It became apparent when working on the next patch in the series,
> > > where functional tests behave unexpectedly when passing the
> > > ssl-protocols options. The reason being that when the protocol list
> > > matches what is already in the static char *ssl_protocols in
> > > lib/stream-ssl.c stream_ssl_set_protocols returns early without
> > > setting any option.
> >
> > If that matches then it is because the default is set, so it
> > shouldn't make any difference to return early. Do you still
> > have the next patch without the default_ssl_protocols change
> > for me to take a look? That might help me to see the issue.
>
> That would be true if do_ssl_init() and stream_ssl_set_protocols()
> manipulated the options the same way, the reality is that they do not,
> and the effective output from do_ssl_init() differ depending on the
> version of OpenSSL Open vSwitch is built with.
>
> > > So I guess the question then becomes, should we stop doing this
> > > multiple places or do you want me to update the call to
> > > SSL_CTX_set_options in do_ssl_init and not introduce this change?
> >
> > Not sure yet because I didn't see the problem yet.
>
> Let me demonstrate, with OVS built from master against OpenSSL 1.1.1f
> and the following modification to the ovs-sandbox script:
> --- a/tutorial/ovs-sandbox
> +++ b/tutorial/ovs-sandbox
> @@ -270,7 +270,7 @@ trap 'kill `cat "$sandbox"/*.pid`' 0 1 2 3 13 14 15
> # Create database and start ovsdb-server.
> touch "$sandbox"/.conf.db.~lock~
> run ovsdb-tool create conf.db "$schema"
> -ovsdb_server_args=
> +ovsdb_server_args="--private-key=db:Open_vSwitch,SSL,private_key
> --certificate=db:Open_vSwitch,SSL,certificate
> --bootstrap-c
[ovs-dev] [PATCH v2] checkpatch: Detect "trojan source" attack
Recently there has been a lot of press about the "trojan source" attack,
where Unicode characters are used to obfuscate the true functionality of
code. This attack didn't effect OVS, but adding the check here will help
guard against it sneaking in later.
Signed-off-by: Mike Pattrick
---
Changes in v2:
- Now all unicode characters will result in an error.
---
utilities/checkpatch.py | 11 +++
1 file changed, 11 insertions(+)
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index bf95358d5..03d91f765 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -181,6 +181,7 @@ __regex_added_doc_rst = re.compile(
__regex_empty_return = re.compile(r'\s*return;')
__regex_if_macros = re.compile(r'^ +(%s) \([\S]([\s\S]+[\S])*\) { +\\' %
__parenthesized_constructs)
+__regex_nonascii_characters = re.compile("[^ -~\t]")
skip_leading_whitespace_check = False
skip_trailing_whitespace_check = False
@@ -294,6 +295,11 @@ def pointer_whitespace_check(line):
return __regex_ptr_declaration_missing_whitespace.search(line) is not None
+def nonascii_character_check(line):
+"""Return TRUE if inappropriate Unicode characters are detected """
+return __regex_nonascii_characters.search(line) is not None
+
+
def cast_whitespace_check(line):
"""Return TRUE if there is no space between the '()' used in a cast and
the expression whose type is cast, i.e.: '(void *)foo'"""
@@ -565,6 +571,11 @@ checks = [
'print':
lambda: print_error("Inappropriate spacing in pointer declaration")},
+{'regex': r'(\.c|\.h)(\.in)?$', 'match_name': None,
+ 'check': lambda x: nonascii_character_check(x),
+ 'print':
+ lambda: print_error("Inappropriate non-ascii characters detected.")},
+
{'regex': r'(\.c|\.h)(\.in)?$', 'match_name': None,
'prereq': lambda x: not is_comment_line(x),
'check': lambda x: cast_whitespace_check(x),
--
2.27.0
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn] northd: fix FIP traffic with distributed gw router port on the same hv
If the hv has FIP assigned, traffic has to be sent out using the FIP
even if a distributed gw router port is scheduled on the local hv.
In this particular use-case without the proposed patch, the traffic
is sent out with FIP mac but using distributed gw router port IP.
Related bz: https://bugzilla.redhat.com/show_bug.cgi?id=1960096
Signed-off-by: Lorenzo Bianconi
---
northd/northd.c | 4
tests/system-ovn.at | 16
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index 1e8a3457c..d10470a4e 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -12427,6 +12427,10 @@ build_lrouter_out_snat_flow(struct hmap *lflows,
struct ovn_datapath *od,
priority += 128;
ds_put_format(match, " && is_chassis_resident(%s)",
od->l3dgw_ports[0]->cr_port->json_key);
+} else if (distributed) {
+priority += 128;
+ds_put_format(match, " && is_chassis_resident(\"%s\")",
+ nat->logical_port);
}
ds_clear(actions);
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 77c811946..c9f5771c9 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -3547,9 +3547,9 @@ NS_CHECK_EXEC([foo2], [ping -q -c 3 -i 0.3 -w 2
172.16.1.2 | FORMAT_PING], \
])
# We verify that SNAT indeed happened via 'dump-conntrack' command.
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.1) | \
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.4) | \
sed -e 's/zone=[[0-9]]*/zone=/'], [0], [dnl
-icmp,orig=(src=192.168.1.3,dst=172.16.1.2,id=,type=8,code=0),reply=(src=172.16.1.2,dst=172.16.1.1,id=,type=0,code=0),zone=
+icmp,orig=(src=192.168.1.3,dst=172.16.1.2,id=,type=8,code=0),reply=(src=172.16.1.2,dst=172.16.1.4,id=,type=0,code=0),zone=
])
AT_CHECK([ovs-appctl dpctl/flush-conntrack])
@@ -3719,9 +3719,9 @@ NS_CHECK_EXEC([foo2], [ping6 -q -c 3 -i 0.3 -w 2 fd20::2
| FORMAT_PING], \
])
# We verify that SNAT indeed happened via 'dump-conntrack' command.
-AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fd20::1) | \
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fd11::3) | \
sed -e 's/zone=[[0-9]]*/zone=/'], [0], [dnl
-icmpv6,orig=(src=fd11::3,dst=fd20::2,id=,type=128,code=0),reply=(src=fd20::2,dst=fd20::1,id=,type=129,code=0),zone=
+icmpv6,orig=(src=fd11::3,dst=fd20::2,id=,type=128,code=0),reply=(src=fd20::2,dst=fd11::3,id=,type=129,code=0),zone=
])
AT_CHECK([ovs-appctl dpctl/flush-conntrack])
@@ -3907,8 +3907,8 @@ NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2
172.16.1.4 | FORMAT_PING], \
# Then DNAT of 'bar1' address happens (listed first below).
AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(172.16.1.4) | \
sed -e 's/zone=[[0-9]]*/zone=/'], [0], [dnl
-icmp,orig=(src=172.16.1.1,dst=172.16.1.4,id=,type=8,code=0),reply=(src=192.168.2.2,dst=172.16.1.1,id=,type=0,code=0),zone=
-icmp,orig=(src=192.168.1.2,dst=172.16.1.4,id=,type=8,code=0),reply=(src=172.16.1.4,dst=172.16.1.1,id=,type=0,code=0),zone=
+icmp,orig=(src=172.16.1.3,dst=172.16.1.4,id=,type=8,code=0),reply=(src=192.168.2.2,dst=172.16.1.3,id=,type=0,code=0),zone=
+icmp,orig=(src=192.168.1.2,dst=172.16.1.4,id=,type=8,code=0),reply=(src=172.16.1.4,dst=172.16.1.3,id=,type=0,code=0),zone=
icmp,orig=(src=192.168.1.2,dst=172.16.1.4,id=,type=8,code=0),reply=(src=172.16.1.4,dst=192.168.1.2,id=,type=0,code=0),zone=
])
@@ -4102,8 +4102,8 @@ NS_CHECK_EXEC([foo1], [ping -q -c 3 -i 0.3 -w 2 fd20::4 |
FORMAT_PING], \
AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fd20::4) | \
sed -e 's/zone=[[0-9]]*/zone=/'], [0], [dnl
icmpv6,orig=(src=fd11::2,dst=fd20::4,id=,type=128,code=0),reply=(src=fd20::4,dst=fd11::2,id=,type=129,code=0),zone=
-icmpv6,orig=(src=fd11::2,dst=fd20::4,id=,type=128,code=0),reply=(src=fd20::4,dst=fd20::1,id=,type=129,code=0),zone=
-icmpv6,orig=(src=fd20::1,dst=fd20::4,id=,type=128,code=0),reply=(src=fd12::2,dst=fd20::1,id=,type=129,code=0),zone=
+icmpv6,orig=(src=fd11::2,dst=fd20::4,id=,type=128,code=0),reply=(src=fd20::4,dst=fd20::3,id=,type=129,code=0),zone=
+icmpv6,orig=(src=fd20::3,dst=fd20::4,id=,type=128,code=0),reply=(src=fd12::2,dst=fd20::3,id=,type=129,code=0),zone=
])
AT_CHECK([ovs-appctl dpctl/flush-conntrack])
--
2.31.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Terminate native tunnels only on ports with IP addresses.
Hello Ilya,
For some reason, the included test fails for me when I try to run it.
The diff is:
./tunnel-push-pop.at:751: tail -2 stdout
--- -2021-11-15 12:09:05.838890065 -0500
+++ /root/ovs/tests/testsuite.dir/at-groups/782/stdout2021-11-15
12:09:05.836652743 -0500
@@ -1,3 +1,3 @@
-Megaflow: recirc_id=0,eth,in_port=2,dl_type=0x
-Datapath actions: 3
+Megaflow: recirc_id=0,eth,in_port=1,dl_type=0x
+Datapath actions: 5
I see that it passed github CI, so I'm unsure why it's failing for me.
-M
On Mon, Nov 1, 2021 at 4:15 PM Ilya Maximets wrote:
>
> Commit dc0bd12f5b04 removed restriction that tunnel endpoint must be a
> bridge port. So, currently OVS has to check if the native tunnel needs
> to be terminated regardless of the output port. Unfortunately, there
> is a side effect: tnl_port_map_lookup() always adds at least 'dl_dst'
> match to the megaflow that ends up in the corresponding datapath flow.
> And since tunneling works on L3 level and not restricted by any
> particular bridge, this extra match criteria is added to every
> datapath flow on every bridge even if that bridge cannot be part of
> a tunnel processing.
>
> For example, if OVS has at least one tunnel configured and we're
> adding a completely separate bridge with 2 ports and simple rules
> to forward packets between two ports, there still will be a match on
> a destination mac address:
>
> 1.
> 2. ovs-vsctl add-br br-non-tunnel -- set bridge datapath_type=netdev
> 3. ovs-vsctl add-port br-non-tunnel port0
>-- add-port br-non-tunnel port1
> 4. ovs-ofctl del-flows br-non-tunnel
> 5. ovs-ofctl add-flow br-non-tunnel in_port=port0,actions=port1
> 6. ovs-ofctl add-flow br-non-tunnel in_port=port1,actions=port0
>
> # ovs-appctl ofproto/trace br-non-tunnel in_port=port0
>
> Flow: in_port=1,vlan_tci=0x,
>dl_src=00:00:00:00:00:00,dl_dst=00:00:00:00:00:00,dl_type=0x
>
> bridge("br-non-tunnel")
> ---
> 0. in_port=1, priority 32768
> output:2
>
> Final flow: unchanged
> Megaflow: recirc_id=0,eth,in_port=1,dl_dst=00:00:00:00:00:00,dl_type=0x
> Datapath actions: 5
>
> This increases the number of upcalls and installed datapath flows,
> since separate flow needs to be installed per destination MAC, reducing
> the switching performance. This also blocks datapath performance
> optimizations that are based on the datapath flow simplicity.
>
> In general, in order to be a tunnel endpoint, port has to have an IP
> address. Hence native tunnel termination should be attempted only
> for such ports. This allows to avoid extra matches in most cases.
>
> Fixes: dc0bd12f5b04 ("userspace: Enable non-bridge port as tunnel endpoint.")
> Reported-by: Sriharsha Basavapatna
> Reported-at:
> https://mail.openvswitch.org/pipermail/ovs-dev/2021-October/388904.html
> Signed-off-by: Ilya Maximets
> ---
>
> I'm sure not an expert in this part of a code, but the change seems to pass
> all unit and system tests. It also passes OVN tests. So, I'm assuming that
> it's correct.
>
> ofproto/ofproto-dpif-xlate.c | 31 +++
> tests/packet-type-aware.at | 4 +--
> tests/tunnel-push-pop.at | 58
> 3 files changed, 86 insertions(+), 7 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 9d336bc6a..701902b0c 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4081,14 +4081,35 @@ is_neighbor_reply_correct(const struct xlate_ctx
> *ctx, const struct flow *flow)
> }
>
> static bool
> -terminate_native_tunnel(struct xlate_ctx *ctx, struct flow *flow,
> -struct flow_wildcards *wc, odp_port_t *tnl_port)
> +xport_has_ip(const struct xport *xport)
> +{
> +struct in6_addr *ip_addr, *mask;
> +int n_in6 = 0;
> +
> +if (netdev_get_addr_list(xport->netdev, &ip_addr, &mask, &n_in6)) {
> +n_in6 = 0;
> +} else {
> +free(ip_addr);
> +free(mask);
> +}
> +return n_in6 ? true : false;
> +}
> +
> +static bool
> +terminate_native_tunnel(struct xlate_ctx *ctx, const struct xport *xport,
> +struct flow *flow, struct flow_wildcards *wc,
> +odp_port_t *tnl_port)
> {
> *tnl_port = ODPP_NONE;
>
> /* XXX: Write better Filter for tunnel port. We can use in_port
> - * in tunnel-port flow to avoid these checks completely. */
> -if (ovs_native_tunneling_is_on(ctx->xbridge->ofproto)) {
> + * in tunnel-port flow to avoid these checks completely.
> + *
> + * Port without an IP address cannot be a tunnel termination point.
> + * Not performing a lookup in this case to avoid unwildcarding extra
> + * flow fields (dl_dst). */
> +if (ovs_native_tunneling_is_on(ctx->xbridge->ofproto)
> +&& xport_has_ip(xport)) {
> *tnl_port = tnl_port_map_lookup(flow, wc);
>
> /*
Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Terminate native tunnels only on ports with IP addresses.
On 11/15/21 18:28, Mike Pattrick wrote: > Hello Ilya, > > For some reason, the included test fails for me when I try to run it. > The diff is: > > ./tunnel-push-pop.at:751: tail -2 stdout > --- -2021-11-15 12:09:05.838890065 -0500 > +++ /root/ovs/tests/testsuite.dir/at-groups/782/stdout2021-11-15 > 12:09:05.836652743 -0500 > @@ -1,3 +1,3 @@ > -Megaflow: recirc_id=0,eth,in_port=2,dl_type=0x > -Datapath actions: 3 > +Megaflow: recirc_id=0,eth,in_port=1,dl_type=0x > +Datapath actions: 5 > > I see that it passed github CI, so I'm unsure why it's failing for me. > > -M Good catch. Thanks! I suppose, you're building with higher instruction set enabled, e.g. sse4.2 or something like this (We definitely need to add a job like this to GHA). This may affect the order in which ports are added and hence the port numbers. I shouldn't add ports this way. add_of_ports should be used instead, it will set ofport_request and will use names that will be translated into datapath port numbers. Could you try this: diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at index ed72ff986..1c9f6cad1 100644 --- a/tests/tunnel-push-pop.at +++ b/tests/tunnel-push-pop.at @@ -737,26 +737,25 @@ AT_CHECK([ dnl Creating a separate bridge that is completely unrelated to a tunnel dnl configuration. Ports in this bridge cannot be tunnel endpoints. AT_CHECK([ovs-vsctl add-br br-non-tunnel dnl - -- set bridge br-non-tunnel datapath_type=dummy fail-mode=secure dnl - -- add-port br-non-tunnel port0 -- set Interface port0 type=dummy dnl - -- add-port br-non-tunnel port1 -- set Interface port1 type=dummy]) + -- set bridge br-non-tunnel datapath_type=dummy fail-mode=secure]) +add_of_ports br-non-tunnel 7 8 AT_CHECK([ovs-ofctl del-flows br-non-tunnel]) -AT_CHECK([ovs-ofctl add-flow br-non-tunnel in_port=port0,action=port1]) -AT_CHECK([ovs-ofctl add-flow br-non-tunnel in_port=port1,action=port0]) +AT_CHECK([ovs-ofctl add-flow br-non-tunnel in_port=p7,action=p8]) +AT_CHECK([ovs-ofctl add-flow br-non-tunnel in_port=p8,action=p7]) dnl Checking that tunnel configuration doesn't impact flow translation dnl on this bridge (Megaflow should contain a bare minimum of fields dnl according to installed OF rules). -AT_CHECK([ovs-appctl ofproto/trace br-non-tunnel in_port=port0], [0], [stdout]) +AT_CHECK([ovs-appctl ofproto/trace br-non-tunnel in_port=p7], [0], [stdout]) AT_CHECK([tail -2 stdout], [0], [dnl -Megaflow: recirc_id=0,eth,in_port=2,dl_type=0x -Datapath actions: 3 +Megaflow: recirc_id=0,eth,in_port=7,dl_type=0x +Datapath actions: 8 ]) -AT_CHECK([ovs-appctl ofproto/trace br-non-tunnel in_port=port1], [0], [stdout]) +AT_CHECK([ovs-appctl ofproto/trace br-non-tunnel in_port=p8], [0], [stdout]) AT_CHECK([tail -2 stdout], [0], [dnl -Megaflow: recirc_id=0,eth,in_port=1,dl_type=0x -Datapath actions: 5 +Megaflow: recirc_id=0,eth,in_port=8,dl_type=0x +Datapath actions: 7 ]) OVS_VSWITCHD_STOP --- ? Bets regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn v10 0/4] Introduce infrastructure for VIF plug providers.
On Tue, Nov 9, 2021 at 3:03 PM Han Zhou wrote: > > On Tue, Nov 9, 2021 at 11:12 AM Numan Siddique wrote: > > > > On Tue, Nov 9, 2021 at 1:38 AM Frode Nordahl > > wrote: > > > > > > On Mon, Nov 8, 2021 at 11:08 PM Numan Siddique wrote: > > > > > > > > On Fri, Nov 5, 2021 at 5:32 PM Frode Nordahl > > > > wrote: > > > > > > > > > > fre. 5. nov. 2021, 20:43 skrev Han Zhou : > > > > > > > > > > > > > > > > > > > > > > > On Fri, Nov 5, 2021 at 7:00 AM Frode Nordahl < > frode.nord...@canonical.com> > > > > > > wrote: > > > > > > > > > > > > > > Introduce infrastructure for VIF plug providers and add feature > to > > > > > > > ovn-controller to add and remove ports on the integration > bridge as > > > > > > > directed by CMS through Logical_Switch_Port options. > > > > > > > > > > > > > > Traditionally it has been the CMSs responsibility to create > Virtual > > > > > > > Interfaces (VIFs) as part of instance (Container, Pod, Virtual > > > > > > > Machine etc.) life cycle, and subsequently manage plug/unplug > > > > > > > operations on the Open vSwitch integration bridge. > > > > > > > > > > > > > > With the advent of NICs connected to multiple distinct CPUs we > can > > > > > > > have a topology where the instance runs on one host and Open > > > > > > > vSwitch and OVN runs on a different host, the smartnic control > plane > > > > > > > CPU. The host facing interfaces will be visible to Open vSwitch > > > > > > > and OVN as representor ports. > > > > > > > > > > > > > > The actions necessary for plugging and unplugging the > representor > > > > > > > port in Open vSwitch running on the smartnic control plane CPU > would > > > > > > > be the same for every CMS. > > > > > > > > > > > > > > Hardware or platform specific details for initialization and > lookup > > > > > > > of representor ports is provided by an plugging provider hosted > > > > > > > inside or outside the core OVN repository, and linked at OVN > build > > > > > > > time. > > > > > > > > > > > > > > RFC1 -> RFC2: > > > > > > > - Introduce the plug-provider interface, remove > hardware/platform > > > > > > >dependent code. > > > > > > > - Add ovsport module. > > > > > > > - Integrate with binding module. > > > > > > > - Split into multiple patches. > > > > > > > > > > > > > > RFC2 -> v1: > > > > > > > - Extend build system, `--with-plug-provider`. > > > > > > > - Check for required data in b_ctx_in and lbinding data > structures. > > > > > > > - Split consider_plug_lport into update and create processing > > > > > > >functions. > > > > > > > - Consider unplug on release where relevant. > > > > > > > - Add ovn-controller `--enable-dummy-plug` option for testing. > > > > > > > - Consistent function naming and move ovsport module to > controller/. > > > > > > > - Rename plug-test.c -> plug-dummy.c. > > > > > > > - Add functional- and unit- tests. > > > > > > > > > > > > > > v1 -> v2: > > > > > > > - Move update to controller/binding.h from patch 6 -> patch 5. > > > > > > > - Fix lint problems reported by 0-day Robot. > > > > > > > > > > > > > > v2 -> v3: > > > > > > > - Fix build system extension for plug provider. > > > > > > > - Implement DDlog version of northd change. > > > > > > > - Rebase on tip. > > > > > > > > > > > > > > v3 -> v4: > > > > > > > - sb:Port_Binding:plugged_by -> > sb:Port_Binding:requested_chassis. > > > > > > > - Move documentation of plugin specific options to plugin > > > > > > >implementation. > > > > > > > - ovn-northd-ddlog: squash changes into same patch as C > version, rework > > > > > > >the DDlog implementation. > > > > > > > - controller: Use requested_chassis column instead of parsing > options. > > > > > > > - plug-provider: Remove the extra class instantiation layer and > > > > > > >refcounting. Add documentation and various tweaks. > > > > > > > - controller: Add engine node for plug provider so that a > plugin run > > > > > > >function can run at an appropriate time and also allow > feeding back > > > > > > >information about changes that can not be handled > incrementally. > > > > > > > - binding: Fix return values for plug functions so they adhere > to the > > > > > > >incremental processing engine contract. > > > > > > > - Add support for building in-tree plug providers. > > > > > > > > > > > > > > v4 -> v5: > > > > > > > - binding: Make some data structures and functions public to > allow > > > > > > >other modules to make use of its local binding tracking data > > > > > > >structures. > > > > > > > - Add separate incremental processing engine nodes for the plug > > > > > > >provider. > > > > > > > - Do change handling in the controller/plug module rather than > piggy > > > > > > >backing on the binding module. > > > > > > > - Deal with asynchronous notification to plug provider class > and > > > > > > >subsequent freeing of data structures when the transaction > commits. > > > > > > > - Drop the representor plugin as in-tree
Re: [ovs-dev] [PATCH ovn] northd: fix ttl exceeded with FIP
On Thu, Nov 11, 2021 at 3:44 PM Mark Michelson wrote:
>
> Hi Lorenzo, the patch looks good to me.
>
> Acked-by: Mark Michelson
>
> On 11/10/21 17:35, Lorenzo Bianconi wrote:
> > Properly manage ttl exceeded ICMP error messages when traffic is
> > directed to a FIP. The issue can be verified running traceroute from an
> > external device to a FIP:
> > $traceroute -I -z 1 -n
> >
> > Fix ttl exceeded priority respect to ping responder.
> >
> > Related bz: https://bugzilla.redhat.com/show_bug.cgi?id=2006349
> >
> > Signed-off-by: Lorenzo Bianconi
> > ---
> > northd/northd.c | 59 -
> > northd/ovn-northd.8.xml | 2 +-
> > tests/ovn.at| 10 +++
> > 3 files changed, 41 insertions(+), 30 deletions(-)
> >
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 1e8a3457c..ce962cdc4 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -11798,6 +11798,7 @@ build_ipv6_input_flows_for_lrouter_port(
> > }
> >
> > /* ICMPv6 time exceeded */
> > +struct ds ip_ds = DS_EMPTY_INITIALIZER;
> > for (int i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) {
> > /* skip link-local address */
> > if (in6_is_lla(&op->lrp_networks.ipv6_addrs[i].network)) {
> > @@ -11806,7 +11807,13 @@ build_ipv6_input_flows_for_lrouter_port(
> >
> > ds_clear(match);
> > ds_clear(actions);
> > -
> > +ds_clear(&ip_ds);
> > +if (op->od->n_l3dgw_ports && op->od->l3dgw_ports[0] == op) {
> > +ds_put_cstr(&ip_ds, "ip6.dst <-> ip6.src");
> > +} else {
> > +ds_put_format(&ip_ds, "ip6.dst = ip6.src; ip6.src = %s",
> > + op->lrp_networks.ipv6_addrs[i].addr_s);
> > +}
> > ds_put_format(match,
> > "inport == %s && ip6 && "
> > "ip6.src == %s/%d && "
> > @@ -11817,20 +11824,18 @@ build_ipv6_input_flows_for_lrouter_port(
> > ds_put_format(actions,
> > "icmp6 {"
> > "eth.dst <-> eth.src; "
> > - "ip6.dst = ip6.src; "
> > - "ip6.src = %s; "
> > - "ip.ttl = 255; "
> > + "%s ; ip.ttl = 254; "
Hi Lorenzo,
Can you please explain why the ttl is changed to 254 ?
> > "icmp6.type = 3; /* Time exceeded */ "
> > "icmp6.code = 0; /* TTL exceeded in transit */ "
> > - "next; };",
> > - op->lrp_networks.ipv6_addrs[i].addr_s);
> > -ovn_lflow_add_with_hint__(lflows, op->od,
> > S_ROUTER_IN_IP_INPUT, 40,
> > - ds_cstr(match), ds_cstr(actions),
> > NULL,
> > - copp_meter_get(COPP_ICMP6_ERR,
> > - op->od->nbr->copp,
> > - meter_groups),
> > - &op->nbrp->header_);
> > + "outport = %s; flags.loopback = 1; output; };",
> > + ds_cstr(&ip_ds), op->json_key);
> > +ovn_lflow_add_with_hint__(lflows, op->od, S_ROUTER_IN_IP_INPUT,
> > +100, ds_cstr(match), ds_cstr(actions), NULL,
> > +copp_meter_get(COPP_ICMP6_ERR, op->od->nbr->copp,
> > + meter_groups),
> > +&op->nbrp->header_);
> > }
> > +ds_destroy(&ip_ds);
> > }
> >
> > }
> > @@ -11930,10 +11935,17 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
> > build_lrouter_bfd_flows(lflows, op, meter_groups);
> >
> > /* ICMP time exceeded */
> > +struct ds ip_ds = DS_EMPTY_INITIALIZER;
> > for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) {
> > ds_clear(match);
> > ds_clear(actions);
> > -
> > +ds_clear(&ip_ds);
> > +if (op->od->n_l3dgw_ports && op->od->l3dgw_ports[0] == op) {
> > +ds_put_cstr(&ip_ds, "ip4.dst <-> ip4.src");
> > +} else {
> > +ds_put_format(&ip_ds, "ip4.dst = ip4.src; ip4.src = %s",
> > + op->lrp_networks.ipv4_addrs[i].addr_s);
> > +}
> > ds_put_format(match,
> > "inport == %s && ip4 && "
> > "ip.ttl == {0, 1} && !ip.later_frag",
> > op->json_key);
> > @@ -11942,18 +11954,17 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
> > "eth.dst <-> eth.src; "
> > "icmp4.type = 11; /* Time exceeded */ "
> > "icmp4.code = 0; /* TTL exceeded in transit */ "
> > -
[ovs-dev] [PATCH v3] conntrack: support default timeout policy get/set cmd for netdev datapath
From: wenxu
Now, the default timeout policy for netdev datapath is hard codeing. In
some case show or modify is needed.
Add command for get/set default timeout policy. Using like this:
ovs-appctl dpctl/ct-get-default-timeout-policy [dp]
ovs-appctl dpctl/ct-set-default-timeout-policy [dp] policies
Signed-off-by: wenxu
---
NEWS| 4 +++
lib/conntrack-tp.c | 9 ++
lib/conntrack-tp.h | 2 ++
lib/ct-dpif.c | 63 +
lib/ct-dpif.h | 8 +
lib/dpctl.c | 84 +
tests/system-traffic.at | 6
7 files changed, 176 insertions(+)
diff --git a/NEWS b/NEWS
index 434ee57..c6a2eda 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,10 @@ Post-v2.16.0
- ovs-dpctl and 'ovs-appctl dpctl/':
* New commands 'cache-get-size' and 'cache-set-size' that allows to
get or configure linux kernel datapath cache sizes.
+ - ovs-appctl dpctl/:
+ * New commands 'ct-set-default-timeout-policy' and
+ 'ct-set-default-timeout-policy' that allows to get or configure
+ netdev datapath ct default timeout policy.
v2.16.0 - 16 Aug 2021
diff --git a/lib/conntrack-tp.c b/lib/conntrack-tp.c
index a586d3a..726b854 100644
--- a/lib/conntrack-tp.c
+++ b/lib/conntrack-tp.c
@@ -230,6 +230,15 @@ tm_to_ct_dpif_tp(enum ct_timeout tm)
return CT_DPIF_TP_ATTR_MAX;
}
+void
+timeout_policy_dump(const struct ct_dpif_timeout_policy *tp, struct ds *ds)
+{
+for (unsigned i = 0; i < N_CT_TM; i++) {
+ds_put_format(ds, "\n\t%s = %"PRIu32, ct_timeout_str[i],
+ tp->attrs[tm_to_ct_dpif_tp(i)]);
+}
+}
+
static void
conn_update_expiration__(struct conntrack *ct, struct conn *conn,
enum ct_timeout tm, long long now,
diff --git a/lib/conntrack-tp.h b/lib/conntrack-tp.h
index 4d411d1..bd22242 100644
--- a/lib/conntrack-tp.h
+++ b/lib/conntrack-tp.h
@@ -22,6 +22,8 @@ enum ct_timeout;
void timeout_policy_init(struct conntrack *ct);
int timeout_policy_update(struct conntrack *ct, struct timeout_policy *tp);
int timeout_policy_delete(struct conntrack *ct, uint32_t tp_id);
+void timeout_policy_dump(const struct ct_dpif_timeout_policy *tp,
+ struct ds *ds);
struct timeout_policy *timeout_policy_get(struct conntrack *ct, int32_t tp_id);
void conn_init_expiration(struct conntrack *ct, struct conn *conn,
enum ct_timeout tm, long long now);
diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c
index cfc2315..6e36da2 100644
--- a/lib/ct-dpif.c
+++ b/lib/ct-dpif.c
@@ -20,6 +20,8 @@
#include
#include "ct-dpif.h"
+#include "conntrack-private.h"
+#include "conntrack-tp.h"
#include "openvswitch/ofp-parse.h"
#include "openvswitch/vlog.h"
@@ -180,6 +182,24 @@ ct_dpif_get_tcp_seq_chk(struct dpif *dpif, bool *enabled)
}
int
+ct_dpif_set_default_timeout_policy(struct dpif *dpif,
+ const struct ct_dpif_timeout_policy *tp)
+{
+return (dpif->dpif_class->ct_set_timeout_policy
+? dpif->dpif_class->ct_set_timeout_policy(dpif, tp)
+: EOPNOTSUPP);
+}
+
+int
+ct_dpif_get_default_timeout_policy(struct dpif *dpif,
+ struct ct_dpif_timeout_policy *tp)
+{
+return (dpif->dpif_class->ct_get_timeout_policy
+? dpif->dpif_class->ct_get_timeout_policy(dpif, DEFAULT_TP_ID, tp)
+: EOPNOTSUPP);
+}
+
+int
ct_dpif_set_limits(struct dpif *dpif, const uint32_t *default_limit,
const struct ovs_list *zone_limits)
{
@@ -710,6 +730,42 @@ ct_dpif_free_zone_limits(struct ovs_list *zone_limits)
}
}
+
+/* Parses a specification of a timeout policy from 's' into '*tp'
+ * . Returns true on success. Otherwise, returns false and
+ * and puts the error message in 'ds'. */
+bool
+ct_dpif_parse_timeout_policy_tuple(const char *s, struct ds *ds,
+ struct ct_dpif_timeout_policy *tp)
+{
+char *pos, *key, *value, *copy, *err;
+
+pos = copy = xstrdup(s);
+while (ofputil_parse_key_value(&pos, &key, &value)) {
+uint32_t tmp;
+
+if (!*value) {
+ds_put_format(ds, "field %s missing value", key);
+goto error;
+}
+
+err = str_to_u32(value, &tmp);
+if (err) {
+ free(err);
+ goto error_with_msg;
+}
+
+ct_dpif_set_timeout_policy_attr_by_name(tp, key, tmp);
+}
+free(copy);
+return true;
+
+error_with_msg:
+ds_put_format(ds, "failed to parse field %s", key);
+error:
+free(copy);
+return false;
+}
/* Parses a specification of a conntrack zone limit from 's' into '*pzone'
* and '*plimit'. Returns true on success. Otherwise, returns false and
* and puts the error message in 'ds'. */
@@ -792,6 +848,13 @@ static const char *const ct_dpif_tp_attr_string[] = {
#undef CT_DPIF_TP_ICMP_ATTR
};
+void
+ct_dpif_format_t
