Re: [ovs-dev] [PATCH ovn 1/5] ovn-controller: Support ssl cert rotation when command line options are used.

[Han Zhou]

On Mon, May 17, 2021 at 7:05 PM Han Zhou  wrote:
>
>
>
> On Mon, May 17, 2021 at 6:08 PM Mark Michelson 
wrote:
> >
> > Hi Han,
> >
> > My comments below apply equally to the other patches in this series
> > since they are generally similar.
> >
> > I think each patch could use a simple accompanying test case. The test
> > could ensure that updates to the files are picked up by the OVN
> > component. It could also potentially ensure that nothing awful happens
> > if, say, you delete one or more of the files.
> >
> Thanks Mark for the review.
> Ok, I was manually testing by playing around with the expiration time of
the certs. Let me see if I can work out a way that is feasible for a test
case.
>
> > See below for more:
> >
> > On 5/13/21 6:46 PM, Han Zhou wrote:
> > > When SSL configurations are set in Open_vSwitch SSL table,
> > > ovn-controller handles file update properly by re-applying the
settings
> > > in the main loop. However, it is also valid to set the options in
> > > command line of ovn-controller without using the SSL table. In this
> > > case, the options are set onetime only and it never reapplies when the
> > > file content changes. This patch fixes this by allowing reapplying the
> > > command line options in the main loop, if they are set. SSL table
> > > settings still takes precedence if both exist.
> > >
> > > Signed-off-by: Han Zhou 
> > > ---
> > >   controller/ovn-controller.c | 24 +++-
> > >   1 file changed, 23 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > > index 67c51a86f..5a755276b 100644
> > > --- a/controller/ovn-controller.c
> > > +++ b/controller/ovn-controller.c
> > > @@ -97,6 +97,11 @@ static unixctl_cb_func debug_delay_nb_cfg_report;
> > >   static char *parse_options(int argc, char *argv[]);
> > >   OVS_NO_RETURN static void usage(void);
> > >
> > > +/* SSL options */
> > > +static const char *ssl_private_key_file;
> > > +static const char *ssl_certificate_file;
> > > +static const char *ssl_ca_cert_file;
> > > +
> > >   /* By default don't set an upper bound for the lflow cache. */
> > >   #define DEFAULT_LFLOW_CACHE_MAX_ENTRIES UINT32_MAX
> > >   #define DEFAULT_LFLOW_CACHE_MAX_MEM_KB (UINT64_MAX / 1024)
> > > @@ -441,6 +446,11 @@ update_ssl_config(const struct ovsrec_ssl_table
*ssl_table)
> > >   if (ssl) {
> > >   stream_ssl_set_key_and_cert(ssl->private_key,
ssl->certificate);
> > >   stream_ssl_set_ca_cert_file(ssl->ca_cert,
ssl->bootstrap_ca_cert);
> > > +} else if (ssl_private_key_file && ssl_certificate_file &&
> > > +   ssl_ca_cert_file) {
> >
> > Why must all three of these be non-NULL to call the stream_ssl functions
> > below? Since the command line options used to result in individual calls
> > to stream_ssl functions while parsing options, this represents a
> > potential behavior change. For instance, if a user had for some reason
> > only specified the -C option when starting ovn-controller, we would have
> > called stream_ssl_set_ca_cert_file(). But now if they only specify -C,
> > then we will not call stream_ssl_set_ca_cert_file() since they did not
> > also set the -c and -p options.
> >
> IIUC, the three files must be supplied for the SSL to work, so I think it
makes no sense to proceed if any of them is NULL.
> However, you are right that this is a behavior change. In the old
implementation, there will be error logs complaining the missing file, but
now it would be complaining about all the three files. Since the
stream_ssl_xxx() interfaces already checks NULL, I will skip the NULL check
here and call the functions with whatever is available in v2.
>
Hi Mark, I think it is still necessary to check NULL pointers, so in v2 I
just change the if condition to check key+cert and cacert separately before
calling the respective ssl_set_xxx functions. Please take a look at v2:
https://patchwork.ozlabs.org/project/ovn/list/?series=244813

Thanks,
Han
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH ovn v2 1/3] ovn-controller: Support ssl cert rotation when command line options are used.

[Han Zhou]

When SSL configurations are set in Open_vSwitch SSL table,
ovn-controller handles file update properly by re-applying the settings
in the main loop. However, it is also valid to set the options in
command line of ovn-controller without using the SSL table. In this
case, the options are set onetime only and it never reapplies when the
file content changes. This patch fixes this by allowing reapplying the
command line options in the main loop, if they are set. SSL table
settings still takes precedence if both exist.

Signed-off-by: Han Zhou 
---
v1 -> v2:
- Addressed Mark Michelson's comments
- Added test cases.
- Check NULL pointer for key & cert, cacert separately.
- Combined ovn-northd/ddlog/ovn-ic changes to one patch.

 controller/ovn-controller.c | 27 ++-
 tests/ovn-controller.at | 53 +
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 031b275c8..d48ddc7a2 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -98,6 +98,11 @@ static unixctl_cb_func debug_delay_nb_cfg_report;
 static char *parse_options(int argc, char *argv[]);
 OVS_NO_RETURN static void usage(void);
 
+/* SSL options */
+static const char *ssl_private_key_file;
+static const char *ssl_certificate_file;
+static const char *ssl_ca_cert_file;
+
 /* By default don't set an upper bound for the lflow cache. */
 #define DEFAULT_LFLOW_CACHE_MAX_ENTRIES UINT32_MAX
 #define DEFAULT_LFLOW_CACHE_MAX_MEM_KB (UINT64_MAX / 1024)
@@ -447,6 +452,14 @@ update_ssl_config(const struct ovsrec_ssl_table *ssl_table)
 if (ssl) {
 stream_ssl_set_key_and_cert(ssl->private_key, ssl->certificate);
 stream_ssl_set_ca_cert_file(ssl->ca_cert, ssl->bootstrap_ca_cert);
+} else {
+if (ssl_private_key_file && ssl_certificate_file) {
+stream_ssl_set_key_and_cert(ssl_private_key_file,
+ssl_certificate_file);
+}
+if (ssl_ca_cert_file) {
+stream_ssl_set_ca_cert_file(ssl_ca_cert_file, false);
+}
 }
 }
 
@@ -,7 +3346,19 @@ parse_options(int argc, char *argv[])
 
 VLOG_OPTION_HANDLERS
 OVN_DAEMON_OPTION_HANDLERS
-STREAM_SSL_OPTION_HANDLERS
+
+case 'p':
+ssl_private_key_file = optarg;
+break;
+
+case 'c':
+ssl_certificate_file = optarg;
+break;
+
+case 'C':
+ssl_ca_cert_file = optarg;
+break;
+
 
 case OPT_PEER_CA_CERT:
 stream_ssl_set_peer_ca_cert_file(optarg);
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index d1d758c49..cff08cb7b 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -549,3 +549,56 @@ done
 
 OVN_CLEANUP([hv1])
 AT_CLEANUP
+
+AT_SETUP([ovn-controller - ssl files change when using command line options])
+AT_KEYWORDS([ovn])
+AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
+PKIDIR="$(cd $abs_top_builddir/tests && pwd)"
+AT_SKIP_IF([expr "$PKIDIR" : ".*[  '\"
+\\]"])
+ovn_start
+
+net_add n1
+sim_add hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+
+key=testpki-hv1-privkey.pem
+cert=testpki-hv1-cert.pem
+cacert=testpki-cacert.pem
+
+key2=testpki-hv2-privkey.pem
+cert3=testpki-hv3-cert.pem
+
+# Use mismatched key and cert when restarting ovn-controller
+cp $PKIDIR/$key2 $key
+cp $PKIDIR/$cert3 $cert
+cp $PKIDIR/$cacert $cacert
+
+# Restart ovn-controller using command line options for SSL parameters
+OVS_APP_EXIT_AND_WAIT([ovn-controller])
+check ovs-vsctl del-ssl
+start_daemon ovn-controller -p $key -c $cert -C $cacert
+
+# SSL should not connect because of key and cert mismatch
+OVS_WAIT_UNTIL([ovn-appctl -t ovn-controller connection-status], [0], [not 
connected])
+
+# Modify the files with the correct key and cert, and reconnect should succeed
+cp $PKIDIR/$key $key
+cp $PKIDIR/$cert $cert
+
+OVS_WAIT_UNTIL([ovn-appctl -t ovn-controller connection-status], [0], 
[connected])
+
+# Remove the files and expect the connection to drop
+rm $key $cert
+OVS_WAIT_UNTIL([ovn-appctl -t ovn-controller connection-status], [0], [not 
connected])
+
+# Restore the files again and expect the connection to recover
+cp $PKIDIR/$key $key
+cp $PKIDIR/$cert $cert
+OVS_WAIT_UNTIL([ovn-appctl -t ovn-controller connection-status], [0], 
[connected])
+
+cat hv1/ovn-controller.log
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
-- 
2.30.2

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH ovn v2 3/3] ovn-nbctl: Support ssl cert rotation for daemon mode.

[Han Zhou]

Update SSL in the server_loop so that updated pki files can be reapplied.

Signed-off-by: Han Zhou 
---
 tests/ovn-nbctl.at| 40 
 utilities/ovn-nbctl.c | 32 +++-
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 8af55161f..c5c07f738 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -2016,3 +2016,43 @@ AT_CHECK([ovsdb-server --detach --no-chdir --pidfile 
--log-file --remote=punix:$
 AT_CHECK([ovn-nbctl show], [0], [ignore])
 OVN_NBCTL_TEST_STOP "/terminating with signal 15/d"
 AT_CLEANUP
+
+AT_SETUP([ovn-nbctl - daemon ssl files change])
+dnl Create ovn-nb database.
+AT_CHECK([ovsdb-tool create ovn-nb.db $abs_top_srcdir/ovn-nb.ovsschema])
+
+dnl Start ovsdb-server.
+
+key=testpki-hv1-privkey.pem
+cert=testpki-hv1-cert.pem
+
+key_server=$PKIDIR/testpki-test-privkey.pem
+cert_server=$PKIDIR/testpki-test-cert.pem
+cacert=$PKIDIR/testpki-cacert.pem
+
+key2=testpki-hv2-privkey.pem
+cert3=testpki-hv3-cert.pem
+
+ssl_options="--remote=pssl:0:127.0.0.1 ovn-nb.db -p $key_server -c 
$cert_server -C $cacert"
+AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --log-file $ssl_options], 
[0], [], [stderr])
+on_exit "kill `cat ovsdb-server.pid`"
+PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
+
+cp $PKIDIR/$key $key
+cp $PKIDIR/$cert $cert
+
+start_daemon ovn-nbctl --pidfile=ovn-nbctl.pid --db=ssl:127.0.0.1:$TCP_PORT \
+-p $key -c $cert -C $cacert
+
+check ovn-appctl -t ovn-nbctl run init
+
+# Overwrite with mismatched key and cert
+cp $PKIDIR/$key2 $key
+cp $PKIDIR/$cert3 $cert
+OVS_WAIT_UNTIL([grep mismatch ovn-nbctl.log])
+
+cp $PKIDIR/$key $key
+cp $PKIDIR/$cert $cert
+OVS_WAIT_UNTIL([ovn-appctl -t ovn-nbctl run show])
+
+AT_CLEANUP
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 84e228f02..65c61f799 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -57,6 +57,11 @@ static bool oneline;
 /* --dry-run: Do not commit any changes. */
 static bool dry_run;
 
+/* SSL options */
+static const char *ssl_private_key_file;
+static const char *ssl_certificate_file;
+static const char *ssl_ca_cert_file;
+
 /* --wait=TYPE: Wait for configuration change to take effect? */
 enum nbctl_wait_type {
 NBCTL_WAIT_NONE,/* Do not wait. */
@@ -549,6 +554,18 @@ add_local_option(const char *name, const char *arg,
 return NULL;
 }
 
+static void
+update_ssl_config(void)
+{
+if (ssl_private_key_file && ssl_certificate_file) {
+stream_ssl_set_key_and_cert(ssl_private_key_file,
+ssl_certificate_file);
+}
+if (ssl_ca_cert_file) {
+stream_ssl_set_ca_cert_file(ssl_ca_cert_file, false);
+}
+}
+
 static void
 apply_options_direct(const struct ovs_cmdl_parsed_option *parsed_options,
  size_t n, struct shash *local_options)
@@ -621,7 +638,18 @@ apply_options_direct(const struct ovs_cmdl_parsed_option 
*parsed_options,
 OVN_DAEMON_OPTION_HANDLERS
 VLOG_OPTION_HANDLERS
 TABLE_OPTION_HANDLERS(_style)
-STREAM_SSL_OPTION_HANDLERS
+
+case 'p':
+ssl_private_key_file = optarg;
+break;
+
+case 'c':
+ssl_certificate_file = optarg;
+break;
+
+case 'C':
+ssl_ca_cert_file = optarg;
+break;
 
 case OPT_BOOTSTRAP_CA_CERT:
 stream_ssl_set_ca_cert_file(po->arg, true);
@@ -641,6 +669,7 @@ apply_options_direct(const struct ovs_cmdl_parsed_option 
*parsed_options,
 if (!db) {
 db = default_nb_db();
 }
+update_ssl_config();
 }
 
 static void
@@ -6958,6 +6987,7 @@ server_loop(struct ovsdb_idl *idl, int argc, char *argv[])
 server_cmd_init(idl, );
 
 for (;;) {
+update_ssl_config();
 memory_run();
 if (memory_should_report()) {
 struct simap usage = SIMAP_INITIALIZER();
-- 
2.30.2

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH ovn v2 2/3] ovn-northd/ovn-ic: Support ssl cert rotation.

[Han Zhou]

Update SSL in the main loop so that updated pki files can be reapplied.

Signed-off-by: Han Zhou 
---
 ic/ovn-ic.c   | 31 ++-
 northd/ovn-northd-ddlog.c | 31 ++-
 northd/ovn-northd.c   | 31 ++-
 tests/ovn-northd.at   | 38 ++
 4 files changed, 128 insertions(+), 3 deletions(-)

diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index 18e37a31f..d69583956 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -80,6 +80,11 @@ static const char *ovn_ic_nb_db;
 static const char *ovn_ic_sb_db;
 static const char *unixctl_path;
 
+/* SSL options */
+static const char *ssl_private_key_file;
+static const char *ssl_certificate_file;
+static const char *ssl_ca_cert_file;
+
 
 static void
 usage(void)
@@ -1519,7 +1524,18 @@ parse_options(int argc OVS_UNUSED, char *argv[] 
OVS_UNUSED)
 switch (c) {
 OVN_DAEMON_OPTION_HANDLERS;
 VLOG_OPTION_HANDLERS;
-STREAM_SSL_OPTION_HANDLERS;
+
+case 'p':
+ssl_private_key_file = optarg;
+break;
+
+case 'c':
+ssl_certificate_file = optarg;
+break;
+
+case 'C':
+ssl_ca_cert_file = optarg;
+break;
 
 case 'd':
 ovnsb_db = optarg;
@@ -1585,6 +1601,18 @@ add_column_noalert(struct ovsdb_idl *idl,
 ovsdb_idl_omit_alert(idl, column);
 }
 
+static void
+update_ssl_config(void)
+{
+if (ssl_private_key_file && ssl_certificate_file) {
+stream_ssl_set_key_and_cert(ssl_private_key_file,
+ssl_certificate_file);
+}
+if (ssl_ca_cert_file) {
+stream_ssl_set_ca_cert_file(ssl_ca_cert_file, false);
+}
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -1655,6 +1683,7 @@ main(int argc, char *argv[])
 state.had_lock = false;
 state.paused = false;
 while (!exiting) {
+update_ssl_config();
 memory_run();
 if (memory_should_report()) {
 struct simap usage = SIMAP_INITIALIZER();
diff --git a/northd/ovn-northd-ddlog.c b/northd/ovn-northd-ddlog.c
index b7d2c8a5e..73f50e049 100644
--- a/northd/ovn-northd-ddlog.c
+++ b/northd/ovn-northd-ddlog.c
@@ -74,6 +74,11 @@ static const char *ovnnb_db;
 static const char *ovnsb_db;
 static const char *unixctl_path;
 
+/* SSL options */
+static const char *ssl_private_key_file;
+static const char *ssl_certificate_file;
+static const char *ssl_ca_cert_file;
+
 /* Frequently used table ids. */
 static table_id WARNING_TABLE_ID;
 static table_id NB_CFG_TIMESTAMP_ID;
@@ -1094,7 +1099,18 @@ parse_options(int argc OVS_UNUSED, char *argv[] 
OVS_UNUSED)
 switch (c) {
 OVN_DAEMON_OPTION_HANDLERS;
 VLOG_OPTION_HANDLERS;
-STREAM_SSL_OPTION_HANDLERS;
+
+case 'p':
+ssl_private_key_file = optarg;
+break;
+
+case 'c':
+ssl_certificate_file = optarg;
+break;
+
+case 'C':
+ssl_ca_cert_file = optarg;
+break;
 
 case OPT_DDLOG_RECORD:
 record_file = optarg;
@@ -1140,6 +1156,18 @@ parse_options(int argc OVS_UNUSED, char *argv[] 
OVS_UNUSED)
 free(short_options);
 }
 
+static void
+update_ssl_config(void)
+{
+if (ssl_private_key_file && ssl_certificate_file) {
+stream_ssl_set_key_and_cert(ssl_private_key_file,
+ssl_certificate_file);
+}
+if (ssl_ca_cert_file) {
+stream_ssl_set_ca_cert_file(ssl_ca_cert_file, false);
+}
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -1219,6 +1247,7 @@ main(int argc, char *argv[])
 /* Main loop. */
 exiting = false;
 while (!exiting) {
+update_ssl_config();
 memory_run();
 if (memory_should_report()) {
 struct simap usage = SIMAP_INITIALIZER();
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 0e5092a87..04965dd6e 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -107,6 +107,11 @@ static bool use_ct_inv_match = true;
 static int northd_probe_interval_nb = 0;
 static int northd_probe_interval_sb = 0;
 
+/* SSL options */
+static const char *ssl_private_key_file;
+static const char *ssl_certificate_file;
+static const char *ssl_ca_cert_file;
+
 #define MAX_OVN_TAGS 4096
 
 /* Pipeline stages. */
@@ -14009,7 +14014,18 @@ parse_options(int argc OVS_UNUSED, char *argv[] 
OVS_UNUSED)
 switch (c) {
 OVN_DAEMON_OPTION_HANDLERS;
 VLOG_OPTION_HANDLERS;
-STREAM_SSL_OPTION_HANDLERS;
+
+case 'p':
+ssl_private_key_file = optarg;
+break;
+
+case 'c':
+ssl_certificate_file = optarg;
+break;
+
+case 'C':
+ssl_ca_cert_file = optarg;
+break;
 
 case 'd':
 ovnsb_db = optarg;
@@ -14059,6 +14075,18 @@ add_column_noalert(struct ovsdb_idl *idl,
 ovsdb_idl_omit_alert(idl, 

Re: [ovs-dev] [RFC net-next] openvswitch: Introduce per-cpu upcall dispatch

[Pravin Shelar]

On Fri, Apr 30, 2021 at 8:33 AM Mark Gray  wrote:
>
> The Open vSwitch kernel module uses the upcall mechanism to send
> packets from kernel space to user space when it misses in the kernel
> space flow table. The upcall sends packets via a Netlink socket.
> Currently, a Netlink socket is created for every vport. In this way,
> there is a 1:1 mapping between a vport and a Netlink socket.
> When a packet is received by a vport, if it needs to be sent to
> user space, it is sent via the corresponding Netlink socket.
>
> This mechanism, with various iterations of the corresponding user
> space code, has seen some limitations and issues:
>
> * On systems with a large number of vports, there is a correspondingly
> large number of Netlink sockets which can limit scaling.
> (https://bugzilla.redhat.com/show_bug.cgi?id=1526306)
> * Packet reordering on upcalls.
> (https://bugzilla.redhat.com/show_bug.cgi?id=1844576)
> * A thundering herd issue.
> (https://bugzilla.redhat.com/show_bug.cgi?id=183)
>
> This patch introduces an alternative, feature-negotiated, upcall
> mode using a per-cpu dispatch rather than a per-vport dispatch.
>
> In this mode, the Netlink socket to be used for the upcall is
> selected based on the CPU of the thread that is executing the upcall.
> In this way, it resolves the issues above as:
>
> a) The number of Netlink sockets scales with the number of CPUs
> rather than the number of vports.
> b) Ordering per-flow is maintained as packets are distributed to
> CPUs based on mechanisms such as RSS and flows are distributed
> to a single user space thread.
> c) Packets from a flow can only wake up one user space thread.
>
> The corresponding user space code can be found at:
> https://mail.openvswitch.org/pipermail/ovs-dev/2021-April/382618.html
>
> Bugzilla: https://bugzilla.redhat.com/1844576
> Signed-off-by: Mark Gray 
> ---
>  include/uapi/linux/openvswitch.h |  8 
>  net/openvswitch/datapath.c   | 70 +++-
>  net/openvswitch/datapath.h   | 18 
>  net/openvswitch/flow_netlink.c   |  4 --
>  4 files changed, 94 insertions(+), 6 deletions(-)
>
> diff --git a/include/uapi/linux/openvswitch.h 
> b/include/uapi/linux/openvswitch.h
> index 8d16744edc31..6571b57b2268 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -70,6 +70,8 @@ enum ovs_datapath_cmd {
>   * set on the datapath port (for OVS_ACTION_ATTR_MISS).  Only valid on
>   * %OVS_DP_CMD_NEW requests. A value of zero indicates that upcalls should
>   * not be sent.
> + * OVS_DP_ATTR_PER_CPU_PIDS: Per-cpu array of PIDs for upcalls when
> + * OVS_DP_F_DISPATCH_UPCALL_PER_CPU feature is set.
>   * @OVS_DP_ATTR_STATS: Statistics about packets that have passed through the
>   * datapath.  Always present in notifications.
>   * @OVS_DP_ATTR_MEGAFLOW_STATS: Statistics about mega flow masks usage for 
> the
> @@ -87,6 +89,9 @@ enum ovs_datapath_attr {
> OVS_DP_ATTR_USER_FEATURES,  /* OVS_DP_F_*  */
> OVS_DP_ATTR_PAD,
> OVS_DP_ATTR_MASKS_CACHE_SIZE,
> +   OVS_DP_ATTR_PER_CPU_PIDS,   /* Netlink PIDS to receive upcalls in 
> per-cpu
> +* dispatch mode
> +*/
> __OVS_DP_ATTR_MAX
>  };
>
> @@ -127,6 +132,9 @@ struct ovs_vport_stats {
>  /* Allow tc offload recirc sharing */
>  #define OVS_DP_F_TC_RECIRC_SHARING (1 << 2)
>
> +/* Allow per-cpu dispatch of upcalls */
> +#define OVS_DP_F_DISPATCH_UPCALL_PER_CPU   (1 << 3)
> +
>  /* Fixed logical ports. */
>  #define OVSP_LOCAL  ((__u32)0)
>
> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
> index 9d6ef6cb9b26..98d54f41fdaa 100644
> --- a/net/openvswitch/datapath.c
> +++ b/net/openvswitch/datapath.c
> @@ -121,6 +121,8 @@ int lockdep_ovsl_is_held(void)
>  #endif
>
>  static struct vport *new_vport(const struct vport_parms *);
> +static u32 ovs_dp_get_upcall_portid(const struct datapath *, uint32_t);
> +static int ovs_dp_set_upcall_portids(struct datapath *, const struct nlattr 
> *);
>  static int queue_gso_packets(struct datapath *dp, struct sk_buff *,
>  const struct sw_flow_key *,
>  const struct dp_upcall_info *,
> @@ -238,7 +240,12 @@ void ovs_dp_process_packet(struct sk_buff *skb, struct 
> sw_flow_key *key)
>
> memset(, 0, sizeof(upcall));
> upcall.cmd = OVS_PACKET_CMD_MISS;
> -   upcall.portid = ovs_vport_find_upcall_portid(p, skb);
> +
> +   if (dp->user_features & OVS_DP_F_DISPATCH_UPCALL_PER_CPU)
> +   upcall.portid = ovs_dp_get_upcall_portid(dp, 
> smp_processor_id());
> +   else
> +   upcall.portid = ovs_vport_find_upcall_portid(p, skb);
> +
> upcall.mru = OVS_CB(skb)->mru;
> error = ovs_dp_upcall(dp, skb, key, , 0);
> if (unlikely(error))
> @@ -1590,16 

Re: [ovs-dev] [PATCH] Extends the existing mirror configuration parameters

[Wang, Liang-min]

> -Original Message-
> From: Maxime Coquelin 
> Sent: Wednesday, May 19, 2021 4:47 PM
> To: Wang, Liang-min ; Miskell, Timothy
> ; d...@openvswitch.org
> Subject: Re: [PATCH] Extends the existing mirror configuration parameters
> 
> 
> 
> On 5/19/21 4:17 PM, Wang, Liang-min wrote:
> >> -Original Message-
> >> From: Maxime Coquelin 
> >> Sent: Wednesday, May 19, 2021 8:50 AM
> >> To: Wang, Liang-min ; Miskell, Timothy
> >> ; d...@openvswitch.org
> >> Subject: Re: [PATCH] Extends the existing mirror configuration
> parameters
> >>
> >>
> >>
> >> On 5/19/21 1:53 PM, Wang, Liang-min wrote:
>  -Original Message-
>  From: Maxime Coquelin 
>  Sent: Wednesday, May 19, 2021 3:56 AM
>  To: Wang, Liang-min ; Miskell, Timothy
>  ; d...@openvswitch.org
>  Subject: Re: [PATCH] Extends the existing mirror configuration
> >> parameters
> 
>  Hi Liang-min,
> 
>  When replying inline, please do not prefix with ">>" as it is handled as
>  quoted text. There is no need to prefix.
> 
>  On 5/18/21 8:00 PM, Wang, Liang-min wrote:
> >> -Original Message-
> >> From: Maxime Coquelin 
> >> Sent: Tuesday, May 18, 2021 12:15 PM
> >> To: Miskell, Timothy ;
> >> d...@openvswitch.org
> >> Cc: Wang, Liang-min 
> >> Subject: Re: [PATCH] Extends the existing mirror configuration
>  parameters
> >>
> >> Hi Timothy, Liang-min,
> >>
> >> Thanks for rebasing the patch.
> >> A list of delta against the first RFC could help the reviewers.
> >> I notice one change in the right direction is the conversion to Vhost
> >> API datapath instead of Vhost PMD.
> >>
> >>> In this patch we support both Vhost API and Vhost PMD because
> OVS
>  supports both
> >>> VhostUser and Vdev ports.
> >>>
> >> Also, I would suggest to have the patch split in several incremental
> >> patches to ease the review.
> >>
> >>> Thank you for suggestion. We will provide incremental patches on
> >> next
>  submission
> >>>
> >> On 5/10/21 6:00 PM, Timothy Miskell wrote:
> >>> From: Liang-min Wang 
> >>>
> >>> The following parameters are added:
> >>>  - mirror-offload: to turn on/off mirror offloading.
> >>>  - output-port-name: specify a port, using name string, that is on a
>  different
> >>>bridge
> >>>  - output-src-vlan: output port vlan for each select-src-port.
> >>>  - output-dst-vlan: output port vlan for each select-dst-port.
> >>>  - flow-src-mac: use src mac address of each select-dst-port for the
>  header
> >>>scan.
> >>>  - flow-dst-mac: use dst mac address of each select-src-port for the
>  header
> >>>scan.
> >>>  - mirror-tunnel-addr: BDF string of the tunnel device.
> >>>
> >>> ovs-vsctl test change because new mirroring parameters are
> >> introduced
>  in
> >> this patch
> >>
> >> It would help to provide examples of usage of these new
> parameters.
> >>
> >>> Will add examples in the new patches
> >>>
> >>> Create a defer procedure call thread to handle all mirror offload
>  requests.
> >>> This is a light-weight thread which remains in sleep-state when
> there
> >> is
>  no
> >> new request.
> >>> This is created between ovs-vsctl and mirror offloading back end
> >>>
> >>> Implementing DPDK tx-burst (VIRTIO ingress traffic
> >>> mirror) and rx-burst (VIRTIO egress traffic mirror) callbacks.
> >>> Each callback  functions implement the following tasks:
> >>>  1. Enable per-packet VLAN insertion
> >>>- for port mirroring, all packets are enabled per-packet VLAN
> >> insertion.
> >>>- for flow mirroring, only packet header matches the required mac
> >> address
> >>>  are enabled.
> >>>  2. Sending the packets to the specified transport port (output-port
> in
> >>> mirror offload configuration)
> >>>- for port mirroring, all packets are sent to the transport port.
> >>>- for flow mirroring, only matched packets are sent.
> >>>  3. Restore each packet attributes (remove DPDK per-packet
> offload
>  flag)
> >>
> >> I will for sure have more questions later, but please find a few
> >> comments/questions below:
> >>
> >>> Signed-off-by: Liang-min Wang 
> >>> Tested-by: Timothy Miskell 
> >>> Suggested-by: Munish Mehan 
> >>> ---
> >>>  lib/automake.mk|   2 +
> >>>  lib/netdev-dpdk-mirror.c   | 516
> >> +
> >>>  lib/netdev-dpdk-mirror.h   |  83 ++
> >>>  lib/netdev-dpdk.c  | 397 
> >>>  lib/netdev-provider.h  |  16 ++
> >>>  lib/netdev.c   | 386 +++
> >>>  lib/netdev.h   |  16 ++
> >>>  tests/ovs-vsctl.at |   2 +
> >>>  vswitchd/bridge.c  

Re: [ovs-dev] [PATCH] Extends the existing mirror configuration parameters

[Maxime Coquelin]

On 5/19/21 4:17 PM, Wang, Liang-min wrote:
>> -Original Message-
>> From: Maxime Coquelin 
>> Sent: Wednesday, May 19, 2021 8:50 AM
>> To: Wang, Liang-min ; Miskell, Timothy
>> ; d...@openvswitch.org
>> Subject: Re: [PATCH] Extends the existing mirror configuration parameters
>>
>>
>>
>> On 5/19/21 1:53 PM, Wang, Liang-min wrote:
 -Original Message-
 From: Maxime Coquelin 
 Sent: Wednesday, May 19, 2021 3:56 AM
 To: Wang, Liang-min ; Miskell, Timothy
 ; d...@openvswitch.org
 Subject: Re: [PATCH] Extends the existing mirror configuration
>> parameters

 Hi Liang-min,

 When replying inline, please do not prefix with ">>" as it is handled as
 quoted text. There is no need to prefix.

 On 5/18/21 8:00 PM, Wang, Liang-min wrote:
>> -Original Message-
>> From: Maxime Coquelin 
>> Sent: Tuesday, May 18, 2021 12:15 PM
>> To: Miskell, Timothy ;
>> d...@openvswitch.org
>> Cc: Wang, Liang-min 
>> Subject: Re: [PATCH] Extends the existing mirror configuration
 parameters
>>
>> Hi Timothy, Liang-min,
>>
>> Thanks for rebasing the patch.
>> A list of delta against the first RFC could help the reviewers.
>> I notice one change in the right direction is the conversion to Vhost
>> API datapath instead of Vhost PMD.
>>
>>> In this patch we support both Vhost API and Vhost PMD because OVS
 supports both
>>> VhostUser and Vdev ports.
>>>
>> Also, I would suggest to have the patch split in several incremental
>> patches to ease the review.
>>
>>> Thank you for suggestion. We will provide incremental patches on
>> next
 submission
>>>
>> On 5/10/21 6:00 PM, Timothy Miskell wrote:
>>> From: Liang-min Wang 
>>>
>>> The following parameters are added:
>>>  - mirror-offload: to turn on/off mirror offloading.
>>>  - output-port-name: specify a port, using name string, that is on a
 different
>>>bridge
>>>  - output-src-vlan: output port vlan for each select-src-port.
>>>  - output-dst-vlan: output port vlan for each select-dst-port.
>>>  - flow-src-mac: use src mac address of each select-dst-port for the
 header
>>>scan.
>>>  - flow-dst-mac: use dst mac address of each select-src-port for the
 header
>>>scan.
>>>  - mirror-tunnel-addr: BDF string of the tunnel device.
>>>
>>> ovs-vsctl test change because new mirroring parameters are
>> introduced
 in
>> this patch
>>
>> It would help to provide examples of usage of these new parameters.
>>
>>> Will add examples in the new patches
>>>
>>> Create a defer procedure call thread to handle all mirror offload
 requests.
>>> This is a light-weight thread which remains in sleep-state when there
>> is
 no
>> new request.
>>> This is created between ovs-vsctl and mirror offloading back end
>>>
>>> Implementing DPDK tx-burst (VIRTIO ingress traffic
>>> mirror) and rx-burst (VIRTIO egress traffic mirror) callbacks.
>>> Each callback  functions implement the following tasks:
>>>  1. Enable per-packet VLAN insertion
>>>- for port mirroring, all packets are enabled per-packet VLAN
>> insertion.
>>>- for flow mirroring, only packet header matches the required mac
>> address
>>>  are enabled.
>>>  2. Sending the packets to the specified transport port (output-port in
>>> mirror offload configuration)
>>>- for port mirroring, all packets are sent to the transport port.
>>>- for flow mirroring, only matched packets are sent.
>>>  3. Restore each packet attributes (remove DPDK per-packet offload
 flag)
>>
>> I will for sure have more questions later, but please find a few
>> comments/questions below:
>>
>>> Signed-off-by: Liang-min Wang 
>>> Tested-by: Timothy Miskell 
>>> Suggested-by: Munish Mehan 
>>> ---
>>>  lib/automake.mk|   2 +
>>>  lib/netdev-dpdk-mirror.c   | 516
>> +
>>>  lib/netdev-dpdk-mirror.h   |  83 ++
>>>  lib/netdev-dpdk.c  | 397 
>>>  lib/netdev-provider.h  |  16 ++
>>>  lib/netdev.c   | 386 +++
>>>  lib/netdev.h   |  16 ++
>>>  tests/ovs-vsctl.at |   2 +
>>>  vswitchd/bridge.c  | 271 ++-
>>>  vswitchd/vswitch.ovsschema |  24 +-
>>>  vswitchd/vswitch.xml   |  50 
>>>  11 files changed, 1759 insertions(+), 4 deletions(-)
>>>  create mode 100644 lib/netdev-dpdk-mirror.c
>>>  create mode 100644 lib/netdev-dpdk-mirror.h
>>>

 ...

>>> +
>>> +/* port/flow mirror traffic processors */
>>> +static inline uint16_t
>>> +netdev_custom_mirror_offload_cb(uint16_t qidx, struct rte_mbuf
>> **pkts,

Re: [ovs-dev] Moving of the primary #openvswitch channel to irc.libera.chat ?

[Ben Pfaff]

On Wed, May 19, 2021 at 10:03:57PM +0200, Ilya Maximets wrote:
> Hi.
> 
> Taking into account some very unhealthy things that happened recently
> with FreeNode network and resigning of lots of its stuff [1], we
> probably need to discuss if Open vSwitch project wants to change the
> IRC server for a primary #openvswitch channel.  User's data is the
> main concern, IIUC, as it's unclear what the new management will
> do with the network.
> 
> The main alternative now seems to be the Libera.Chat [2] where most of
> the former FreeNode stuff.
> 
> Some projects already announced [3][4] movement to Libera.Chat.  Others
> are discussing the possibility [5].
> 
> So, I think, it make sense to discuss the future of #openvswitch
> channel too.  Any thoughts?
> 
> Will we have an OVN meeting on a different server tomorrow?

Thanks for sending such a detailed message (with footnotes!).
I think I support the move.  I have already registered the #openvswitch
channel on libera.chat (and copied the topic across).

I'm going to log in on both servers tomorrow but I suggest that we
transition to libera.chat after tomorrow's meeting, since this is pretty
short notice and I think that most of us (including me) are only light
users of IRC.

Also, I suspect that the infrastructure for recording meetings has not
yet moved to libera.chat.  (I did notice that the 'openstack' user just
dropped out of #openvswitch.  I think that's the bot that did the
meeting recordings.  So maybe that infastructure is moving across right
as I write.)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVN 21.06 soft freeze May 7

[Mark Michelson]

Hi Ihar,

I floated this by the OVN team in #coreovn and got positive responses. 
So I think this is a candidate.


On 5/18/21 11:40 AM, Ihar Hrachyshka wrote:

Is it already time to request exceptions for freeze?

I would like the following bug fixes incorporated in 21.06, all
related to the new allow-stateless ACL:
https://patchwork.ozlabs.org/project/ovn/list/?series=244288

They are a bit involving but I tried to cover each with some test
scenarios proving the fixes.

Thanks,
Ihar

On Mon, Apr 26, 2021 at 11:57 AM Mark Michelson  wrote:


Hi everyone,

This is a 2 week (-ish) warning that OVN is approaching soft freeze for
the 21.06 branch. As a reminder, soft freeze is the deadline for new
features to be posted for review. After that, there is a two week period
where we can review features and get them integrated. We will then
create the 21.06 branch on May 21.

If you have new OVN features that you would like to get integrated for
the 21.06.0 release, please do your best to have the reviews posted by
May 7.

Thanks,
Mark Michelson

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev





___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] netdev-linux.c : nl_sock_listen_all_nsid triggers NULL deref.

[0-day Robot]

Bleep bloop.  Greetings lin huang, 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:
ERROR: Author lin huang  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: miter 
Lines checked: 30, Warnings: 1, Errors: 1


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] [PATCH ovn 0/3] Introduce check_pkt_len for ingress traffic

[Lorenzo Bianconi]

> On Wed, 2021-05-19 at 17:35 +0200, Lorenzo Bianconi wrote:
> > In the current codebase, check_pkt_len is applied just for traffic
> > leaving the
> > ovn cluster. This series introduces the same capability for traffic
> > entering
> > the network from a gateway router or distributed gateway router port
> > in order
> > to send an ICMP error packet if the frame size is greater than the
> > configured
> > MTU.
> > 
> > Lorenzo Bianconi (3):
> >   northd: introduce build_check_pkt_len_flows_for_lrp routine
> >   northd: enable check_pkt_len for gw router
> >   northd: add chec_pkt_len lflows for ingress traffic
> 
> I know I said "check_pkt_len" a lot when talking about it internally,
> but the commit messages should probably reference check_pkt_larger()
> right?

yes, correct. I mixed the ovn action and the kernel one, I will fix it in v2.
Thanks.

> 
> Also small nit, commit message for last patch is missing a "k" in
> check.

ack, I will fix it in v2.

Regards,
Lorenzo

> 
> Thanks,
> Dan
> 
> > 
> >  northd/ovn-northd.c | 236 +++--
> >  tests/ovn-northd.at | 150 +++---
> >  tests/ovn.at    | 492
> > ++--
> >  3 files changed, 682 insertions(+), 196 deletions(-)
> > 
> 
> 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn 0/3] Introduce check_pkt_len for ingress traffic

[Dan Williams]

On Wed, 2021-05-19 at 17:35 +0200, Lorenzo Bianconi wrote:
> In the current codebase, check_pkt_len is applied just for traffic
> leaving the
> ovn cluster. This series introduces the same capability for traffic
> entering
> the network from a gateway router or distributed gateway router port
> in order
> to send an ICMP error packet if the frame size is greater than the
> configured
> MTU.
> 
> Lorenzo Bianconi (3):
>   northd: introduce build_check_pkt_len_flows_for_lrp routine
>   northd: enable check_pkt_len for gw router
>   northd: add chec_pkt_len lflows for ingress traffic

I know I said "check_pkt_len" a lot when talking about it internally,
but the commit messages should probably reference check_pkt_larger()
right?

Also small nit, commit message for last patch is missing a "k" in
check.

Thanks,
Dan

> 
>  northd/ovn-northd.c | 236 +++--
>  tests/ovn-northd.at | 150 +++---
>  tests/ovn.at    | 492
> ++--
>  3 files changed, 682 insertions(+), 196 deletions(-)
> 


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpif-netdev.c : Fix indentation.

[0-day Robot]

Bleep bloop.  Greetings lin huang, 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:
ERROR: Author lin huang  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: miter 
Lines checked: 29, Warnings: 1, Errors: 1


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] netdev-linux.c : nl_sock_listen_all_nsid triggers NULL deref.

[lin huang]

netdev-linux.c : nl_sock_listen_all_nsid triggers NULL deref.

Signed-off-by: miter 
---
 lib/netdev-linux.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 15b25084b..0994044ec 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -634,7 +634,9 @@ netdev_linux_notify_sock(void)
 }
 }
 }
-nl_sock_listen_all_nsid(sock, true);
+if (sock) {
+nl_sock_listen_all_nsid(sock, true);
+ }
 ovsthread_once_done();
 }

--
2.31.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [v12 01/16] dpif-netdev: Refactor to multiple header files.

[Stokes, Ian]

> Split the very large file dpif-netdev.c and the datastructures
> it contains into multiple header files. Each header file is
> responsible for the datastructures of that component.
> 
> This logical split allows better reuse and modularity of the code,
> and reduces the very large file dpif-netdev.c to be more managable.
> 
> Due to dependencies between components, it is not possible to
> move component in smaller granularities than this patch.
> 
> To explain the dependencies better, eg:
> 
> DPCLS has no deps (from dpif-netdev.c file)
> FLOW depends on DPCLS (struct dpcls_rule)
> DFC depends on DPCLS (netdev_flow_key) and FLOW (netdev_flow_key)
> THREAD depends on DFC (struct dfc_cache)
> 
> DFC_PROC depends on THREAD (struct pmd_thread)
> 
> DPCLS lookup.h/c require only DPCLS
> DPCLS implementations require only dpif-netdev-lookup.h.
> - This change was made in 2.12 release with function pointers
> - This commit only refactors the name to "private-dpcls.h"
> 
> Signed-off-by: Harry van Haaren 
> Co-authored-by: Cian Ferriter 
> Signed-off-by: Cian Ferriter 

Hi Cian/Harry,

Thanks for the patch.

One risk I think we need to flag/discuss further  with the community is the 
impact the refactor will have on other patches in progress and that is best 
dealt with.

In an effort to help with this I may Cc folks who have patches that would be 
affected.

Best to come with a plan WRT integration order because although functionally 
little changes here there would be a wider impact to ongoing patches.

I have a few queries below inline below.

> ---
>  lib/automake.mk|   4 +
>  lib/dpif-netdev-lookup-autovalidator.c |   1 -
>  lib/dpif-netdev-lookup-avx512-gather.c |   1 -
>  lib/dpif-netdev-lookup-generic.c   |   1 -
>  lib/dpif-netdev-lookup.h   |   2 +-
>  lib/dpif-netdev-private-dfc.h  | 244 
>  lib/dpif-netdev-private-dpcls.h| 129 ++
>  lib/dpif-netdev-private-flow.h | 162 
>  lib/dpif-netdev-private-thread.h   | 206 ++
>  lib/dpif-netdev-private.h  | 100 +
>  lib/dpif-netdev.c  | 519 +
>  11 files changed, 760 insertions(+), 609 deletions(-)
>  create mode 100644 lib/dpif-netdev-private-dfc.h
>  create mode 100644 lib/dpif-netdev-private-dpcls.h
>  create mode 100644 lib/dpif-netdev-private-flow.h
>  create mode 100644 lib/dpif-netdev-private-thread.h
> 
> diff --git a/lib/automake.mk b/lib/automake.mk
> index 39901bd6d..9fa8712c3 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -111,6 +111,10 @@ lib_libopenvswitch_la_SOURCES = \
>   lib/dpif-netdev-lookup-generic.c \
>   lib/dpif-netdev.c \
>   lib/dpif-netdev.h \
> + lib/dpif-netdev-private-dfc.h \
> + lib/dpif-netdev-private-dpcls.h \
> + lib/dpif-netdev-private-flow.h \
> + lib/dpif-netdev-private-thread.h \
>   lib/dpif-netdev-private.h \
>   lib/dpif-netdev-perf.c \
>   lib/dpif-netdev-perf.h \
> diff --git a/lib/dpif-netdev-lookup-autovalidator.c b/lib/dpif-netdev-lookup-
> autovalidator.c
> index 97b59fdd0..475e1ab1e 100644
> --- a/lib/dpif-netdev-lookup-autovalidator.c
> +++ b/lib/dpif-netdev-lookup-autovalidator.c
> @@ -17,7 +17,6 @@
>  #include 
>  #include "dpif-netdev.h"
>  #include "dpif-netdev-lookup.h"
> -#include "dpif-netdev-private.h"
>  #include "openvswitch/vlog.h"
> 
>  VLOG_DEFINE_THIS_MODULE(dpif_lookup_autovalidator);
> diff --git a/lib/dpif-netdev-lookup-avx512-gather.c b/lib/dpif-netdev-lookup-
> avx512-gather.c
> index 5e3634249..8fc1cdfa5 100644
> --- a/lib/dpif-netdev-lookup-avx512-gather.c
> +++ b/lib/dpif-netdev-lookup-avx512-gather.c
> @@ -21,7 +21,6 @@
> 
>  #include "dpif-netdev.h"
>  #include "dpif-netdev-lookup.h"
> -#include "dpif-netdev-private.h"
>  #include "cmap.h"
>  #include "flow.h"
>  #include "pvector.h"
> diff --git a/lib/dpif-netdev-lookup-generic.c b/lib/dpif-netdev-lookup-
> generic.c
> index b1a0cfc36..e3b6be4b6 100644
> --- a/lib/dpif-netdev-lookup-generic.c
> +++ b/lib/dpif-netdev-lookup-generic.c
> @@ -17,7 +17,6 @@
> 
>  #include 
>  #include "dpif-netdev.h"
> -#include "dpif-netdev-private.h"
>  #include "dpif-netdev-lookup.h"
> 
>  #include "bitmap.h"
> diff --git a/lib/dpif-netdev-lookup.h b/lib/dpif-netdev-lookup.h
> index bd72aa29b..59f51faa0 100644
> --- a/lib/dpif-netdev-lookup.h
> +++ b/lib/dpif-netdev-lookup.h
> @@ -19,7 +19,7 @@
> 
>  #include 
>  #include "dpif-netdev.h"
> -#include "dpif-netdev-private.h"
> +#include "dpif-netdev-private-dpcls.h"
> 
>  /* Function to perform a probe for the subtable bit fingerprint.
>   * Returns NULL if not valid, or a valid function pointer to call for this
> diff --git a/lib/dpif-netdev-private-dfc.h b/lib/dpif-netdev-private-dfc.h
> new file mode 100644
> index 0..52349a3fc
> --- /dev/null
> +++ b/lib/dpif-netdev-private-dfc.h
> @@ -0,0 +1,244 @@
> +/*
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2015 Nicira, Inc.
> + * 

[ovs-dev] [PATCH] dpif-netdev.c : Fix indentation.

[lin huang]

dpif-netdev.c : Fix indentation.
Add extra space to fix indentation.

Signed-off-by: miter 
---
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 816945375..dc77fa2fa 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -7404,7 +7404,7 @@ dp_netdev_input__(struct dp_netdev_pmd_thread *pmd,
 enum { PKT_ARRAY_SIZE = NETDEV_MAX_BURST };
 #endif
 OVS_ALIGNED_VAR(CACHE_LINE_SIZE)
-struct netdev_flow_key keys[PKT_ARRAY_SIZE];
+struct netdev_flow_key keys[PKT_ARRAY_SIZE];
 struct netdev_flow_key *missed_keys[PKT_ARRAY_SIZE];
 struct packet_batch_per_flow batches[PKT_ARRAY_SIZE];
 size_t n_batches;
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH ovn 2/3] northd: enable check_pkt_len for gw router

[Lorenzo Bianconi]

As it is already done for distributed gw router scenario, introduce
check_pkt_len logical flows for gw router use case.

Signed-off-by: Lorenzo Bianconi 
---
 northd/ovn-northd.c |  31 --
 tests/ovn.at| 238 
 2 files changed, 260 insertions(+), 9 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index a642d9020..ec63eb75c 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -10511,17 +10511,30 @@ build_check_pkt_len_flows_for_lrouter(
 struct hmap *ports,
 struct ds *match, struct ds *actions)
 {
-if (od->nbr) {
+if (!od->nbr) {
+return;
+}
 
-/* Packets are allowed by default. */
-ovn_lflow_add(lflows, od, S_ROUTER_IN_CHK_PKT_LEN, 0, "1",
-  "next;");
-ovn_lflow_add(lflows, od, S_ROUTER_IN_LARGER_PKTS, 0, "1",
-  "next;");
+/* Packets are allowed by default. */
+ovn_lflow_add(lflows, od, S_ROUTER_IN_CHK_PKT_LEN, 0, "1",
+  "next;");
+ovn_lflow_add(lflows, od, S_ROUTER_IN_LARGER_PKTS, 0, "1",
+  "next;");
 
-if (od->l3dgw_port && od->l3redirect_port) {
-build_check_pkt_len_flows_for_lrp(od->l3dgw_port, lflows,
-  ports, match, actions);
+if (od->l3dgw_port && od->l3redirect_port) {
+/* gw router port */
+build_check_pkt_len_flows_for_lrp(od->l3dgw_port, lflows,
+  ports, match, actions);
+} else if (smap_get(>nbr->options, "chassis")) {
+for (size_t i = 0; i < od->nbr->n_ports; i++) {
+/* gw router */
+struct ovn_port *rp = ovn_port_find(ports,
+od->nbr->ports[i]->name);
+if (!rp) {
+continue;
+}
+build_check_pkt_len_flows_for_lrp(rp, lflows, ports, match,
+  actions);
 }
 }
 }
diff --git a/tests/ovn.at b/tests/ovn.at
index db877cb7e..248319262 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -16375,6 +16375,244 @@ OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn -- gw router - check packet length - icmp defrag])
+AT_KEYWORDS([gwr-check_packet_length])
+ovn_start
+
+ovn-nbctl ls-add sw0
+ovn-nbctl lsp-add sw0 sw0-port1
+ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01 10.0.0.3 1000::3"
+
+ovn-nbctl create Logical_Router name=lr0 options:chassis="hv1"
+ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 1000::1/64
+ovn-nbctl lsp-add sw0 sw0-lr0
+ovn-nbctl lsp-set-type sw0-lr0 router
+ovn-nbctl lsp-set-addresses sw0-lr0 router
+ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0
+
+ovn-nbctl ls-add public
+ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 172.168.0.100/24 2000::1/64
+ovn-nbctl lsp-add public public-lr0
+ovn-nbctl lsp-set-type public-lr0 router
+ovn-nbctl lsp-set-addresses public-lr0 router
+ovn-nbctl lsp-set-options public-lr0 router-port=lr0-public
+
+# localnet port
+ovn-nbctl lsp-add public ln-public
+ovn-nbctl lsp-set-type ln-public localnet
+ovn-nbctl lsp-set-addresses ln-public unknown
+ovn-nbctl lsp-set-options ln-public network_name=phys
+
+ovn-nbctl lr-nat-add lr0 snat 172.168.0.100 10.0.0.0/24
+ovn-nbctl lr-nat-add lr0 snat 2000::1 1000::/64
+
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
+ovs-vsctl -- add-port br-int hv1-vif1 -- \
+set interface hv1-vif1 external-ids:iface-id=sw0-port1 \
+options:tx_pcap=hv1/vif1-tx.pcap \
+options:rxq_pcap=hv1/vif1-rx.pcap \
+ofport-request=1
+
+reset_pcap_file() {
+ local iface=$1
+ local pcap_file=$2
+ ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \
+ options:rxq_pcap=dummy-rx.pcap
+ rm -f ${pcap_file}*.pcap
+ ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \
+ options:rxq_pcap=${pcap_file}-rx.pcap
+}
+
+test_ip_packet_larger() {
+local mtu=$1
+
+# Send ip packet from sw0-port1 to outside
+src_mac="50540001" # sw-port1 mac
+dst_mac="ff01" # sw0-lr0 mac (internal router leg)
+src_ip=`ip_to_hex 10 0 0 3`
+dst_ip=`ip_to_hex 172 168 0 3`
+# Set the packet length to 118.
+pkt_len=0076
+packet=${dst_mac}${src_mac}08004500${pkt_len}4001c3d9
+orig_packet_l3=${src_ip}${dst_ip}0304
+orig_packet_l3=${orig_packet_l3}
+orig_packet_l3=${orig_packet_l3}
+orig_packet_l3=${orig_packet_l3}
+orig_packet_l3=${orig_packet_l3}
+orig_packet_l3=${orig_packet_l3}
+
+packet=${packet}${orig_packet_l3}
+
+

[ovs-dev] [PATCH ovn 3/3] northd: add chec_pkt_len lflows for ingress traffic

[Lorenzo Bianconi]

Introduce chec_pkt_len action for ingress traffic entering the cluster
from a distributed gw router port or from a gw router.
Rearrange logical router ingress pipeline in order to properly manage
icmp error packets.

Signed-off-by: Lorenzo Bianconi 
---
 northd/ovn-northd.c | 154 +-
 tests/ovn-northd.at | 150 +-
 tests/ovn.at| 258 
 3 files changed, 394 insertions(+), 168 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index ec63eb75c..95fcee69d 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -183,18 +183,18 @@ enum ovn_stage {
 PIPELINE_STAGE(ROUTER, IN,  LEARN_NEIGHBOR,  2, "lr_in_learn_neighbor") \
 PIPELINE_STAGE(ROUTER, IN,  IP_INPUT,3, "lr_in_ip_input") \
 PIPELINE_STAGE(ROUTER, IN,  DEFRAG,  4, "lr_in_defrag")   \
-PIPELINE_STAGE(ROUTER, IN,  UNSNAT,  5, "lr_in_unsnat")   \
-PIPELINE_STAGE(ROUTER, IN,  DNAT,6, "lr_in_dnat") \
-PIPELINE_STAGE(ROUTER, IN,  ECMP_STATEFUL,   7, "lr_in_ecmp_stateful") \
-PIPELINE_STAGE(ROUTER, IN,  ND_RA_OPTIONS,   8, "lr_in_nd_ra_options") \
-PIPELINE_STAGE(ROUTER, IN,  ND_RA_RESPONSE,  9, "lr_in_nd_ra_response") \
-PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING,  10, "lr_in_ip_routing")   \
-PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING_ECMP, 11, "lr_in_ip_routing_ecmp") \
-PIPELINE_STAGE(ROUTER, IN,  POLICY,  12, "lr_in_policy")   \
-PIPELINE_STAGE(ROUTER, IN,  POLICY_ECMP, 13, "lr_in_policy_ecmp")  \
-PIPELINE_STAGE(ROUTER, IN,  ARP_RESOLVE, 14, "lr_in_arp_resolve")  \
-PIPELINE_STAGE(ROUTER, IN,  CHK_PKT_LEN   ,  15, "lr_in_chk_pkt_len")  \
-PIPELINE_STAGE(ROUTER, IN,  LARGER_PKTS, 16, "lr_in_larger_pkts")  \
+PIPELINE_STAGE(ROUTER, IN,  CHK_PKT_LEN, 5, "lr_in_chk_pkt_len")  \
+PIPELINE_STAGE(ROUTER, IN,  LARGER_PKTS, 6, "lr_in_larger_pkts")  \
+PIPELINE_STAGE(ROUTER, IN,  UNSNAT,  7, "lr_in_unsnat")   \
+PIPELINE_STAGE(ROUTER, IN,  DNAT,8, "lr_in_dnat") \
+PIPELINE_STAGE(ROUTER, IN,  ECMP_STATEFUL,   9, "lr_in_ecmp_stateful") \
+PIPELINE_STAGE(ROUTER, IN,  ND_RA_OPTIONS,   10, "lr_in_nd_ra_options") \
+PIPELINE_STAGE(ROUTER, IN,  ND_RA_RESPONSE,  11, "lr_in_nd_ra_response") \
+PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING,  12, "lr_in_ip_routing")   \
+PIPELINE_STAGE(ROUTER, IN,  IP_ROUTING_ECMP, 13, "lr_in_ip_routing_ecmp") \
+PIPELINE_STAGE(ROUTER, IN,  POLICY,  14, "lr_in_policy")   \
+PIPELINE_STAGE(ROUTER, IN,  POLICY_ECMP, 15, "lr_in_policy_ecmp")  \
+PIPELINE_STAGE(ROUTER, IN,  ARP_RESOLVE, 16, "lr_in_arp_resolve")  \
 PIPELINE_STAGE(ROUTER, IN,  GW_REDIRECT, 17, "lr_in_gw_redirect")  \
 PIPELINE_STAGE(ROUTER, IN,  ARP_REQUEST, 18, "lr_in_arp_request")  \
   \
@@ -10399,6 +10399,67 @@ build_arp_resolve_flows_for_lrouter_port(
 
 }
 
+static void
+build_icmperr_pkt_big_flows(struct ovn_port *op, int mtu, struct hmap *lflows,
+struct ds *match, struct ds *actions)
+{
+if (op->lrp_networks.ipv4_addrs) {
+ds_clear(match);
+ds_put_format(match, "inport == %s && ip4 && "REGBIT_PKT_LARGER,
+  op->json_key);
+
+ds_clear(actions);
+/* Set icmp4.frag_mtu to gw_mtu */
+ds_put_format(actions,
+"icmp4_error {"
+REGBIT_EGRESS_LOOPBACK" = 1; "
+REGBIT_PKT_LARGER" = 0; "
+"eth.dst = %s; "
+"ip4.dst = ip4.src; "
+"ip4.src = %s; "
+"ip.ttl = 255; "
+"icmp4.type = 3; /* Destination Unreachable. */ "
+"icmp4.code = 4; /* Frag Needed and DF was Set. */ "
+"icmp4.frag_mtu = %d; "
+"next(pipeline=ingress, table=%d); };",
+op->lrp_networks.ea_s,
+op->lrp_networks.ipv4_addrs[0].addr_s,
+mtu, ovn_stage_get_table(S_ROUTER_IN_ADMISSION));
+ovn_lflow_add_with_hint(lflows, op->od,
+S_ROUTER_IN_LARGER_PKTS, 50,
+ds_cstr(match), ds_cstr(actions),
+>nbrp->header_);
+}
+
+if (op->lrp_networks.ipv6_addrs) {
+ds_clear(match);
+ds_put_format(match, "inport == %s&& ip6 && "REGBIT_PKT_LARGER,
+  op->json_key);
+
+ds_clear(actions);
+/* Set icmp6.frag_mtu to gw_mtu */
+ds_put_format(actions,
+"icmp6_error {"
+REGBIT_EGRESS_LOOPBACK" = 1; "
+REGBIT_PKT_LARGER" = 0; "
+"eth.dst = %s; "
+"ip6.dst = ip6.src; "
+"ip6.src = %s; "
+"ip.ttl = 255; "
+"icmp6.type = 2; /* Packet Too Big. */ "
+"icmp6.code = 0; "
+

[ovs-dev] [PATCH ovn 1/3] northd: introduce build_check_pkt_len_flows_for_lrp routine

[Lorenzo Bianconi]

Introduce build_check_pkt_len_flows_for_lrp routine to configure
check_pkt_len logical flow for a given logical port. This is a
preliminary patch to enable check_pkt_len support for gw router use
case.

Signed-off-by: Lorenzo Bianconi 
---
 northd/ovn-northd.c | 181 +++-
 1 file changed, 95 insertions(+), 86 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 0e5092a87..a642d9020 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -10399,6 +10399,99 @@ build_arp_resolve_flows_for_lrouter_port(
 
 }
 
+static void
+build_check_pkt_len_flows_for_lrp(struct ovn_port *op,
+  struct hmap *lflows, struct hmap *ports,
+  struct ds *match, struct ds *actions)
+{
+int gw_mtu = 0;
+
+if (op->nbrp) {
+ gw_mtu = smap_get_int(>nbrp->options, "gateway_mtu", 0);
+}
+/* Add the flows only if gateway_mtu is configured. */
+if (gw_mtu <= 0) {
+return;
+}
+
+ds_clear(match);
+ds_put_format(match, "outport == %s", op->json_key);
+
+ds_clear(actions);
+ds_put_format(actions,
+  REGBIT_PKT_LARGER" = check_pkt_larger(%d);"
+  " next;", gw_mtu + VLAN_ETH_HEADER_LEN);
+ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_CHK_PKT_LEN, 50,
+ds_cstr(match), ds_cstr(actions),
+>nbrp->header_);
+
+for (size_t i = 0; i < op->od->nbr->n_ports; i++) {
+struct ovn_port *rp = ovn_port_find(ports,
+op->od->nbr->ports[i]->name);
+if (!rp || rp == op) {
+continue;
+}
+
+if (rp->lrp_networks.ipv4_addrs) {
+ds_clear(match);
+ds_put_format(match, "inport == %s && outport == %s"
+  " && ip4 && "REGBIT_PKT_LARGER,
+  rp->json_key, op->json_key);
+
+ds_clear(actions);
+/* Set icmp4.frag_mtu to gw_mtu */
+ds_put_format(actions,
+"icmp4_error {"
+REGBIT_EGRESS_LOOPBACK" = 1; "
+"eth.dst = %s; "
+"ip4.dst = ip4.src; "
+"ip4.src = %s; "
+"ip.ttl = 255; "
+"icmp4.type = 3; /* Destination Unreachable. */ "
+"icmp4.code = 4; /* Frag Needed and DF was Set. */ "
+"icmp4.frag_mtu = %d; "
+"next(pipeline=ingress, table=%d); };",
+rp->lrp_networks.ea_s,
+rp->lrp_networks.ipv4_addrs[0].addr_s,
+gw_mtu,
+ovn_stage_get_table(S_ROUTER_IN_ADMISSION));
+ovn_lflow_add_with_hint(lflows, op->od,
+S_ROUTER_IN_LARGER_PKTS, 50,
+ds_cstr(match), ds_cstr(actions),
+>nbrp->header_);
+}
+
+if (rp->lrp_networks.ipv6_addrs) {
+ds_clear(match);
+ds_put_format(match, "inport == %s && outport == %s"
+  " && ip6 && "REGBIT_PKT_LARGER,
+  rp->json_key, op->json_key);
+
+ds_clear(actions);
+/* Set icmp6.frag_mtu to gw_mtu */
+ds_put_format(actions,
+"icmp6_error {"
+REGBIT_EGRESS_LOOPBACK" = 1; "
+"eth.dst = %s; "
+"ip6.dst = ip6.src; "
+"ip6.src = %s; "
+"ip.ttl = 255; "
+"icmp6.type = 2; /* Packet Too Big. */ "
+"icmp6.code = 0; "
+"icmp6.frag_mtu = %d; "
+"next(pipeline=ingress, table=%d); };",
+rp->lrp_networks.ea_s,
+rp->lrp_networks.ipv6_addrs[0].addr_s,
+gw_mtu,
+ovn_stage_get_table(S_ROUTER_IN_ADMISSION));
+ovn_lflow_add_with_hint(lflows, op->od,
+S_ROUTER_IN_LARGER_PKTS, 50,
+ds_cstr(match), ds_cstr(actions),
+>nbrp->header_);
+}
+}
+}
+
 /* Local router ingress table CHK_PKT_LEN: Check packet length.
  *
  * Any IPv4 packet with outport set to the distributed gateway
@@ -10427,92 +10520,8 @@ build_check_pkt_len_flows_for_lrouter(
   "next;");
 
 if (od->l3dgw_port && od->l3redirect_port) {
-int gw_mtu = 0;
-if (od->l3dgw_port->nbrp) {
- gw_mtu = smap_get_int(>l3dgw_port->nbrp->options,
-   "gateway_mtu", 0);
-}
-/* Add the flows only if gateway_mtu is configured. */
-if (gw_mtu <= 0) {
-return;
-}
-
-ds_clear(match);
-ds_put_format(match, "outport == %s", od->l3dgw_port->json_key);
-
-

[ovs-dev] [PATCH ovn 0/3] Introduce check_pkt_len for ingress traffic

[Lorenzo Bianconi]

In the current codebase, check_pkt_len is applied just for traffic leaving the
ovn cluster. This series introduces the same capability for traffic entering
the network from a gateway router or distributed gateway router port in order
to send an ICMP error packet if the frame size is greater than the configured
MTU.

Lorenzo Bianconi (3):
  northd: introduce build_check_pkt_len_flows_for_lrp routine
  northd: enable check_pkt_len for gw router
  northd: add chec_pkt_len lflows for ingress traffic

 northd/ovn-northd.c | 236 +++--
 tests/ovn-northd.at | 150 +++---
 tests/ovn.at| 492 ++--
 3 files changed, 682 insertions(+), 196 deletions(-)

-- 
2.31.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] controller: Add support for PTR DNS requests.

[Numan Siddique]

On Mon, May 17, 2021 at 10:01 PM  wrote:
>
> From: Vladislav Odintsov 
>
> The native OVN DNS support doesn't yet support for PTR DNS requests.
> This patch adds the support for it.  If suppose there is a dns record
> as - "vm1.ovn.org"="10.0.0.4", then a normal DNS request will query for
> "vm1.ovn.org" and the reply will be the IP address - 10.0.0.4.
> PTR DNS request helps in getting the domain name of the IP address.
> For the above example, the PTR DNS request will have a query name as
> - "4.0.0.10.in-addr.arpa".  And the response will have "vm1.ovn.org".
> In order to support this feature, this patch expects the CMS to define
> an another entry in the DNS record  as - 
> "4.0.0.10.in-addr.arpa"="vm1.ovn.org".
>
> This makes the job of ovn-controller easier to support this feature.
>
> Submitted-at: https://github.com/ovn-org/ovn/pull/74
> Signed-off-by: Vladislav Odintsov 

Hi Vladislav,

Thanks for adding this feature.  I applied this patch to master.

Sorry that the PR was up for a long time.

Regards
Numan

> ---
>  NEWS |   1 +
>  controller/pinctrl.c | 181 +++
>  lib/ovn-l7.h |   8 ++
>  ovn-nb.xml   |   6 ++
>  tests/ovn.at |  83 +++-
>  5 files changed, 229 insertions(+), 50 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 1d3603269..f96ed73f0 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -18,6 +18,7 @@ Post-v21.03.0
>  datapath flows with this field used.
>- Introduce a new "allow-stateless" ACL verb to always bypass connection
>  tracking. The existing "allow" verb behavior is left intact.
> +  - Added support in native DNS to respond to PTR request types.
>
>  OVN v21.03.0 - 12 Mar 2021
>  -
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 523a45b9a..01cbbacfd 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -2698,6 +2698,106 @@ destroy_dns_cache(void)
>  }
>  }
>
> +/* Populates dns_answer struct with base data.
> + * Copy the answer section
> + * Format of the answer section is
> + *  - NAME -> The domain name
> + *  - TYPE -> 2 octets containing one of the RR type codes
> + *  - CLASS-> 2 octets which specify the class of the data
> + *in the RDATA field.
> + *  - TTL  -> 32 bit unsigned int specifying the time
> + *interval (in secs) that the resource record
> + * may be cached before it should be discarded.
> + *  - RDLENGTH -> 16 bit integer specifying the length of the
> + *RDATA field.
> + *  - RDATA-> a variable length string of octets that
> + *describes the resource.
> + */
> +static void
> +dns_build_base_answer(
> +struct ofpbuf *dns_answer, const uint8_t *in_queryname,
> +uint16_t query_length, int query_type)
> +{
> +ofpbuf_put(dns_answer, in_queryname, query_length);
> +put_be16(dns_answer, htons(query_type));
> +put_be16(dns_answer, htons(DNS_CLASS_IN));
> +put_be32(dns_answer, htonl(DNS_DEFAULT_RR_TTL));
> +}
> +
> +/* Populates dns_answer struct with a TYPE A answer. */
> +static void
> +dns_build_a_answer(
> +struct ofpbuf *dns_answer, const uint8_t *in_queryname,
> +uint16_t query_length, const ovs_be32 addr)
> +{
> +dns_build_base_answer(dns_answer, in_queryname, query_length,
> +  DNS_QUERY_TYPE_A);
> +put_be16(dns_answer, htons(sizeof(ovs_be32)));
> +put_be32(dns_answer, addr);
> +}
> +
> +/* Populates dns_answer struct with a TYPE  answer. */
> +static void
> +dns_build__answer(
> +struct ofpbuf *dns_answer, const uint8_t *in_queryname,
> +uint16_t query_length, const struct in6_addr *addr)
> +{
> +dns_build_base_answer(dns_answer, in_queryname, query_length,
> +  DNS_QUERY_TYPE_);
> +put_be16(dns_answer, htons(sizeof(*addr)));
> +ofpbuf_put(dns_answer, addr, sizeof(*addr));
> +}
> +
> +/* Populates dns_answer struct with a TYPE PTR answer. */
> +static void
> +dns_build_ptr_answer(
> +struct ofpbuf *dns_answer, const uint8_t *in_queryname,
> +uint16_t query_length, const char *answer_data)
> +{
> +char *encoded_answer;
> +uint16_t encoded_answer_length;
> +
> +dns_build_base_answer(dns_answer, in_queryname, query_length,
> +  DNS_QUERY_TYPE_PTR);
> +
> +/* Initialize string 2 chars longer than real answer:
> + * first label length and terminating zero-length label.
> + * If the answer_data is - vm1tst.ovn.org, it will be encoded as
> + *  - 0010 (Total length which is 16)
> + *  - 06766d31747374 (vm1tst)
> + *  - 036f766e (ovn)
> + *  - 036f7267 (org
> + *  - 00 (zero length field) */
> +encoded_answer_length = strlen(answer_data) + 2;
> +encoded_answer = (char *)xzalloc(encoded_answer_length);
> +
> +put_be16(dns_answer, htons(encoded_answer_length));
> +uint8_t label_len_index = 0;
> +  

Re: [ovs-dev] [PATCH] Extends the existing mirror configuration parameters

[Wang, Liang-min]

> -Original Message-
> From: Maxime Coquelin 
> Sent: Wednesday, May 19, 2021 8:50 AM
> To: Wang, Liang-min ; Miskell, Timothy
> ; d...@openvswitch.org
> Subject: Re: [PATCH] Extends the existing mirror configuration parameters
> 
> 
> 
> On 5/19/21 1:53 PM, Wang, Liang-min wrote:
> >> -Original Message-
> >> From: Maxime Coquelin 
> >> Sent: Wednesday, May 19, 2021 3:56 AM
> >> To: Wang, Liang-min ; Miskell, Timothy
> >> ; d...@openvswitch.org
> >> Subject: Re: [PATCH] Extends the existing mirror configuration
> parameters
> >>
> >> Hi Liang-min,
> >>
> >> When replying inline, please do not prefix with ">>" as it is handled as
> >> quoted text. There is no need to prefix.
> >>
> >> On 5/18/21 8:00 PM, Wang, Liang-min wrote:
>  -Original Message-
>  From: Maxime Coquelin 
>  Sent: Tuesday, May 18, 2021 12:15 PM
>  To: Miskell, Timothy ;
> d...@openvswitch.org
>  Cc: Wang, Liang-min 
>  Subject: Re: [PATCH] Extends the existing mirror configuration
> >> parameters
> 
>  Hi Timothy, Liang-min,
> 
>  Thanks for rebasing the patch.
>  A list of delta against the first RFC could help the reviewers.
>  I notice one change in the right direction is the conversion to Vhost
>  API datapath instead of Vhost PMD.
> 
> > In this patch we support both Vhost API and Vhost PMD because OVS
> >> supports both
> > VhostUser and Vdev ports.
> >
>  Also, I would suggest to have the patch split in several incremental
>  patches to ease the review.
> 
> > Thank you for suggestion. We will provide incremental patches on
> next
> >> submission
> >
>  On 5/10/21 6:00 PM, Timothy Miskell wrote:
> > From: Liang-min Wang 
> >
> > The following parameters are added:
> >  - mirror-offload: to turn on/off mirror offloading.
> >  - output-port-name: specify a port, using name string, that is on a
> >> different
> >bridge
> >  - output-src-vlan: output port vlan for each select-src-port.
> >  - output-dst-vlan: output port vlan for each select-dst-port.
> >  - flow-src-mac: use src mac address of each select-dst-port for the
> >> header
> >scan.
> >  - flow-dst-mac: use dst mac address of each select-src-port for the
> >> header
> >scan.
> >  - mirror-tunnel-addr: BDF string of the tunnel device.
> >
> > ovs-vsctl test change because new mirroring parameters are
> introduced
> >> in
>  this patch
> 
>  It would help to provide examples of usage of these new parameters.
> 
> > Will add examples in the new patches
> >
> > Create a defer procedure call thread to handle all mirror offload
> >> requests.
> > This is a light-weight thread which remains in sleep-state when there
> is
> >> no
>  new request.
> > This is created between ovs-vsctl and mirror offloading back end
> >
> > Implementing DPDK tx-burst (VIRTIO ingress traffic
> > mirror) and rx-burst (VIRTIO egress traffic mirror) callbacks.
> > Each callback  functions implement the following tasks:
> >  1. Enable per-packet VLAN insertion
> >- for port mirroring, all packets are enabled per-packet VLAN
> insertion.
> >- for flow mirroring, only packet header matches the required mac
>  address
> >  are enabled.
> >  2. Sending the packets to the specified transport port (output-port in
> > mirror offload configuration)
> >- for port mirroring, all packets are sent to the transport port.
> >- for flow mirroring, only matched packets are sent.
> >  3. Restore each packet attributes (remove DPDK per-packet offload
> >> flag)
> 
>  I will for sure have more questions later, but please find a few
>  comments/questions below:
> 
> > Signed-off-by: Liang-min Wang 
> > Tested-by: Timothy Miskell 
> > Suggested-by: Munish Mehan 
> > ---
> >  lib/automake.mk|   2 +
> >  lib/netdev-dpdk-mirror.c   | 516
>  +
> >  lib/netdev-dpdk-mirror.h   |  83 ++
> >  lib/netdev-dpdk.c  | 397 
> >  lib/netdev-provider.h  |  16 ++
> >  lib/netdev.c   | 386 +++
> >  lib/netdev.h   |  16 ++
> >  tests/ovs-vsctl.at |   2 +
> >  vswitchd/bridge.c  | 271 ++-
> >  vswitchd/vswitch.ovsschema |  24 +-
> >  vswitchd/vswitch.xml   |  50 
> >  11 files changed, 1759 insertions(+), 4 deletions(-)
> >  create mode 100644 lib/netdev-dpdk-mirror.c
> >  create mode 100644 lib/netdev-dpdk-mirror.h
> >
> >>
> >> ...
> >>
> > +
> > +/* port/flow mirror traffic processors */
> > +static inline uint16_t
> > +netdev_custom_mirror_offload_cb(uint16_t qidx, struct rte_mbuf
>  **pkts,
> > +uint16_t nb_pkts, void *user_params)
> > 

Re: [ovs-dev] [PATCH dpdk-latest 1/6] travis: Switch to dpdk main branch.

[David Marchand]

On Wed, May 19, 2021 at 1:48 PM Stokes, Ian  wrote:
> > > I didn't see anymore comments on above, following he patch sync last
> > week and the discussion had there I'm happy to update the commit
> > message, remove the tested tag and push the series to dpdk-latest.
> >
> > Either leaving as is (since this is a rebase) or rewriting the
> > commitlogs are both fine to me.
> > Thanks!
>
> Thanks David, dpdk-latest is rebased on ovs-master and these are applied.
>
> Travis is in the green
>
> https://travis-ci.org/github/openvswitch/ovs/builds/771661238
>
> We're thinking of looking at updating the github actions ci to maybe handle 
> dpdk-latest also, although this may require some re-design on how we ci 
> currently works.

Argh, I had seen the issue in GHA, but broke my branch when
rebasing/testing with my own dpdk repo.

This could be squashed in the first patch of the dpdk-latest branch:

- .github/workflows/build-and-test.yml -
index e2350c6d9d..6638d1658f 100644
@@ -17,6 +17,7 @@ jobs:
   DEB_PACKAGE: ${{ matrix.deb_package }}
   DPDK:${{ matrix.dpdk }}
   DPDK_SHARED: ${{ matrix.dpdk_shared }}
+  DPDK_VER:refs/heads/main
   KERNEL:  ${{ matrix.kernel }}
   KERNEL_LIST: ${{ matrix.kernel_list }}
   LIBS:${{ matrix.libs }}


-- 
David Marchand

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3] ofp_actions: fix set_mpls_tc formatting

[Ilya Maximets]

On 5/18/21 9:48 AM, Adrian Moreno wrote:
> Apart from a cut-and-paste typo, the man page claims that mpls_labels
> can be provided in hexadecimal format but that's currently not the case.
> 
> Fix mpls ofp-action formatting, add size checks on ofp-action parsing
> and add some unit tests.
> 
> Signed-off-by: Adrian Moreno 
> ---

Thanks!  Applied to master and backported down to 2.12.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpif-netdev.c : Fix indentation.

[0-day Robot]

Bleep bloop.  Greetings lin huang, 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:
ERROR: Author lin huang  needs to sign off.
Lines checked: 31, Warnings: 0, Errors: 1


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] [PATCH v3] ofproto-dpif-xlate: fix zone set from non-frozen-metadata fields

[Ilya Maximets]

On 2/27/21 10:34 AM, Peng He wrote:
> CT zone could be set from a field that is not included in frozen
> metadata. Consider the example rules which are typically seen in
> OpenStack security group rules:
> 
> priority=100,in_port=1,tcp,ct_state=-trk,action=ct(zone=5,table=0)
> priority=100,in_port=1,tcp,ct_state=+trk,action=ct(commit,zone=NXM_NX_CT_ZONE[]),2
> 
> The zone is set from the first rule's ct action. These two rules will
> generate two megaflows: the first one uses zone=5 to query the CT module,
> the second one sets the zone-id from the first megaflow and commit to CT.
> 
> The current implementation will generate a megaflow that does not use
> ct_zone=5 as a match, but directly commit into the ct using zone=5, as zone is
> set by an Imm not a field.
> 
> Consider a situation that one changes the zone id (for example to 15)
> in the first rule, however, still keep the second rule unchanged. During
> this change, there is traffic hitting the two generated megaflows, the
> revaldiator would revalidate all megaflows, however, the revalidator will
> not change the second megaflow, because zone=5 is recorded in the
> megaflow, so the xlate will still translate the commit action into zone=5,
> and the new traffic will still commit to CT as zone=5, not zone=15,
> resulting in taffic drops and other issues.
> 
> Just like OVS set-field convention, if a field X is set by Y
> (Y is a variable not an Imm), we should also mask Y as a match
> in the generated megaflow. An exception is that if the zone-id is
> set by the field that is included in the frozen state (i.e. regs) and this
> upcall is a resume of a thawed xlate, the un-wildcarding can be skipped,
> as the recirc_id is a hash of the values in these fields, and it will change
> following the changes of these fields. When the recirc_id changes,
> all megaflows with the old recirc id will be invalid later.
> 
> Fixes: 07659514c3 ("Add support for connection tracking.")
> Reported-by: Sai Su 
> Signed-off-by: Peng He 
> ---
>  include/openvswitch/meta-flow.h |  1 +
>  lib/meta-flow.c | 13 ++
>  ofproto/ofproto-dpif-xlate.c| 12 +
>  tests/system-traffic.at | 45 +
>  4 files changed, 71 insertions(+)
> 
> diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
> index 95e52e358..045dce8f5 100644
> --- a/include/openvswitch/meta-flow.h
> +++ b/include/openvswitch/meta-flow.h
> @@ -2305,6 +2305,7 @@ void mf_set_flow_value_masked(const struct mf_field *,
>const union mf_value *mask,
>struct flow *);
>  bool mf_is_tun_metadata(const struct mf_field *);
> +bool mf_is_frozen_metadata(const struct mf_field *);
>  bool mf_is_pipeline_field(const struct mf_field *);
>  bool mf_is_set(const struct mf_field *, const struct flow *);
>  void mf_mask_field(const struct mf_field *, struct flow_wildcards *);
> diff --git a/lib/meta-flow.c b/lib/meta-flow.c
> index c808d205d..e03cd8d0c 100644
> --- a/lib/meta-flow.c
> +++ b/lib/meta-flow.c
> @@ -1788,6 +1788,19 @@ mf_is_tun_metadata(const struct mf_field *mf)
> mf->id < MFF_TUN_METADATA0 + TUN_METADATA_NUM_OPTS;
>  }
>  
> +bool
> +mf_is_frozen_metadata(const struct mf_field *mf)
> +{
> +if (mf->id >= MFF_TUN_ID && mf->id <= MFF_IN_PORT_OXM) {
> +return true;
> +}
> +
> +if (mf->id >= MFF_REG0 && mf->id < MFF_ETH_SRC) {
> +return true;
> +}
> +return false;
> +}
> +
>  bool
>  mf_is_pipeline_field(const struct mf_field *mf)
>  {
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 7108c8a30..14d00db1e 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -6195,6 +6195,18 @@ compose_conntrack_action(struct xlate_ctx *ctx, struct 
> ofpact_conntrack *ofc,
>  
>  if (ofc->zone_src.field) {
>  zone = mf_get_subfield(>zone_src, >xin->flow);
> +if (ctx->xin->frozen_state) {
> +/* If the upcall is a resume of a recirculation, we only need to
> + * unwildcard the fields that are not in the frozen_metadata, as
> + * when the rules update, OVS will generate a new recirc_id,
> + * which will invalidate the megaflow with old the recirc_id.
> + */
> +if (!mf_is_frozen_metadata(ofc->zone_src.field)) {
> +mf_mask_field(ofc->zone_src.field, ctx->wc);
> +}
> +} else {
> +mf_mask_field(ofc->zone_src.field, ctx->wc);
> +}

IIUC, in current code above block is equal to just a single line:

mf_mask_field(ofc->zone_src.field, ctx->wc);

That is because zone is not part of a frozen metadata, right?

Can we just remove all this extra logic and unwildcard unconditionally?
I understand that this code might save us one match in case someday
zone will be part of a frozen metadata, but is that really important?
Is there any harm in 

Re: [ovs-dev] [PATCH] Extends the existing mirror configuration parameters

[Maxime Coquelin]

On 5/19/21 1:53 PM, Wang, Liang-min wrote:
>> -Original Message-
>> From: Maxime Coquelin 
>> Sent: Wednesday, May 19, 2021 3:56 AM
>> To: Wang, Liang-min ; Miskell, Timothy
>> ; d...@openvswitch.org
>> Subject: Re: [PATCH] Extends the existing mirror configuration parameters
>>
>> Hi Liang-min,
>>
>> When replying inline, please do not prefix with ">>" as it is handled as
>> quoted text. There is no need to prefix.
>>
>> On 5/18/21 8:00 PM, Wang, Liang-min wrote:
 -Original Message-
 From: Maxime Coquelin 
 Sent: Tuesday, May 18, 2021 12:15 PM
 To: Miskell, Timothy ; d...@openvswitch.org
 Cc: Wang, Liang-min 
 Subject: Re: [PATCH] Extends the existing mirror configuration
>> parameters

 Hi Timothy, Liang-min,

 Thanks for rebasing the patch.
 A list of delta against the first RFC could help the reviewers.
 I notice one change in the right direction is the conversion to Vhost
 API datapath instead of Vhost PMD.

> In this patch we support both Vhost API and Vhost PMD because OVS
>> supports both
> VhostUser and Vdev ports.
>
 Also, I would suggest to have the patch split in several incremental
 patches to ease the review.

> Thank you for suggestion. We will provide incremental patches on next
>> submission
>
 On 5/10/21 6:00 PM, Timothy Miskell wrote:
> From: Liang-min Wang 
>
> The following parameters are added:
>  - mirror-offload: to turn on/off mirror offloading.
>  - output-port-name: specify a port, using name string, that is on a
>> different
>bridge
>  - output-src-vlan: output port vlan for each select-src-port.
>  - output-dst-vlan: output port vlan for each select-dst-port.
>  - flow-src-mac: use src mac address of each select-dst-port for the
>> header
>scan.
>  - flow-dst-mac: use dst mac address of each select-src-port for the
>> header
>scan.
>  - mirror-tunnel-addr: BDF string of the tunnel device.
>
> ovs-vsctl test change because new mirroring parameters are introduced
>> in
 this patch

 It would help to provide examples of usage of these new parameters.

> Will add examples in the new patches
>
> Create a defer procedure call thread to handle all mirror offload
>> requests.
> This is a light-weight thread which remains in sleep-state when there is
>> no
 new request.
> This is created between ovs-vsctl and mirror offloading back end
>
> Implementing DPDK tx-burst (VIRTIO ingress traffic
> mirror) and rx-burst (VIRTIO egress traffic mirror) callbacks.
> Each callback  functions implement the following tasks:
>  1. Enable per-packet VLAN insertion
>- for port mirroring, all packets are enabled per-packet VLAN 
> insertion.
>- for flow mirroring, only packet header matches the required mac
 address
>  are enabled.
>  2. Sending the packets to the specified transport port (output-port in
> mirror offload configuration)
>- for port mirroring, all packets are sent to the transport port.
>- for flow mirroring, only matched packets are sent.
>  3. Restore each packet attributes (remove DPDK per-packet offload
>> flag)

 I will for sure have more questions later, but please find a few
 comments/questions below:

> Signed-off-by: Liang-min Wang 
> Tested-by: Timothy Miskell 
> Suggested-by: Munish Mehan 
> ---
>  lib/automake.mk|   2 +
>  lib/netdev-dpdk-mirror.c   | 516
 +
>  lib/netdev-dpdk-mirror.h   |  83 ++
>  lib/netdev-dpdk.c  | 397 
>  lib/netdev-provider.h  |  16 ++
>  lib/netdev.c   | 386 +++
>  lib/netdev.h   |  16 ++
>  tests/ovs-vsctl.at |   2 +
>  vswitchd/bridge.c  | 271 ++-
>  vswitchd/vswitch.ovsschema |  24 +-
>  vswitchd/vswitch.xml   |  50 
>  11 files changed, 1759 insertions(+), 4 deletions(-)
>  create mode 100644 lib/netdev-dpdk-mirror.c
>  create mode 100644 lib/netdev-dpdk-mirror.h
>
>>
>> ...
>>
> +
> +/* port/flow mirror traffic processors */
> +static inline uint16_t
> +netdev_custom_mirror_offload_cb(uint16_t qidx, struct rte_mbuf
 **pkts,
> +uint16_t nb_pkts, void *user_params)
> +{
> +struct mirror_param *data = user_params;
> +uint16_t i, dst_qidx, match_count = 0;
> +uint16_t pkt_trans;
> +uint16_t dst_port_id = data->dst_port_id;
> +uint16_t dst_vlan_id = data->dst_vlan_id;
> +struct rte_mbuf **pkt_buf = >pkt_buf[qidx * data-
> max_burst_size];
> +
> +if (nb_pkts == 0) {
> +return 0;
> +}
> +
> +if (nb_pkts > data->max_burst_size) {
> +

[ovs-dev] [PATCH] dpif-netdev.c : Fix indentation.

[lin huang]

dpif-netdev.c : Fix indentation.

Add extra space to fix indentation.

Signed-off-by: miter 

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 816945375..dc77fa2fa 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -7404,7 +7404,7 @@ dp_netdev_input__(struct dp_netdev_pmd_thread *pmd,
 enum { PKT_ARRAY_SIZE = NETDEV_MAX_BURST };
 #endif
 OVS_ALIGNED_VAR(CACHE_LINE_SIZE)
-struct netdev_flow_key keys[PKT_ARRAY_SIZE];
+struct netdev_flow_key keys[PKT_ARRAY_SIZE];
 struct netdev_flow_key *missed_keys[PKT_ARRAY_SIZE];
 struct packet_batch_per_flow batches[PKT_ARRAY_SIZE];
 size_t n_batches;
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Extends the existing mirror configuration parameters

[Wang, Liang-min]

> -Original Message-
> From: Maxime Coquelin 
> Sent: Wednesday, May 19, 2021 3:56 AM
> To: Wang, Liang-min ; Miskell, Timothy
> ; d...@openvswitch.org
> Subject: Re: [PATCH] Extends the existing mirror configuration parameters
> 
> Hi Liang-min,
> 
> When replying inline, please do not prefix with ">>" as it is handled as
> quoted text. There is no need to prefix.
> 
> On 5/18/21 8:00 PM, Wang, Liang-min wrote:
> >> -Original Message-
> >> From: Maxime Coquelin 
> >> Sent: Tuesday, May 18, 2021 12:15 PM
> >> To: Miskell, Timothy ; d...@openvswitch.org
> >> Cc: Wang, Liang-min 
> >> Subject: Re: [PATCH] Extends the existing mirror configuration
> parameters
> >>
> >> Hi Timothy, Liang-min,
> >>
> >> Thanks for rebasing the patch.
> >> A list of delta against the first RFC could help the reviewers.
> >> I notice one change in the right direction is the conversion to Vhost
> >> API datapath instead of Vhost PMD.
> >>
> >>> In this patch we support both Vhost API and Vhost PMD because OVS
> supports both
> >>> VhostUser and Vdev ports.
> >>>
> >> Also, I would suggest to have the patch split in several incremental
> >> patches to ease the review.
> >>
> >>> Thank you for suggestion. We will provide incremental patches on next
> submission
> >>>
> >> On 5/10/21 6:00 PM, Timothy Miskell wrote:
> >>> From: Liang-min Wang 
> >>>
> >>> The following parameters are added:
> >>>  - mirror-offload: to turn on/off mirror offloading.
> >>>  - output-port-name: specify a port, using name string, that is on a
> different
> >>>bridge
> >>>  - output-src-vlan: output port vlan for each select-src-port.
> >>>  - output-dst-vlan: output port vlan for each select-dst-port.
> >>>  - flow-src-mac: use src mac address of each select-dst-port for the
> header
> >>>scan.
> >>>  - flow-dst-mac: use dst mac address of each select-src-port for the
> header
> >>>scan.
> >>>  - mirror-tunnel-addr: BDF string of the tunnel device.
> >>>
> >>> ovs-vsctl test change because new mirroring parameters are introduced
> in
> >> this patch
> >>
> >> It would help to provide examples of usage of these new parameters.
> >>
> >>> Will add examples in the new patches
> >>>
> >>> Create a defer procedure call thread to handle all mirror offload
> requests.
> >>> This is a light-weight thread which remains in sleep-state when there is
> no
> >> new request.
> >>> This is created between ovs-vsctl and mirror offloading back end
> >>>
> >>> Implementing DPDK tx-burst (VIRTIO ingress traffic
> >>> mirror) and rx-burst (VIRTIO egress traffic mirror) callbacks.
> >>> Each callback  functions implement the following tasks:
> >>>  1. Enable per-packet VLAN insertion
> >>>- for port mirroring, all packets are enabled per-packet VLAN 
> >>> insertion.
> >>>- for flow mirroring, only packet header matches the required mac
> >> address
> >>>  are enabled.
> >>>  2. Sending the packets to the specified transport port (output-port in
> >>> mirror offload configuration)
> >>>- for port mirroring, all packets are sent to the transport port.
> >>>- for flow mirroring, only matched packets are sent.
> >>>  3. Restore each packet attributes (remove DPDK per-packet offload
> flag)
> >>
> >> I will for sure have more questions later, but please find a few
> >> comments/questions below:
> >>
> >>> Signed-off-by: Liang-min Wang 
> >>> Tested-by: Timothy Miskell 
> >>> Suggested-by: Munish Mehan 
> >>> ---
> >>>  lib/automake.mk|   2 +
> >>>  lib/netdev-dpdk-mirror.c   | 516
> >> +
> >>>  lib/netdev-dpdk-mirror.h   |  83 ++
> >>>  lib/netdev-dpdk.c  | 397 
> >>>  lib/netdev-provider.h  |  16 ++
> >>>  lib/netdev.c   | 386 +++
> >>>  lib/netdev.h   |  16 ++
> >>>  tests/ovs-vsctl.at |   2 +
> >>>  vswitchd/bridge.c  | 271 ++-
> >>>  vswitchd/vswitch.ovsschema |  24 +-
> >>>  vswitchd/vswitch.xml   |  50 
> >>>  11 files changed, 1759 insertions(+), 4 deletions(-)
> >>>  create mode 100644 lib/netdev-dpdk-mirror.c
> >>>  create mode 100644 lib/netdev-dpdk-mirror.h
> >>>
> 
> ...
> 
> >>> +
> >>> +/* port/flow mirror traffic processors */
> >>> +static inline uint16_t
> >>> +netdev_custom_mirror_offload_cb(uint16_t qidx, struct rte_mbuf
> >> **pkts,
> >>> +uint16_t nb_pkts, void *user_params)
> >>> +{
> >>> +struct mirror_param *data = user_params;
> >>> +uint16_t i, dst_qidx, match_count = 0;
> >>> +uint16_t pkt_trans;
> >>> +uint16_t dst_port_id = data->dst_port_id;
> >>> +uint16_t dst_vlan_id = data->dst_vlan_id;
> >>> +struct rte_mbuf **pkt_buf = >pkt_buf[qidx * data-
> >>> max_burst_size];
> >>> +
> >>> +if (nb_pkts == 0) {
> >>> +return 0;
> >>> +}
> >>> +
> >>> +if (nb_pkts > data->max_burst_size) {
> >>> +VLOG_ERR("Per-flow batch size, %d, exceeds maximum limit\n",
> >> nb_pkts);
> 

Re: [ovs-dev] [PATCH dpdk-latest 1/6] travis: Switch to dpdk main branch.

[Stokes, Ian]

> Hi Ian,
> 
> On Wed, May 19, 2021 at 1:23 PM Stokes, Ian  wrote:
> >
> > > > On 06/05/2021 16:25, David Marchand wrote:
> > > > > Make this branch point to current main master branch so that we can
> > > > > track API breakage.
> > > > >
> > > >
> > > > s/master//
> > >
> > > I can strip out master above and keep it as main on commit.
> > >
> > > >
> > > > > Note: this should not be merged to master, intended for dpdk-latest
> > > > > branch only.
> > > > >
> > > >
> > > > It's probably better to give full project/branch names when branches
> > > > from dpdk and ovs are referred to in the commit log. e.g. "dpdk main
> > > > branch". It would get confusing for someone not familiar.
> > >
> > > Agreed, can make this change on commit.
> > >
> > > >
> > > > > Signed-off-by: David Marchand 
> > > > > Acked-by: Ilya Maximets 
> > > > > Signed-off-by: Ian Stokes 
> > > > >
> > > > > The default branch name in DPDK is changed from master to main.
> > > > > This patch reflects the same on travis builds for dpdk-latest branch.
> > > > >
> > > > > Tested-at: https://travis-ci.org/github/Sunil-Pai-G/ovs-
> > > > copy/builds/723223426
> > > >
> > > > "We couldn't display the repository Sunil-Pai-G/ovs-copy".
> > >
> > > Yes looks like this isn't available anymore, also this points to 
> > > travis-ci.org,
> this
> > > is set to be removed completely in a few weeks and changed to travis-
> > > ci.com.
> > >
> > > As such it probably doesn't make sense to have the tested by tag above
> as is.
> > > As this will never be in an release we could just remove it, otherwise 
> > > wait
> for
> > > a new tag from travis-ci.com.
> > >
> > > As the change is small I thinks its ok to remove? Thoughts?
> > >
> > > >
> > > > > Signed-off-by: Sunil Pai G 
> > > > > Signed-off-by: Ian Stokes 
> > > >
> > > > It's a very small patch to be squashed patches with multiple signoffs?
> > > > Ignore comment if commit log makes sense, but checkpatch is
> complaining
> > > > too.
> > > >
> > > > WARNING: Unexpected sign-offs from developers who are not authors
> or
> > > > co-authors or committers: Ian Stokes , Sunil Pai
> G
> > > > , Ian Stokes 
> > > > Lines checked: 36, Warnings: 1, Errors: 0
> > >
> > > I think it's minor, can add a co-author and sign off for Sunil here to 
> > > avoid
> the
> > > error? As David has done the rebase maybe that’s the correct approach?
> > >
> > I didn't see anymore comments on above, following he patch sync last
> week and the discussion had there I'm happy to update the commit
> message, remove the tested tag and push the series to dpdk-latest.
> 
> Either leaving as is (since this is a rebase) or rewriting the
> commitlogs are both fine to me.
> Thanks!

Thanks David, dpdk-latest is rebased on ovs-master and these are applied.

Travis is in the green

https://travis-ci.org/github/openvswitch/ovs/builds/771661238

We're thinking of looking at updating the github actions ci to maybe handle 
dpdk-latest also, although this may require some re-design on how we ci 
currently works.

This is just to have a replacement for when Travis.org eventually shuts down. 
Will keep you in the loop.

Thanks for these.

Regards
Ian

> 
> --
> David Marchand

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH dpdk-latest 1/6] travis: Switch to dpdk main branch.

[David Marchand]

Hi Ian,

On Wed, May 19, 2021 at 1:23 PM Stokes, Ian  wrote:
>
> > > On 06/05/2021 16:25, David Marchand wrote:
> > > > Make this branch point to current main master branch so that we can
> > > > track API breakage.
> > > >
> > >
> > > s/master//
> >
> > I can strip out master above and keep it as main on commit.
> >
> > >
> > > > Note: this should not be merged to master, intended for dpdk-latest
> > > > branch only.
> > > >
> > >
> > > It's probably better to give full project/branch names when branches
> > > from dpdk and ovs are referred to in the commit log. e.g. "dpdk main
> > > branch". It would get confusing for someone not familiar.
> >
> > Agreed, can make this change on commit.
> >
> > >
> > > > Signed-off-by: David Marchand 
> > > > Acked-by: Ilya Maximets 
> > > > Signed-off-by: Ian Stokes 
> > > >
> > > > The default branch name in DPDK is changed from master to main.
> > > > This patch reflects the same on travis builds for dpdk-latest branch.
> > > >
> > > > Tested-at: https://travis-ci.org/github/Sunil-Pai-G/ovs-
> > > copy/builds/723223426
> > >
> > > "We couldn't display the repository Sunil-Pai-G/ovs-copy".
> >
> > Yes looks like this isn't available anymore, also this points to 
> > travis-ci.org, this
> > is set to be removed completely in a few weeks and changed to travis-
> > ci.com.
> >
> > As such it probably doesn't make sense to have the tested by tag above as 
> > is.
> > As this will never be in an release we could just remove it, otherwise wait 
> > for
> > a new tag from travis-ci.com.
> >
> > As the change is small I thinks its ok to remove? Thoughts?
> >
> > >
> > > > Signed-off-by: Sunil Pai G 
> > > > Signed-off-by: Ian Stokes 
> > >
> > > It's a very small patch to be squashed patches with multiple signoffs?
> > > Ignore comment if commit log makes sense, but checkpatch is complaining
> > > too.
> > >
> > > WARNING: Unexpected sign-offs from developers who are not authors or
> > > co-authors or committers: Ian Stokes , Sunil Pai G
> > > , Ian Stokes 
> > > Lines checked: 36, Warnings: 1, Errors: 0
> >
> > I think it's minor, can add a co-author and sign off for Sunil here to 
> > avoid the
> > error? As David has done the rebase maybe that’s the correct approach?
> >
> I didn't see anymore comments on above, following he patch sync last week and 
> the discussion had there I'm happy to update the commit message, remove the 
> tested tag and push the series to dpdk-latest.

Either leaving as is (since this is a rebase) or rewriting the
commitlogs are both fine to me.
Thanks!

-- 
David Marchand

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH dpdk-latest 1/6] travis: Switch to dpdk main branch.

[Stokes, Ian]

> > On 06/05/2021 16:25, David Marchand wrote:
> > > Make this branch point to current main master branch so that we can
> > > track API breakage.
> > >
> >
> > s/master//
> 
> I can strip out master above and keep it as main on commit.
> 
> >
> > > Note: this should not be merged to master, intended for dpdk-latest
> > > branch only.
> > >
> >
> > It's probably better to give full project/branch names when branches
> > from dpdk and ovs are referred to in the commit log. e.g. "dpdk main
> > branch". It would get confusing for someone not familiar.
> 
> Agreed, can make this change on commit.
> 
> >
> > > Signed-off-by: David Marchand 
> > > Acked-by: Ilya Maximets 
> > > Signed-off-by: Ian Stokes 
> > >
> > > The default branch name in DPDK is changed from master to main.
> > > This patch reflects the same on travis builds for dpdk-latest branch.
> > >
> > > Tested-at: https://travis-ci.org/github/Sunil-Pai-G/ovs-
> > copy/builds/723223426
> >
> > "We couldn't display the repository Sunil-Pai-G/ovs-copy".
> 
> Yes looks like this isn't available anymore, also this points to 
> travis-ci.org, this
> is set to be removed completely in a few weeks and changed to travis-
> ci.com.
> 
> As such it probably doesn't make sense to have the tested by tag above as is.
> As this will never be in an release we could just remove it, otherwise wait 
> for
> a new tag from travis-ci.com.
> 
> As the change is small I thinks its ok to remove? Thoughts?
> 
> >
> > > Signed-off-by: Sunil Pai G 
> > > Signed-off-by: Ian Stokes 
> >
> > It's a very small patch to be squashed patches with multiple signoffs?
> > Ignore comment if commit log makes sense, but checkpatch is complaining
> > too.
> >
> > WARNING: Unexpected sign-offs from developers who are not authors or
> > co-authors or committers: Ian Stokes , Sunil Pai G
> > , Ian Stokes 
> > Lines checked: 36, Warnings: 1, Errors: 0
> 
> I think it's minor, can add a co-author and sign off for Sunil here to avoid 
> the
> error? As David has done the rebase maybe that’s the correct approach?
> 
> 
> BR
> Ian
> >
> > > ---
> > >  .travis.yml | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/.travis.yml b/.travis.yml
> > > index 51d0511080..7e87360256 100644
> > > --- a/.travis.yml
> > > +++ b/.travis.yml
> > > @@ -30,7 +30,9 @@ addons:
> > >
> > >  before_install: ./.ci/${TRAVIS_OS_NAME}-prepare.sh
> > >
> > > -before_script: export PATH=$PATH:$HOME/bin
> > > +before_script:
> > > +  - export PATH=$PATH:$HOME/bin
> > > +  - export DPDK_VER=refs/heads/main
> > >
> > >  matrix:
> > >include:
> > >
> 

Hi all,

I didn't see anymore comments on above, following he patch sync last week and 
the discussion had there I'm happy to update the commit message, remove the 
tested tag and push the series to dpdk-latest.

Regards
Ian
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH branch-2.14] ovsdb-idl: Consider all tables when computing expected cond seqno.

[Ilya Maximets]

On 5/17/21 11:22 AM, Dumitru Ceara wrote:
> In ovsdb_idl_db_set_condition(), take into account all pending
> condition changes for all tables when computing the db->cond_seqno at
> which the monitor is expected to be updated.
> 
> In the following scenario, with two tables, A and B, the old code
> performed the following steps:
> 1. Initial db->cond_seqno = X.
> 2. Client changes condition for table A:
>- A->new_cond gets set
>- expected cond seqno returned to the client: X + 1
> 3. ovsdb-idl sends the monitor_cond_change for table A
>- A->req_cond <- A->new_cond
> 4. Client changes condition for table B:
>- B->new_cond gets set
>- expected cond seqno returned to the client: X + 1
>- however, because the condition change at step 3 is still not replied
>  to, table B's monitor_cond_change request is not sent yet.
> 5. ovsdb-idl receives the reply for the condition change at step 3:
>- db->cond_seqno <- X + 1
> 6. ovsdb-idl sends the monitor_cond_change for table B
> 7. ovsdb-idl receives the reply for the condition change at step 6:
>   - db->cond_seqno <- X + 2
> 
> The client was incorrectly informed that it will have all relevant
> updates for table B at seqno X + 1 while actually that happens later, at
> seqno X + 2.
> 
> Fixes: 46437c5232bd ("ovsdb-idl: Enhance conditional monitoring API")
> Acked-by: Ben Pfaff 
> Signed-off-by: Dumitru Ceara 
> (cherry picked from commit b5bb044fbe4c1395dcde5cc7d5081ef0099bb8b3)
> ---
> Note: This backport applies cleanly to both 2.14 and 2.13.
> ---

Thanks!  Applied to branches 2.14 and 2.13.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpif-netdev: Remove meter rate from the bucket size calculation.

[Ilya Maximets]

On 4/21/21 3:48 PM, Ilya Maximets wrote:
> Implementation of meters supposed to be a classic token bucket with 2
> typical parameters: rate and burst size.
> 
> Burst size in this schema is the maximum number of bytes/packets that
> could pass without being rate limited.
> 
> Recent changes to userspace datapath made meter implementation to be
> in line with the kernel one, and this uncovered several issues.
> 
> The main problem is that maximum bucket size for unknown reason
> accounts not only burst size, but also the numerical value of rate.
> This creates a lot of confusion around behavior of meters.
> 
> For example, if rate is configured as 1000 pps and burst size set to 1,
> this should mean that meter will tolerate bursts of 1 packet at most,
> i.e. not a single packet above the rate should pass the meter.
> However, current implementation calculates maximum bucket size as
> (rate + burst size), so the effective bucket size will be 1001.  This
> means that first 1000 packets will not be rate limited and average
> rate might be twice as high as the configured rate.  This also makes
> it practically impossible to configure meter that will have burst size
> lower than the rate, which might be a desirable configuration if the
> rate is high.
> 
> Inability to configure low values of a burst size and overall inability
> for a user to predict what will be a maximum and average rate from the
> configured parameters of a meter without looking at the OVS and kernel
> code might be also classified as a security issue, because drop meters
> are frequently used as a way of protection from DoS attacks.
> 
> This change removes rate from the calculation of a bucket size, making
> it in line with the classic token bucket algorithm and essentially
> making the rate and burst tolerance being predictable from a users'
> perspective.
> 
> Same change will be proposed for the kernel implementation.
> Unit tests changed back to their correct version and enhanced.
> 
> Signed-off-by: Ilya Maximets 
> ---

Applied to master and backported down to 2.13.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2] Fix redundant datapath set ethernet action with NSH Decap

[Ilya Maximets]

On 5/19/21 5:26 AM, Martin Varghese wrote:
> On Tue, May 18, 2021 at 10:03:39PM +0200, Ilya Maximets wrote:
>> On 5/17/21 3:45 PM, Martin Varghese wrote:
>>> From: Martin Varghese 
>>>
>>> When a decap action is applied on NSH header encapsulatiing a
>>> ethernet packet a redundant set mac address action is programmed
>>> to the datapath.
>>>
>>> Fixes: f839892a206a ("OF support and translation of generic encap and 
>>> decap")
>>> Signed-off-by: Martin Varghese 
>>> Acked-by: Jan Scheurich 
>>> Acked-by: Eelco Chaudron 
>>> ---
>>> Changes in v2:
>>>   - Fixed code styling
>>>   - Added Ack from jan.scheur...@ericsson.com
>>>   - Added Ack from echau...@redhat.com
>>>
>>
>> Hi, Martin.
>> For some reason this patch triggers frequent failures of the following
>> unit test:
>>   
>> 2314. packet-type-aware.at:619: testing ptap - L3 over patch port
>> ...
>> stdout:
>> warped
>> ./packet-type-aware.at:726:
>> ovs-appctl dpctl/dump-flows --names dummy@ovs-dummy | strip_used | grep 
>> -v ipv6 | sort
>>
>> --- -   2021-05-18 21:57:56.810513366 +0200
>> +++ /home/i.maximets/work/git/ovs/tests/testsuite.dir/at-groups/2314/stdout  
>>2021-05-18 21:57:56.806609814 +0200
>> @@ -1,3 +1,3 @@
>>  flow-dump from the main thread:
>> -recirc_id(0),in_port(n0),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),eth_type(0x0800),ipv4(tos=0/0x3,frag=no),
>>  packets:1, bytes:98, used:0.0s, 
>> actions:pop_eth,clone(tnl_push(tnl_port(gre_sys),header(size=38,type=3,eth(dst=de:af:be:ef:ba:be,src=aa:55:00:00:00:02,dl_type=0x0800),ipv4(src=10.0.0.1,dst=10.0.0.2,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x0,proto=0x800))),out_port(br2)),n2)
>> +recirc_id(0),in_port(n0),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),eth_type(0x0800),ipv4(tos=0/0x3,frag=no),
>>  packets:1, bytes:98, used:0.0s, actions:drop
>>
>>
>> It fails very frequently in GitHub Actions, but it's harder to make it fail
>> on my local machine.  Following change to the test allows to reproduce the
>> failure almost always on my local machine:
>>
>> diff --git a/tests/packet-type-aware.at b/tests/packet-type-aware.at
>> index 540cf98f3..01dbc8030 100644
>> --- a/tests/packet-type-aware.at
>> +++ b/tests/packet-type-aware.at
>> @@ -721,7 +721,7 @@ AT_CHECK([
>>  ovs-appctl netdev-dummy/receive n0 
>> 1e2ce92a669e3a6dd2099cab080045548a83400040011aadc0a80a0ac0a80a1e0800b7170a4d0002fd509a58de1c0200101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637
>>  ], [0], [ignore])
>>  
>> -ovs-appctl time/warp 1000
>> +ovs-appctl time/warp 1000 100
>>  
>>  AT_CHECK([
>>  ovs-appctl dpctl/dump-flows --names dummy@ovs-dummy | strip_used | grep 
>> -v ipv6 | sort
>> --
>>
>> Without your patch I can not make it fail locally even with above wrapping
>> change applied.
>>
>> Could you, please, take a look?
>>
> 
> Hi Ilya
> 
> Travis CI was good. i will rebase & try again
> https://travis-ci.org/github/martinpattara/ovs/builds/770919003

Travis has only one job with tests enabled and it tests on arm.
GitHub Actions (which is our main CI now) wasn't good:
  https://github.com/martinpattara/ovs/runs/2567454405

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH net-next v2] net: openvswitch: IPv6: Add IPv6 extension header support

[Ilya Maximets]

On 5/17/21 5:20 PM, Toms Atteka wrote:
> IPv6 extension headers carry optional internet layer information
> and are placed between the fixed header and the upper-layer
> protocol header.
> 
> This change adds a new OpenFlow field OFPXMT_OFB_IPV6_EXTHDR and
> packets can be filtered using ipv6_ext flag.
> 
> Tested-at: https://github.com/TomCodeLV/ovs/actions/runs/504185214
> Signed-off-by: Toms Atteka 
> ---
>  include/uapi/linux/openvswitch.h |   1 +
>  net/openvswitch/flow.c   | 141 +++
>  net/openvswitch/flow.h   |  14 +++
>  net/openvswitch/flow_netlink.c   |   5 +-
>  4 files changed, 160 insertions(+), 1 deletion(-)
> 
> 
> base-commit: 5d869070569a23aa909c6e7e9d010fc438a492ef
> 
> diff --git a/include/uapi/linux/openvswitch.h 
> b/include/uapi/linux/openvswitch.h
> index 8d16744edc31..a19812b6631a 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -420,6 +420,7 @@ struct ovs_key_ipv6 {
>   __u8   ipv6_tclass;
>   __u8   ipv6_hlimit;
>   __u8   ipv6_frag;   /* One of OVS_FRAG_TYPE_*. */
> + __u16  ipv6_exthdr;
>  };

Wouldn't this break existing userspace?  Curent OVS expects netlink
message with attribute size equal to the old version of 'struct ovs_key_ipv6'
and it will discard OVS_KEY_ATTR_IPV6 as malformed.

This should likely be a completely new structure and a completely new
OVS_KEY_ATTR.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v12 00/11] Add offload support for sFlow

[Simon Horman]

On Tue, May 18, 2021 at 02:22:26PM +0200, Ilya Maximets wrote:
> On 4/27/21 3:23 AM, Chris Mi wrote:

...

> > Hi Ilya,
> > 
> > The code according to your suggestion is ready. But during the internal
> > code review, Eli Britstein thought flow_api is netdev based, but the
> > psample/sFlow offload is not netdev based, but dpif based. Maybe we
> > should introduce dpif-offload-provider and have a dpf_offload_api, so
> > it won't be related to a specific netdev, but to a dpif type.
> > 
> > Could I know what's your opinion about it?
> 
> This might be not bad idea in general.  The problem is that if we'll
> introduce this kind of API, we'll need to remove the usage of
> netdev-offload API from all the layers higher than dpif implementation
> including dpif.c and make all offloading go through
> dpif-offload-provider.
> 
> In general, I think, it might be good to have a separate dpif type, e.g.
> dpif-offload that will use netdev-offload APIs internally and provide
> offloading service.  In this case, dpif_operate(), depending on offload
> configuration, will try to install flows to dpif-offload first and
> fallback to install flows to the main dpif implementation if offload
> failed or if it was partial (i.e.  classification offloading).  This way
> we will delete all the offloading related code from dpif-netdev and
> dpif-netlink and will have a unified offloading architecture.  It's not
> easy to implement and there will be a lot of issues that we'll need to
> figure out how to fix (e.g. dpif-netdev performs upcalls by itself, so it
> will have to install resulted flows by calling a dpif_flow_put/operate
> and not install flows directly to its local flow tables).  Anyway, this
> looks like a long run and will require rework of the whole offloading
> architecture, so might be a separate thing to work on.  Thoughts?

Hi Ilya, Hi Chris,

I think that this approach makes sense in terms of an evolution of
the internal offload APIs/architecture. And might lean itself to more
advanced options in future - such as layered dpif implementations for
some purpose.

I do also there will be quite a lot of implementation challenges
to overcome.

Kind regards,
Simon


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH V3 1/1] dpdk: Add debug appctl to get malloc statistics.

[Eli Britstein]

New appctl 'dpdk/get-malloc-stats' implemented to get result of
'rte_malloc_dump_stats()' function.

Could be used for debugging.

Signed-off-by: Eli Britstein 
Reviewed-by: Salem Sol 
---

v2:
- Following Eelco Chaudron's comments, abandoned get-socket-stats.
- Acked-by: Eelco Chaudron 
  https://mail.openvswitch.org/pipermail/ovs-dev/2021-May/382920.html
- Acked-by: Kevin Traynor 
  https://mail.openvswitch.org/pipermail/ovs-dev/2021-May/382932.html
v3:
- Following Ilya Maximets's comment, using dpdk_unixctl_mem_stream.

 NEWS |  1 +
 lib/dpdk-unixctl.man |  2 ++
 lib/dpdk.c   | 10 ++
 3 files changed, 13 insertions(+)

diff --git a/NEWS b/NEWS
index 402ce5969..bcb44e045 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,7 @@ Post-v2.15.0
- DPDK:
  * OVS validated with DPDK 20.11.1. It is recommended to use this version
until further releases.
+ * New debug appctl command 'dpdk/get-malloc-stats'.
 
 
 v2.15.0 - 15 Feb 2021
diff --git a/lib/dpdk-unixctl.man b/lib/dpdk-unixctl.man
index 2d6d576f2..a0d1fa2ea 100644
--- a/lib/dpdk-unixctl.man
+++ b/lib/dpdk-unixctl.man
@@ -10,5 +10,7 @@ list of words separated by spaces: a word can be either a 
logging \fBlevel\fR
 \fBnotice\fR, \fBinfo\fR or \fBdebug\fR) or a \fBpattern\fR matching DPDK
 components (see \fBdpdk/log-list\fR command on \fBovs\-appctl\fR(8)) separated
 by a colon from the logging \fBlevel\fR to apply.
+.IP "\fBdpdk/get-malloc-stats\fR"
+Prints the heap information statistics about DPDK malloc.
 .RE
 .
diff --git a/lib/dpdk.c b/lib/dpdk.c
index 319540394..2eaaa569c 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -356,6 +357,12 @@ dpdk_unixctl_log_set(struct unixctl_conn *conn, int argc, 
const char *argv[],
 unixctl_command_reply(conn, NULL);
 }
 
+static void
+malloc_dump_stats_wrapper(FILE *stream)
+{
+rte_malloc_dump_stats(stream, NULL);
+}
+
 static bool
 dpdk_init__(const struct smap *ovs_other_config)
 {
@@ -525,6 +532,9 @@ dpdk_init__(const struct smap *ovs_other_config)
  dpdk_unixctl_mem_stream, rte_log_dump);
 unixctl_command_register("dpdk/log-set", "{level | pattern:level}", 0,
  INT_MAX, dpdk_unixctl_log_set, NULL);
+unixctl_command_register("dpdk/get-malloc-stats", "", 0, 0,
+ dpdk_unixctl_mem_stream,
+ malloc_dump_stats_wrapper);
 
 /* We are called from the main thread here */
 RTE_PER_LCORE(_lcore_id) = NON_PMD_CORE_ID;
-- 
2.28.0.2311.g225365fb51

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Extends the existing mirror configuration parameters

[Maxime Coquelin]

Hi Liang-min,

When replying inline, please do not prefix with ">>" as it is handled as
quoted text. There is no need to prefix.

On 5/18/21 8:00 PM, Wang, Liang-min wrote:
>> -Original Message-
>> From: Maxime Coquelin 
>> Sent: Tuesday, May 18, 2021 12:15 PM
>> To: Miskell, Timothy ; d...@openvswitch.org
>> Cc: Wang, Liang-min 
>> Subject: Re: [PATCH] Extends the existing mirror configuration parameters
>>
>> Hi Timothy, Liang-min,
>>
>> Thanks for rebasing the patch.
>> A list of delta against the first RFC could help the reviewers.
>> I notice one change in the right direction is the conversion to Vhost
>> API datapath instead of Vhost PMD.
>>
>>> In this patch we support both Vhost API and Vhost PMD because OVS supports 
>>> both
>>> VhostUser and Vdev ports.
>>>
>> Also, I would suggest to have the patch split in several incremental
>> patches to ease the review.
>>
>>> Thank you for suggestion. We will provide incremental patches on next 
>>> submission
>>>
>> On 5/10/21 6:00 PM, Timothy Miskell wrote:
>>> From: Liang-min Wang 
>>>
>>> The following parameters are added:
>>>  - mirror-offload: to turn on/off mirror offloading.
>>>  - output-port-name: specify a port, using name string, that is on a 
>>> different
>>>bridge
>>>  - output-src-vlan: output port vlan for each select-src-port.
>>>  - output-dst-vlan: output port vlan for each select-dst-port.
>>>  - flow-src-mac: use src mac address of each select-dst-port for the header
>>>scan.
>>>  - flow-dst-mac: use dst mac address of each select-src-port for the header
>>>scan.
>>>  - mirror-tunnel-addr: BDF string of the tunnel device.
>>>
>>> ovs-vsctl test change because new mirroring parameters are introduced in
>> this patch
>>
>> It would help to provide examples of usage of these new parameters.
>>
>>> Will add examples in the new patches
>>>
>>> Create a defer procedure call thread to handle all mirror offload requests.
>>> This is a light-weight thread which remains in sleep-state when there is no
>> new request.
>>> This is created between ovs-vsctl and mirror offloading back end
>>>
>>> Implementing DPDK tx-burst (VIRTIO ingress traffic
>>> mirror) and rx-burst (VIRTIO egress traffic mirror) callbacks.
>>> Each callback  functions implement the following tasks:
>>>  1. Enable per-packet VLAN insertion
>>>- for port mirroring, all packets are enabled per-packet VLAN insertion.
>>>- for flow mirroring, only packet header matches the required mac
>> address
>>>  are enabled.
>>>  2. Sending the packets to the specified transport port (output-port in
>>> mirror offload configuration)
>>>- for port mirroring, all packets are sent to the transport port.
>>>- for flow mirroring, only matched packets are sent.
>>>  3. Restore each packet attributes (remove DPDK per-packet offload flag)
>>
>> I will for sure have more questions later, but please find a few
>> comments/questions below:
>>
>>> Signed-off-by: Liang-min Wang 
>>> Tested-by: Timothy Miskell 
>>> Suggested-by: Munish Mehan 
>>> ---
>>>  lib/automake.mk|   2 +
>>>  lib/netdev-dpdk-mirror.c   | 516
>> +
>>>  lib/netdev-dpdk-mirror.h   |  83 ++
>>>  lib/netdev-dpdk.c  | 397 
>>>  lib/netdev-provider.h  |  16 ++
>>>  lib/netdev.c   | 386 +++
>>>  lib/netdev.h   |  16 ++
>>>  tests/ovs-vsctl.at |   2 +
>>>  vswitchd/bridge.c  | 271 ++-
>>>  vswitchd/vswitch.ovsschema |  24 +-
>>>  vswitchd/vswitch.xml   |  50 
>>>  11 files changed, 1759 insertions(+), 4 deletions(-)
>>>  create mode 100644 lib/netdev-dpdk-mirror.c
>>>  create mode 100644 lib/netdev-dpdk-mirror.h
>>>

...

>>> +
>>> +/* port/flow mirror traffic processors */
>>> +static inline uint16_t
>>> +netdev_custom_mirror_offload_cb(uint16_t qidx, struct rte_mbuf
>> **pkts,
>>> +uint16_t nb_pkts, void *user_params)
>>> +{
>>> +struct mirror_param *data = user_params;
>>> +uint16_t i, dst_qidx, match_count = 0;
>>> +uint16_t pkt_trans;
>>> +uint16_t dst_port_id = data->dst_port_id;
>>> +uint16_t dst_vlan_id = data->dst_vlan_id;
>>> +struct rte_mbuf **pkt_buf = >pkt_buf[qidx * data-
>>> max_burst_size];
>>> +
>>> +if (nb_pkts == 0) {
>>> +return 0;
>>> +}
>>> +
>>> +if (nb_pkts > data->max_burst_size) {
>>> +VLOG_ERR("Per-flow batch size, %d, exceeds maximum limit\n",
>> nb_pkts);
>>> +return 0;
>>> +}
>>> +
>>> +for (i = 0; i < nb_pkts; i++) {
>>> +if (data->custom_scan(pkts[i], user_params)) {
>>> +pkt_buf[match_count] = pkts[i];
>>> +pkt_buf[match_count]->ol_flags |= PKT_TX_VLAN_PKT;
>>
>> Does it work if the packet already has a VLAN inserted?
>>
>>> Good catch. The design is based upon no VLAN insertion offloading is 
>>> applied on source traffic. 

That is a significant limitation, I haven not seen it 

Re: [ovs-dev] [PATCH ovn v2] ovs: Include monitor condition expected seqno fix.

[Dumitru Ceara]

On 5/18/21 8:49 AM, Dumitru Ceara wrote:
> When setting monitor conditions ovsdb_cs_db_set_condition() returns the
> sequence number when it is expected that all updates that correspond to
> the new condition have been received.  This sequence number is used by
> ovn-controller to determine whether a monitor condition change is
> already in flight.  If that's the case ovn-controller waits until all
> in-flight condition changes have been processed before propagating the
> SB_Global.nb_cfg value to the chassis record.
> 
> This is part of the "ovn-nbctl --wait=hv sync" implementation which, as
> far as I know, is not used actively by any CMS but is, however, used by
> OVN self tests.
> 
> Bump the OVS submodule version to include b5bb044fbe4c ("ovsdb-cs:
> Consider all tables when computing expected cond seqno.") which fixes a
> bug in ovsdb_cs_db_set_condition().  Without it, the returned expected
> sequence number was wrong, leading to occasional test failures in the
> OVN test suite.

Just for reference an instance of a failed self test in CI due to the
IDL bug:

https://github.com/ovn-org/ovn/runs/2604425879#step:13:5037

> 
> Signed-off-by: Dumitru Ceara 
> ---
> v2: Detailed commit log, as suggested by Mark Michelson.
> ---
>  ovs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ovs b/ovs
> index ac85cdb38..b5bb044fb 16
> --- a/ovs
> +++ b/ovs
> @@ -1 +1 @@
> -Subproject commit ac85cdb38c1f33e7952bc4c0347d6c7873fb56a1
> +Subproject commit b5bb044fbe4c1395dcde5cc7d5081ef0099bb8b3
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH net-next v2] net: openvswitch: IPv6: Add IPv6 extension header support

[Tonghao Zhang]

+ d...@openvswitch.org

On Tue, May 18, 2021 at 12:33 AM Toms Atteka  wrote:
>
> IPv6 extension headers carry optional internet layer information
> and are placed between the fixed header and the upper-layer
> protocol header.
>
> This change adds a new OpenFlow field OFPXMT_OFB_IPV6_EXTHDR and
> packets can be filtered using ipv6_ext flag.
>
> Tested-at: https://github.com/TomCodeLV/ovs/actions/runs/504185214
> Signed-off-by: Toms Atteka 
> ---
>  include/uapi/linux/openvswitch.h |   1 +
>  net/openvswitch/flow.c   | 141 +++
>  net/openvswitch/flow.h   |  14 +++
>  net/openvswitch/flow_netlink.c   |   5 +-
>  4 files changed, 160 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/openvswitch.h 
> b/include/uapi/linux/openvswitch.h
> index 8d16744edc31..a19812b6631a 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -420,6 +420,7 @@ struct ovs_key_ipv6 {
> __u8   ipv6_tclass;
> __u8   ipv6_hlimit;
> __u8   ipv6_frag;   /* One of OVS_FRAG_TYPE_*. */
> +   __u16  ipv6_exthdr;
>  };
>
>  struct ovs_key_tcp {
> diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
> index e586424d8b04..cfc5f395f82c 100644
> --- a/net/openvswitch/flow.c
> +++ b/net/openvswitch/flow.c
> @@ -239,6 +239,145 @@ static bool icmphdr_ok(struct sk_buff *skb)
>   sizeof(struct icmphdr));
>  }
>
> +/**
> + * get_ipv6_ext_hdrs() - Parses packet and sets IPv6 extension header flags.
> + *
> + * @skb: Buffer where extension header data starts in packet.
> + * @nh: IPv6 header.
> + * @ext_hdrs: Flags are stored here.
> + *
> + * OFPIEH12_UNREP is set if more than one of a given IPv6 extension header
> + * is unexpectedly encountered. (Two destination options headers may be
> + * expected and would not cause this bit to be set.)
> + *
> + * OFPIEH12_UNSEQ is set if IPv6 extension headers were not in the order
> + * preferred (but not required) by RFC 2460:
> + *
> + * When more than one extension header is used in the same packet, it is
> + * recommended that those headers appear in the following order:
> + *  IPv6 header
> + *  Hop-by-Hop Options header
> + *  Destination Options header
> + *  Routing header
> + *  Fragment header
> + *  Authentication header
> + *  Encapsulating Security Payload header
> + *  Destination Options header
> + *  upper-layer header
> + */
> +static void get_ipv6_ext_hdrs(struct sk_buff *skb, struct ipv6hdr *nh, u16 
> *ext_hdrs)
> +{
> +   int next_type = nh->nexthdr;
> +   unsigned int start = skb_network_offset(skb) + sizeof(struct ipv6hdr);
> +   int dest_options_header_count = 0;
> +
> +   *ext_hdrs = 0;
> +
> +   while (ipv6_ext_hdr(next_type)) {
> +   struct ipv6_opt_hdr _hdr, *hp;
> +
> +   switch (next_type) {
> +   case IPPROTO_NONE:
> +   *ext_hdrs |= OFPIEH12_NONEXT;
> +   /* stop parsing */
> +   return;
> +
> +   case IPPROTO_ESP:
> +   if (*ext_hdrs & OFPIEH12_ESP)
> +   *ext_hdrs |= OFPIEH12_UNREP;
> +   if ((*ext_hdrs & ~(OFPIEH12_HOP |
> +  OFPIEH12_DEST |
> +  OFPIEH12_ROUTER |
> +  IPPROTO_FRAGMENT |
> +  OFPIEH12_AUTH |
> +  OFPIEH12_UNREP)) ||
> +   dest_options_header_count >= 2)
> +   *ext_hdrs |= OFPIEH12_UNSEQ;
> +   *ext_hdrs |= OFPIEH12_ESP;
> +   break;
> +
> +   case IPPROTO_AH:
> +   if (*ext_hdrs & OFPIEH12_AUTH)
> +   *ext_hdrs |= OFPIEH12_UNREP;
> +   if ((*ext_hdrs & ~(OFPIEH12_HOP |
> +  OFPIEH12_DEST |
> +  OFPIEH12_ROUTER |
> +  IPPROTO_FRAGMENT |
> +  OFPIEH12_UNREP)) ||
> +   dest_options_header_count >= 2)
> +   *ext_hdrs |= OFPIEH12_UNSEQ;
> +   *ext_hdrs |= OFPIEH12_AUTH;
> +   break;
> +
> +   case IPPROTO_DSTOPTS:
> +   if (dest_options_header_count == 0) {
> +   if (*ext_hdrs & ~(OFPIEH12_HOP |
> + OFPIEH12_UNREP))
> +   *ext_hdrs |= OFPIEH12_UNSEQ;
> +   *ext_hdrs |= OFPIEH12_DEST;
> +   } else if (dest_options_header_count == 1) {
> +   if 

[ovs-dev] netdev-linux.c : nl_sock_listen_all_nsid triggers NULL deref.

[lin huang]

>From 32113ea0fb65a99271359440316abf7d8a14f551 Mon Sep 17 00:00:00 2001
From: miter 
Date: Wed, 19 May 2021 12:55:24 +0800
Subject: [PATCH] netdev-linux.c : nl_sock_listen_all_nsid triggers NULL deref.

Signed-off-by: miter 
---
 lib/netdev-linux.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 15b25084b..0994044ec 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -634,7 +634,9 @@ netdev_linux_notify_sock(void)
 }
 }
 }
-nl_sock_listen_all_nsid(sock, true);
+if (sock) {
+nl_sock_listen_all_nsid(sock, true);
+ }
 ovsthread_once_done();
 }

--
2.31.1.windows.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev