Re: [ovs-dev] [PATCH] ofproto:ipv6 mld report error send to fports

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings dingxiaoxiong, 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 dingxiaoxiong  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: XiaoXiong Ding 
Lines checked: 34, 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] Amazonセキュリティ警告: サインインが検出されました

2020-09-09 Thread Amazon.co.jp via dev 
 あなたのアカウントは停止されました

 
新しいデバイスからアカウントサービスへのサインインが検出されました。
誰かがあなたのAmazonアカウントで他のデバイスから購入しようとしました。Amazonの保護におけるセキュリティと整合性の問題により、セキュリティ上の理由からアカウントがロックされます。
アカウントを引き続き使用するには、24時間前に情報を更新することをお勧めします。それ以外の場合、あなたのアカウントは永久ロック。 

確認用アカウント 






© 2020 Amazon.com, Inc. or its affiliates. All rights reserved. Amazon, 
Amazon.co.jp, Amazon Prime, Prime およびAmazon.co.jp のロゴは Amazon.com , 
Inc.またはその関連会社の商標です。 Amazon.com, 410 Terry Avenue N., Seattle, WA 98109-5210 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] ofproto:ipv6 mld report error send to fports

2020-09-09 Thread dingxiaoxiong 
when mld report come(same as mld done), will do funciton
xlate_normal_mcast_send_rports, and because of no return,
will doforwarding to group base ports logic,and if
other_config:mcast-snooping-flood is set on a port, will do
xlate_normal_mcast_send_fports. so mld report will be reveived
when xlate_normal_mcast_send_fports is set on a port,whether
xlate_normal_mcast_send_fports is set or not.
but ipv4 is ok.

Signed-off-by: XiaoXiong Ding 
---
 ofproto/ofproto-dpif-xlate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index e0ede2cab..47571e790 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3100,6 +3100,7 @@ xlate_normal(struct xlate_ctx *ctx)
 xlate_report(ctx, OFT_DETAIL, "MLD query, flooding");
 xlate_normal_flood(ctx, in_xbundle, );
 }
+return;
 } else {
 if (is_ip_local_multicast(flow, wc)) {
 /* RFC4541: section 2.1.2, item 2: Packets with a dst IP
-- 
2.14.1.windows.1
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 3/3] Fix tap interface status update issue in network namespace

2020-09-09 Thread yang_y_yi 
From: Yi Yang 

Currently OVS can't get link state, mtu, mac, driver, etc.
when tap interface is in network namespace, with netns option
and netns helper functions, these info can be gotten.

This patch fixed all these issues and make sure tap interface
in network namespace can get same info as it is in root
network namespace.

Here is a result sample for reference:

$ sudo ./ovs-vsctl list interface tap1
_uuid   : 283b0daf-1baf-4d46-8a0e-3774eb39d0e3
admin_state : up
bfd : {}
bfd_status  : {}
cfm_fault   : []
cfm_fault_status: []
cfm_flap_count  : []
cfm_health  : []
cfm_mpid: []
cfm_remote_mpids: []
cfm_remote_opstate  : []
duplex  : full
error   : []
external_ids: {}
ifindex : 1276
ingress_policing_burst: 0
ingress_policing_rate: 0
lacp_current: []
link_resets : 1
link_speed  : 1000
link_state  : up
lldp: {}
mac : []
mac_in_use  : "f6:8d:3f:0a:3a:dd"
mtu : 1450
mtu_request : 1450
name: tap1
ofport  : 4
ofport_request  : []
options : {netns=ns01}
other_config: {}
statistics  : {rx_bytes=726, rx_packets=9, tx_bytes=2178,...}
status  : {driver_name=tun, driver_version="1.6",...}
type: tap
$

Signed-off-by: Yi Yang 
---
 lib/netdev-linux.c | 371 ++---
 lib/socket-util-unix.c |  37 +
 lib/socket-util.h  |   3 +
 3 files changed, 364 insertions(+), 47 deletions(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 346dc94..289ebab 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -532,17 +532,19 @@ static int enter_netns(struct netns_knob *netns_knob, 
const char *netns);
 static void exit_netns(struct netns_knob *netns_knob);
 static int netdev_linux_do_ethtool(const char *name, struct ethtool_cmd *,
int cmd, const char *cmd_name);
-static int get_flags(const struct netdev *, unsigned int *flags);
-static int set_flags(const char *, unsigned int flags);
+static int get_flags(const struct netdev *, unsigned int *flags,
+ bool is_tap_in_netns);
+static int set_flags(const char *, unsigned int flags, bool is_tap_in_netns);
 static int update_flags(struct netdev_linux *netdev, enum netdev_flags off,
-enum netdev_flags on, enum netdev_flags *old_flagsp)
+enum netdev_flags on, enum netdev_flags *old_flagsp,
+bool is_tap_in_netns)
 OVS_REQUIRES(netdev->mutex);
 static int get_ifindex(const struct netdev *, int *ifindexp);
 static int do_set_addr(struct netdev *netdev,
int ioctl_nr, const char *ioctl_name,
-   struct in_addr addr);
-static int get_etheraddr(const char *netdev_name, struct eth_addr *ea);
-static int set_etheraddr(const char *netdev_name, const struct eth_addr);
+   struct in_addr addr, bool is_tap_in_netns);
+static int get_etheraddr(const struct netdev *netdev_, struct eth_addr *ea);
+static int set_etheraddr(const struct netdev *netdev_, const struct eth_addr);
 static int af_packet_sock(void);
 static bool netdev_linux_miimon_enabled(void);
 static void netdev_linux_miimon_run(void);
@@ -791,13 +793,28 @@ netdev_linux_run(const struct netdev_class *netdev_class 
OVS_UNUSED)
 struct netdev *netdev_ = node->data;
 struct netdev_linux *netdev = netdev_linux_cast(netdev_);
 unsigned int flags;
+struct netns_knob netns_knob;
+bool is_tap_in_netns = false;
+
+if (is_tap_netdev(netdev_) && (netdev->netns != NULL)) {
+is_tap_in_netns = true;
+error = enter_netns(_knob, netdev->netns);
+if (error) {
+netdev_close(netdev_);
+continue;
+}
+}
 
 ovs_mutex_lock(>mutex);
-get_flags(netdev_, );
+get_flags(netdev_, , is_tap_in_netns);
 netdev_linux_changed(netdev, flags, 0);
 ovs_mutex_unlock(>mutex);
 
 netdev_close(netdev_);
+
+if (is_tap_in_netns) {
+exit_netns(_knob);
+}
 }
 shash_destroy(_shash);
 } else if (error != EAGAIN) {
@@ -952,7 +969,7 @@ netdev_linux_construct(struct netdev *netdev_)
 return error;
 }
 
-error = get_flags(>up, >ifi_flags);
+error = get_flags(>up, >ifi_flags, false);
 if (error == ENODEV) {
 if (netdev->up.netdev_class != _internal_class) {
 /* The device does not exist, so don't allow it to be opened. */
@@ -996,7 +1013,7 @@

[ovs-dev] [PATCH v2 0/3] userspace: enable tap interface statistics and status update support

2020-09-09 Thread yang_y_yi 
From: Yi Yang 

OVS userspace datapath can't support tap interface statistics
and status update, so users can't get these information by cmd
"ovs-vsctl list interface tap1", the root cause of this issue
is OVS doesn't know network namespace of tap interface.

This patch series fixed this issue and make sure tap interface
can show statistics and get status update.

Yi Yang (3):
  Add netns option for tap interface in userspace datapath
  Fix tap interface statistics issue
  Fix tap interface status update issue in network namespace

 lib/dpif-netlink.c |  51 +
 lib/dpif-netlink.h |   3 +
 lib/netdev-linux-private.h |   1 +
 lib/netdev-linux.c | 481 -
 lib/netlink-socket.c   | 146 ++
 lib/netlink-socket.h   |   2 +
 lib/socket-util-unix.c |  37 
 lib/socket-util.h  |   3 +
 8 files changed, 675 insertions(+), 49 deletions(-)

--

Changelog

  v1 -> v2:
* Split pmd thread support to seperate patch series
* Check enter_netns return error
* Limit setns to network namespace only by CLONE_NEWNET

-- 
1.8.3.1

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

[ovs-dev] [PATCH v2 1/3] Add netns option for tap interface in userspace datapath

2020-09-09 Thread yang_y_yi 
From: Yi Yang 

In userspace datapath, "ovs-vsctl list interface" can't
get interface statistics and there are many WARN log, we
can enable it work normally if it has correct network
namespace. This patch enabled netns option for tap interface
, it is the prerequisite interface statistics and other
ioctl can work normally.

Signed-off-by: Yi Yang 
---
 lib/netdev-linux-private.h |  1 +
 lib/netdev-linux.c | 47 ++
 2 files changed, 48 insertions(+)

diff --git a/lib/netdev-linux-private.h b/lib/netdev-linux-private.h
index c7c515f..fbedfd9 100644
--- a/lib/netdev-linux-private.h
+++ b/lib/netdev-linux-private.h
@@ -68,6 +68,7 @@ struct netdev_linux {
 struct timer miimon_timer;
 
 int netnsid;/* Network namespace ID. */
+char *netns;/* Network namespace name. */
 /* The following are figured out "on demand" only.  They are only valid
  * when the corresponding VALID_* bit in 'cache_valid' is set. */
 int ifindex;
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index fe7fb9b..ebe4ddb 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1054,6 +1054,9 @@ netdev_linux_destruct(struct netdev *netdev_)
 {
 ioctl(netdev->tap_fd, TUNSETPERSIST, 0);
 close(netdev->tap_fd);
+if (netdev->netns != NULL) {
+free(netdev->netns);
+}
 }
 
 if (netdev->miimon_interval > 0) {
@@ -3509,6 +3512,48 @@ exit:
 return error;
 }
 
+static int
+netdev_tap_set_config(struct netdev *netdev, const struct smap *args,
+char **errp OVS_UNUSED)
+{
+struct netdev_linux *dev = netdev_linux_cast(netdev);
+const char *netns;
+
+ovs_mutex_lock(>mutex);
+netns = smap_get(args, "netns");
+if (netns != NULL) {
+char nspath[128];
+int nsfd;
+
+sprintf(nspath, "/var/run/netns/%s", netns);
+nsfd = open(nspath, O_RDONLY);
+if (nsfd < 0) {
+ovs_mutex_unlock(>mutex);
+VLOG_ERR("%s: netns %s doesn't exist.",
+ netdev_get_name(netdev), netns);
+return EINVAL;
+}
+close(nsfd);
+}
+
+if (netns != NULL) {
+dev->netns = xstrdup(netns);
+}
+ovs_mutex_unlock(>mutex);
+return 0;
+}
+
+static int
+netdev_tap_get_config(const struct netdev *netdev, struct smap *args)
+{
+struct netdev_linux *dev = netdev_linux_cast(netdev);
+
+ovs_mutex_lock(>mutex);
+smap_add_format(args, "netns", "%s", dev->netns);
+ovs_mutex_unlock(>mutex);
+return 0;
+}
+
 #define NETDEV_LINUX_CLASS_COMMON   \
 .run = netdev_linux_run,\
 .wait = netdev_linux_wait,  \
@@ -3573,6 +3618,8 @@ const struct netdev_class netdev_tap_class = {
 .get_stats = netdev_tap_get_stats,
 .get_features = netdev_linux_get_features,
 .get_status = netdev_linux_get_status,
+.set_config = netdev_tap_set_config,
+.get_config = netdev_tap_get_config,
 .send = netdev_linux_send,
 .rxq_construct = netdev_linux_rxq_construct,
 .rxq_destruct = netdev_linux_rxq_destruct,
-- 
1.8.3.1

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

[ovs-dev] [PATCH v2 2/3] Fix tap interface statistics issue

2020-09-09 Thread yang_y_yi 
From: Yi Yang 

After tap interface is moved to network namespace,
"ovs-vsctl list interface tapXXX" can get statistics
info of tap interface, the root cause is OVS still
gets statistics info in root namespace.

With netns option help, OVS can get statistics info
in tap interface netns.

This patch added enter and exit netns helpers and
change statistics-related functions for those tap
interfaces which have been moved into netns and
make sure "ovs-vsctl list interface tapXXX" can
get statistics info correctly.

Here is a result sample for reference:
name: tap1
ofport  : 4
ofport_request  : []
options : {netns=ns01}
other_config: {}
statistics  : {rx_bytes=6228, rx_packets=68, tx_bytes=8310, 
tx_packets=95}
status  : {}
type: tap

Signed-off-by: Yi Yang 
---
 lib/dpif-netlink.c   |  51 ++
 lib/dpif-netlink.h   |   3 ++
 lib/netdev-linux.c   |  63 +-
 lib/netlink-socket.c | 146 +++
 lib/netlink-socket.h |   2 +
 5 files changed, 263 insertions(+), 2 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 7da4fb5..8ed37ed 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -4282,6 +4282,43 @@ dpif_netlink_vport_transact(const struct 
dpif_netlink_vport *request,
 return error;
 }
 
+static int
+dpif_netlink_vport_transact_nopool(const struct dpif_netlink_vport *request,
+   struct dpif_netlink_vport *reply,
+   struct ofpbuf **bufp)
+{
+struct ofpbuf *request_buf;
+int error;
+
+ovs_assert((reply != NULL) == (bufp != NULL));
+
+error = dpif_netlink_init();
+if (error) {
+if (reply) {
+*bufp = NULL;
+dpif_netlink_vport_init(reply);
+}
+return error;
+}
+
+request_buf = ofpbuf_new(1024);
+dpif_netlink_vport_to_ofpbuf(request, request_buf);
+error = nl_transact_nopool(NETLINK_GENERIC, request_buf, bufp);
+ofpbuf_delete(request_buf);
+
+if (reply) {
+if (!error) {
+error = dpif_netlink_vport_from_ofpbuf(reply, *bufp);
+}
+if (error) {
+dpif_netlink_vport_init(reply);
+ofpbuf_delete(*bufp);
+*bufp = NULL;
+}
+}
+return error;
+}
+
 /* Obtains information about the kernel vport named 'name' and stores it into
  * '*reply' and '*bufp'.  The caller must free '*bufp' when the reply is no
  * longer needed ('reply' will contain pointers into '*bufp').  */
@@ -4298,6 +4335,20 @@ dpif_netlink_vport_get(const char *name, struct 
dpif_netlink_vport *reply,
 return dpif_netlink_vport_transact(, reply, bufp);
 }
 
+int
+dpif_netlink_vport_get_nopool(const char *name,
+  struct dpif_netlink_vport *reply,
+  struct ofpbuf **bufp)
+{
+struct dpif_netlink_vport request;
+
+dpif_netlink_vport_init();
+request.cmd = OVS_VPORT_CMD_GET;
+request.name = name;
+
+return dpif_netlink_vport_transact_nopool(, reply, bufp);
+}
+
 /* Parses the contents of 'buf', which contains a "struct ovs_header" followed
  * by Netlink attributes, into 'dp'.  Returns 0 if successful, otherwise a
  * positive errno value.
diff --git a/lib/dpif-netlink.h b/lib/dpif-netlink.h
index 24294bc..9372241 100644
--- a/lib/dpif-netlink.h
+++ b/lib/dpif-netlink.h
@@ -55,6 +55,9 @@ int dpif_netlink_vport_transact(const struct 
dpif_netlink_vport *request,
 struct ofpbuf **bufp);
 int dpif_netlink_vport_get(const char *name, struct dpif_netlink_vport *reply,
struct ofpbuf **bufp);
+int dpif_netlink_vport_get_nopool(const char *name,
+  struct dpif_netlink_vport *reply,
+  struct ofpbuf **bufp);
 
 bool dpif_netlink_is_internal_device(const char *name);
 
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index ebe4ddb..346dc94 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -520,8 +520,16 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 
20);
  * changes in the device miimon status, so we can use atomic_count. */
 static atomic_count miimon_cnt = ATOMIC_COUNT_INIT(0);
 
+struct netns_knob {
+int main_fd;
+int netns_fd;
+char nspath[128];
+};
+
 static int netdev_linux_parse_vnet_hdr(struct dp_packet *b);
 static void netdev_linux_prepend_vnet_hdr(struct dp_packet *b, int mtu);
+static int enter_netns(struct netns_knob *netns_knob, const char *netns);
+static void exit_netns(struct netns_knob *netns_knob);
 static int netdev_linux_do_ethtool(const char *name, struct ethtool_cmd *,
int cmd, const char *cmd_name);
 static int get_flags(const struct netdev *, unsigned int *flags);
@@ -2158,11 +2166,18 @@ netdev_stats_from_ovs_vport_stats(struct netdev_stats

[ovs-dev] Motivos de fracaso de los presupuestos

2020-09-09 Thread Elaboración y control de presupuestos 
Webinar en Vivo: 
Elaboración y control de presupuestos
Curso en línea - Webinar Interactivo – Martes 29 de Septiembre- Horario de 
10:00 a 14:00 Hrs.

La constante evolución de la organización empresarial y de su entorno hace que 
el control presupuestario sea una herramienta básica para la consecución 
de los objetivos y estrategias de la empresa. Debido a que cada vez es más 
necesaria la anticipación a los acontecimientos, el curso de presupuestos te 
proporcionará conocimientos para tomar mejores decisiones y a optimizar 
rendimientos de tu empresa.

Objetivos Específicos:

- Conocer cuáles son las fases para la elaboración de un presupuesto general.
- Entender cómo utilizar los presupuestos para conseguir los planes y objetivos 
planteados dentro de la estrategia de la empresa.
- Conocer la información presupuestaria y sus conexiones.
- Saber cómo efectuar el seguimiento del presupuesto.

Para mayor información, responder sobre este correo con la palabra Presupuesto 
+ los siguientes datos:

NOMBRE:
TELÉFONO:
EMPRESA:
CORREO ALTERNO: 

Para información inmediata llamar al:
(+52) 55 15 54 66 30 - (+52) 55 30 16 70 85
O puede enviarnos un Whatsapp  

Innova Learn México - innovalearn. mx - Mérida, Yucatán, México


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

Re: [ovs-dev] [PATCH v2 22/24] acinclude: Enable builds up to Linux 5.8

2020-09-09 Thread Gregory Rose 




On 9/9/2020 12:20 PM, Greg Rose wrote:

Allow building openvswitch against Linux kernels up to and including
version 5.8.

Signed-off-by: Greg Rose 
---
  acinclude.m4 | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 3d56510a0..c04e4e429 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -167,10 +167,10 @@ AC_DEFUN([OVS_CHECK_LINUX], [
  AC_MSG_RESULT([$kversion])
  
  if test "$version" -ge 5; then

-   if test "$version" = 5 && test "$patchlevel" -le 5; then
+   if test "$version" = 5 && test "$patchlevel" -le 8; then
: # Linux 5.x
 else
-  AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version 
newer than 5.5.x is not supported (please refer to the FAQ for advice)])
+  AC_ERROR([Linux kernel in $KBUILD is version $kversion, but version 
newer than 5.9.x is not supported (please refer to the FAQ for advice)])


The warning message isn't right - should be 5.8.x.  Just noticed it in 
other testing.


I'll fix it up after more reviews...

- Greg


 fi
  elif test "$version" = 4; then
 : # Linux 4.x


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

Re: [ovs-dev] [PATCH ovn v2] ovn-northd: Optionally skip the check of lsp_is_up.

2020-09-09 Thread Han Zhou 
On Wed, Sep 9, 2020 at 4:15 AM Numan Siddique  wrote:
>
> On Wed, Sep 9, 2020 at 2:52 AM Han Zhou  wrote:
> >
> > The checking of lsp "up" status before adding ARP responder flows was
> > added to avoid confusion in cases when a network admin sees ARP response
> > for VM/containers that is not up on chassis yet. However, this check
> > introduces an extra round of flow change in SB and triggers computes
> > on hypervisors which is unnecessary cost in most cases, especially for
> > large scale environment. To improve this, this patch provides an option
> > ignore_lsp_down to disable the check for the use cases when ARP reponse
for a
> > port that is "down" isn't considered harmful.
> >
> > Acked-by: Numan Siddique 
> > Signed-off-by: Han Zhou 
> > ---
> > v1 -> v2: change the option from "check_lsp_is_up" to "ignore_lsp_down"
as
> >   suggested by Numan.
>
>
> Hi Han,
>
> The patch LGTM.
>
Thanks Numan. I applied this to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 20/24] datapath: Distribute switch variables for initialization

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Greg Rose, 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 Kees Cook  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 104, 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 v2 19/24] datapath: use skb_list_walk_safe helper for gso segments

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Greg Rose, 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 Jason A. Donenfeld  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 79, 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 v2 0/9] Incremental processing for flow installation.

2020-09-09 Thread Han Zhou 
On Wed, Sep 9, 2020 at 6:39 AM Mark Michelson  wrote:
>
> Thanks for addressing my concerns on patch 9.
>
> Acked-by: Mark Michelson 
>
> Warning: I'm also ACKing Numan's patch series that also adds some
> lflow-level caching (but he is caching expressions). So whoever loses
> this merge race may have some work to do in order to apply their patch
> cleanly.
>
Thanks Mark for the review, and the warning :).
I applied the series to master, and also the first 4 patches to
branch-20.06 and patch 1,2,4 to branch-20.03, to fix the conjunction flow
bug in those branches.

Thanks,
Han

> On 9/7/20 2:45 AM, Han Zhou wrote:
> > Incremental processing has been implementation in ovn-controller, but
we were
> > still doing full comparison between desired flow table and installed
flow table
> > every time to figure out the changes need to be pushed to OVS. This
series is
> > mainly to utilize the incremental processing information to figure out
flow
> > changes to OVS without full table scanning, to further reduce CPU of
> > ovn-controller.
> >
> > In ovn-scale-test with 3000 HVs and 30k lports, the end-to-end latency
between
> > the moment a lflow is updated in SB DB and the moment when all the 3K
HVs
> > complete OVS flow updating has reduced around 60% (from 1s to 400ms).
perf
> > report also shows ~40% of CPU reduced in ovn-controller.
> >
> > Another important change of this series is the fix of the conjunction
handling
> > problem.
> >
> > v1 -> v2:
> >
> > - Addressed Mark's comments for defining different data types for
desired flows
> >and installed flows, and related refactoring. (patch 6/9)
> > - Updated comments for the cross reference structures between desired
flows and
> >SB UUIDs with a diagram to help understanding. (patch 4/9)
> >
> > Han Zhou (9):
> >ofctrl: change ofctrl_dup_flow to module internal function
> >ovn.at: Fix AT for conjunction case.
> >lflow.c: No need to remove flows for adding new datapath.
> >ovn-controller: Fix conjunction handling with incremental processing.
> >ovn.at: Add test case for duplicated flow handling.
> >ofctrl.c: Maintain references between installed flows and desired
flows.
> >ofctrl.c: Refactor - move openflow msg construction to functions.
> >ofctrl: Incremental processing for flow installation by tracking.
> >ofctrl.c: Merge opposite changes of tracked flows before installing.
> >
> >   controller/lflow.c  |  106 --
> >   controller/ofctrl.c | 1041
+--
> >   controller/ofctrl.h |   39 +-
> >   tests/ovn.at|  225 ++-
> >   4 files changed, 1164 insertions(+), 247 deletions(-)
> >
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 18/24] datapath: support asymmetric conntrack

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Greg Rose, 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 aaron conole  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 58, 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 v2 17/24] datapath: remove another BUG_ON()

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Greg Rose, 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 Paolo Abeni  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 59, 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 v2 16/24] datapath: drop unneeded BUG_ON() in ovs_flow_cmd_build_info()

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Greg Rose, 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 Paolo Abeni  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 50, 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 v2 15/24] datapath: fix flow command message size

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Greg Rose, 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 Paolo Abeni  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 52, 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 v2 14/24] datapath: don't call pad_packet if not necessary

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Greg Rose, 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 Tonghao Zhang  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 76, 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 v2 13/24] datapath: select vport upcall portid directly

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Greg Rose, 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 Tonghao Zhang  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 50, 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 v2 12/24] datapath: simplify the ovs_dp_cmd_new

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Greg Rose, 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 Tonghao Zhang  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 138, 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 v2 11/24] datapath: fix possible memleak on destroy flow-table

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Greg Rose, 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 Tonghao Zhang  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 302, 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 v2 09/24] datapath: simplify the flow_hash

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Greg Rose, 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 Tonghao Zhang  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 50, 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 v2 07/24] datapath: don't unlock mutex when changing the user_features fails

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Greg Rose, 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 Tonghao Zhang  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 54, 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 v2 06/24] datapath: fix GFP flags in rtnl_net_notifyid()

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Greg Rose, 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 Guillaume Nault  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 136, 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 v2 05/24] datapath: Set OvS recirc_id from tc chain index

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Greg Rose, 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 Paul Blakey  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 189, 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 v2 04/24] datapath: Print error when ovs_execute_actions() fails

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Greg Rose, 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 Yifeng Sun  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 62, 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 v2 03/24] datapath: do not update max_headroom if new headroom is equal to old headroom

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Greg Rose, 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 Taehee Yoo  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 127, 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 v2 02/24] datapath: drop unneeded likely() call around IS_ERR()

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Greg Rose, 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 Enrico Weigelt  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 40, 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 v2 01/24] datapath: return an error instead of doing BUG_ON()

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Greg Rose, 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 Eelco Chaudron  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Greg Rose 
Lines checked: 54, 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 v2 11/24] datapath: fix possible memleak on destroy flow-table

2020-09-09 Thread Greg Rose 
From: Tonghao Zhang 

Upstream commit:
commit 50b0e61b32ee890a75b4377d5fbe770a86d6a4c1
Author: Tonghao Zhang 
Date:   Fri Nov 1 22:23:52 2019 +0800

net: openvswitch: fix possible memleak on destroy flow-table

When we destroy the flow tables which may contain the flow_mask,
so release the flow mask struct.

Signed-off-by: Tonghao Zhang 
Tested-by: Greg Rose 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Added additional compat layer fixup for WRITE_ONCE()

Cc: Tonghao Zhang 
Reviewed-by: Tonghao Zhang 
Signed-off-by: Greg Rose 
---
 datapath/flow_table.c | 186 +-
 .../linux/compat/include/linux/compiler.h |  13 ++
 2 files changed, 111 insertions(+), 88 deletions(-)

diff --git a/datapath/flow_table.c b/datapath/flow_table.c
index ca2efe94d..bd05dd394 100644
--- a/datapath/flow_table.c
+++ b/datapath/flow_table.c
@@ -234,6 +234,74 @@ static int tbl_mask_array_realloc(struct flow_table *tbl, 
int size)
return 0;
 }
 
+static int tbl_mask_array_add_mask(struct flow_table *tbl,
+  struct sw_flow_mask *new)
+{
+   struct mask_array *ma = ovsl_dereference(tbl->mask_array);
+   int err, ma_count = READ_ONCE(ma->count);
+
+   if (ma_count >= ma->max) {
+   err = tbl_mask_array_realloc(tbl, ma->max +
+ MASK_ARRAY_SIZE_MIN);
+   if (err)
+   return err;
+
+   ma = ovsl_dereference(tbl->mask_array);
+   }
+
+   BUG_ON(ovsl_dereference(ma->masks[ma_count]));
+
+   rcu_assign_pointer(ma->masks[ma_count], new);
+   WRITE_ONCE(ma->count, ma_count +1);
+
+   return 0;
+}
+
+static void tbl_mask_array_del_mask(struct flow_table *tbl,
+   struct sw_flow_mask *mask)
+{
+   struct mask_array *ma = ovsl_dereference(tbl->mask_array);
+   int i, ma_count = READ_ONCE(ma->count);
+
+   /* Remove the deleted mask pointers from the array */
+   for (i = 0; i < ma_count; i++) {
+   if (mask == ovsl_dereference(ma->masks[i]))
+   goto found;
+   }
+
+   BUG();
+   return;
+
+found:
+   WRITE_ONCE(ma->count, ma_count -1);
+
+   rcu_assign_pointer(ma->masks[i], ma->masks[ma_count -1]);
+   RCU_INIT_POINTER(ma->masks[ma_count -1], NULL);
+
+   kfree_rcu(mask, rcu);
+
+   /* Shrink the mask array if necessary. */
+   if (ma->max >= (MASK_ARRAY_SIZE_MIN * 2) &&
+   ma_count <= (ma->max / 3))
+   tbl_mask_array_realloc(tbl, ma->max / 2);
+}
+
+/* Remove 'mask' from the mask list, if it is not needed any more. */
+static void flow_mask_remove(struct flow_table *tbl, struct sw_flow_mask *mask)
+{
+   if (mask) {
+   /* ovs-lock is required to protect mask-refcount and
+* mask list.
+*/
+   ASSERT_OVSL();
+   BUG_ON(!mask->ref_count);
+   mask->ref_count--;
+
+   if (!mask->ref_count)
+   tbl_mask_array_del_mask(tbl, mask);
+   }
+}
+
 int ovs_flow_tbl_init(struct flow_table *table)
 {
struct table_instance *ti, *ufid_ti;
@@ -280,7 +348,28 @@ static void flow_tbl_destroy_rcu_cb(struct rcu_head *rcu)
__table_instance_destroy(ti);
 }
 
-static void table_instance_destroy(struct table_instance *ti,
+static void table_instance_flow_free(struct flow_table *table,
+ struct table_instance *ti,
+ struct table_instance *ufid_ti,
+ struct sw_flow *flow,
+ bool count)
+{
+   hlist_del_rcu(>flow_table.node[ti->node_ver]);
+   if (count)
+   table->count--;
+
+   if (ovs_identifier_is_ufid(>id)) {
+   hlist_del_rcu(>ufid_table.node[ufid_ti->node_ver]);
+
+   if (count)
+   table->ufid_count--;
+   }
+
+   flow_mask_remove(table, flow->mask);
+}
+
+static void table_instance_destroy(struct flow_table *table,
+  struct table_instance *ti,
   struct table_instance *ufid_ti,
   bool deferred)
 {
@@ -297,13 +386,12 @@ static void table_instance_destroy(struct table_instance 
*ti,
struct sw_flow *flow;
struct hlist_head *head = >buckets[i];
struct hlist_node *n;
-   int ver = ti->node_ver;
-   int ufid_ver = ufid_ti->node_ver;
 
-   hlist_for_each_entry_safe(flow, n, head, flow_table.node[ver]) {
-   hlist_del_rcu(>flow_table.node[ver]);
-   if (ovs_identifier_is_ufid(>id))
-   hlist_del_rcu(>ufid_table.node[ufid_ver]);
+   hlist_for_each_entry_safe(flow, n, head,
+

[ovs-dev] [PATCH v2 23/24] travis: Update kernel list as of 5.8

2020-09-09 Thread Greg Rose 
Update the list to more closely track the LTS releases on kernel.org.

Signed-off-by: Greg Rose 
---
 .travis.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 43e6a75cc..9fd8bbe01 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -38,8 +38,8 @@ env:
   - TESTSUITE=1 OPTS="--enable-shared"
   - TESTSUITE=1 DPDK=1
   - TESTSUITE=1 LIBS=-ljemalloc
-  - KERNEL_LIST="5.5  4.20 4.19 4.18 4.17 4.16"
-  - KERNEL_LIST="4.15 4.14 4.9  4.4  3.19 3.16"
+  - KERNEL_LIST="5.8 5.5 5.4 4.19"
+  - KERNEL_LIST="4.14 4.9 4.4 3.16"
   - AFXDP=1 KERNEL=5.3
   - M32=1 OPTS="--disable-ssl"
   - DPDK=1 OPTS="--enable-shared"
-- 
2.17.1

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

Re: [ovs-dev] [PATCH v2 00/24] Add support for Linux kernels up to 5.8.x

2020-09-09 Thread Gregory Rose 




On 9/9/2020 12:19 PM, Greg Rose wrote:

This patch set will add support for Linux kernels up to 5.9. In
addition there are quite a few patches for openvswitch on the Linux
kernel mailing list that have not been backported - here is a first
batch attempting to catch up on some of that technical debt.  There
will be a follow up batch of patches to this one but I didn't want
the patch bomb to get too large.


Ah, forgot something.  Passes Travis here:
https://travis-ci.org/github/gvrose8192/ovs-experimental/build/725651905

- Greg



---
V2 - V2 of this patch set changes the NEWS as suggested by Ilya
- Moves the acinclude patch for 5.8 support to the end of the
  patch series
- Reduces targeted Linux kernel support to 5.8 since 5.9 is
  still not baked
- Updates the travis kernel test list
- Adds tags from authors from the first patch series.

Eelco Chaudron (1):
   datapath: return an error instead of doing BUG_ON()

Enrico Weigelt (1):
   datapath: drop unneeded likely() call around IS_ERR()

Greg Rose (3):
   acinclude: Enable builds up to Linux 5.8
   travis: Update kernel list as of 5.8
   Documentation: Update faq and NEWS for kernel 5.8

Guillaume Nault (1):
   datapath: fix GFP flags in rtnl_net_notifyid()

Jason A. Donenfeld (1):
   datapath: use skb_list_walk_safe helper for gso segments

Kees Cook (1):
   datapath: Distribute switch variables for initialization

Paolo Abeni (3):
   datapath: fix flow command message size
   datapath: drop unneeded BUG_ON() in ovs_flow_cmd_build_info()
   datapath: remove another BUG_ON()

Paul Blakey (1):
   datapath: Set OvS recirc_id from tc chain index

Taehee Yoo (1):
   datapath: do not update max_headroom if new headroom is equal to old
 headroom

Tonghao Zhang (9):
   datapath: don't unlock mutex when changing the user_features fails
   datapath: optimize flow-mask looking up
   datapath: simplify the flow_hash
   datapath: add likely in flow_lookup
   datapath: fix possible memleak on destroy flow-table
   datapath: simplify the ovs_dp_cmd_new
   datapath: select vport upcall portid directly
   datapath: don't call pad_packet if not necessary
   datapath: use hlist_for_each_entry_rcu instead of hlist_for_each_entry

Yifeng Sun (1):
   datapath: Print error when ovs_execute_actions() fails

aaron conole (1):
   datapath: support asymmetric conntrack

  .travis.yml   |   4 +-
  Documentation/faq/releases.rst|   1 +
  NEWS  |   2 +
  acinclude.m4  |   7 +-
  datapath/conntrack.c  |  11 +
  datapath/datapath.c   | 220 --
  datapath/datapath.h   |   2 +
  datapath/flow.c   |  13 ++
  datapath/flow_netlink.c   |  18 +-
  datapath/flow_table.c | 214 +
  .../linux/compat/include/linux/compiler.h |  13 ++
  datapath/linux/compat/include/linux/skbuff.h  |   7 +
  .../linux/compat/include/linux/static_key.h   |   7 +
  datapath/vport.c  |   5 +-
  14 files changed, 332 insertions(+), 192 deletions(-)


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

[ovs-dev] [PATCH v2 15/24] datapath: fix flow command message size

2020-09-09 Thread Greg Rose 
From: Paolo Abeni 

Upstream commit:
commit 4e81c0b3fa93d07653e2415fa71656b080a112fd
Author: Paolo Abeni 
Date:   Tue Nov 26 12:55:50 2019 +0100

openvswitch: fix flow command message size

When user-space sets the OVS_UFID_F_OMIT_* flags, and the relevant
flow has no UFID, we can exceed the computed size, as
ovs_nla_put_identifier() will always dump an OVS_FLOW_ATTR_KEY
attribute.
Take the above in account when computing the flow command message
size.

Fixes: 74ed7ab9264c ("openvswitch: Add support for unique flow IDs.")
Reported-by: Qi Jun Ding 
Signed-off-by: Paolo Abeni 
Signed-off-by: David S. Miller 

Cc: Paolo Abeni 
Signed-off-by: Greg Rose 
---
 datapath/datapath.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index b9ac67635..047b3312e 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -763,9 +763,13 @@ static size_t ovs_flow_cmd_msg_size(const struct 
sw_flow_actions *acts,
 {
size_t len = NLMSG_ALIGN(sizeof(struct ovs_header));
 
-   /* OVS_FLOW_ATTR_UFID */
+   /* OVS_FLOW_ATTR_UFID, or unmasked flow key as fallback
+* see ovs_nla_put_identifier()
+*/
if (sfid && ovs_identifier_is_ufid(sfid))
len += nla_total_size(sfid->ufid_len);
+   else
+   len += nla_total_size(ovs_key_attr_size());
 
/* OVS_FLOW_ATTR_KEY */
if (!sfid || should_fill_key(sfid, ufid_flags))
-- 
2.17.1

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

[ovs-dev] [PATCH v2 09/24] datapath: simplify the flow_hash

2020-09-09 Thread Greg Rose 
From: Tonghao Zhang 

Upstream commit:
commit 515b65a4b99197ae062a795ab4de919e6d04be04
Author: Tonghao Zhang 
Date:   Fri Nov 1 22:23:50 2019 +0800

net: openvswitch: simplify the flow_hash

Simplify the code and remove the unnecessary BUILD_BUG_ON.

Signed-off-by: Tonghao Zhang 
Tested-by: Greg Rose 
Acked-by: William Tu 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Cc: Tonghao Zhang 
Reviewed-by: Tonghao Zhang 
Signed-off-by: Greg Rose 
---
 datapath/flow_table.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/datapath/flow_table.c b/datapath/flow_table.c
index 62d726ddd..7efaa8044 100644
--- a/datapath/flow_table.c
+++ b/datapath/flow_table.c
@@ -455,13 +455,10 @@ err_free_ti:
 static u32 flow_hash(const struct sw_flow_key *key,
 const struct sw_flow_key_range *range)
 {
-   int key_start = range->start;
-   int key_end = range->end;
-   const u32 *hash_key = (const u32 *)((const u8 *)key + key_start);
-   int hash_u32s = (key_end - key_start) >> 2;
+   const u32 *hash_key = (const u32 *)((const u8 *)key + range->start);
 
/* Make sure number of hash bytes are multiple of u32. */
-   BUILD_BUG_ON(sizeof(long) % sizeof(u32));
+   int hash_u32s = range_n_bytes(range) >> 2;
 
return jhash2(hash_key, hash_u32s, 0);
 }
-- 
2.17.1

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

[ovs-dev] [PATCH v2 24/24] Documentation: Update faq and NEWS for kernel 5.8

2020-09-09 Thread Greg Rose 
Update the NEWS and faq now that we will support up to Linux kernel
5.8.

Signed-off-by: Greg Rose 
---
 Documentation/faq/releases.rst | 1 +
 NEWS   | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index 9d5d2c3e1..dcba97e16 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -72,6 +72,7 @@ Q: What Linux kernel versions does each Open vSwitch release 
work with?
 2.12.x   3.16 to 5.0
 2.13.x   3.16 to 5.0
 2.14.x   3.16 to 5.5
+2.15.x   3.16 to 5.8
  ==
 
 Open vSwitch userspace should also work with the Linux kernel module built
diff --git a/NEWS b/NEWS
index 2f67d5047..2cd0a7cbf 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,7 @@
 Post-v2.14.0
 -
+   - Linux datapath:
+ * Support for kernel versions up to 5.8.x.
 
 
 v2.14.0 - 17 Aug 2020
-- 
2.17.1

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

[ovs-dev] [PATCH v2 12/24] datapath: simplify the ovs_dp_cmd_new

2020-09-09 Thread Greg Rose 
From: Tonghao Zhang 

Upstream commit:
commit eec62eadd1d757b0743ccbde55973814f3ad396e
Author: Tonghao Zhang 
Date:   Fri Nov 1 22:23:54 2019 +0800

net: openvswitch: simplify the ovs_dp_cmd_new

use the specified functions to init resource.

Signed-off-by: Tonghao Zhang 
Tested-by: Greg Rose 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Cc: Tonghao Zhang 
Reviewed-by: Tonghao Zhang 
Signed-off-by: Greg Rose 
---
 datapath/datapath.c | 60 -
 1 file changed, 38 insertions(+), 22 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index c222ef88e..f04ce210c 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1665,6 +1665,31 @@ static int ovs_dp_change(struct datapath *dp, struct 
nlattr *a[])
return 0;
 }
 
+static int ovs_dp_stats_init(struct datapath *dp)
+{
+   dp->stats_percpu = netdev_alloc_pcpu_stats(struct dp_stats_percpu);
+   if (!dp->stats_percpu)
+   return -ENOMEM;
+
+   return 0;
+}
+
+static int ovs_dp_vport_init(struct datapath *dp)
+{
+   int i;
+
+   dp->ports = kmalloc_array(DP_VPORT_HASH_BUCKETS,
+ sizeof(struct hlist_head),
+ GFP_KERNEL);
+   if (!dp->ports)
+   return -ENOMEM;
+
+   for (i = 0; i < DP_VPORT_HASH_BUCKETS; i++)
+   INIT_HLIST_HEAD(>ports[i]);
+
+   return 0;
+}
+
 static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
 {
struct nlattr **a = info->attrs;
@@ -1673,7 +1698,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct 
genl_info *info)
struct datapath *dp;
struct vport *vport;
struct ovs_net *ovs_net;
-   int err, i;
+   int err;
 
err = -EINVAL;
if (!a[OVS_DP_ATTR_NAME] || !a[OVS_DP_ATTR_UPCALL_PID])
@@ -1686,35 +1711,26 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct 
genl_info *info)
err = -ENOMEM;
dp = kzalloc(sizeof(*dp), GFP_KERNEL);
if (dp == NULL)
-   goto err_free_reply;
+   goto err_destroy_reply;
 
ovs_dp_set_net(dp, sock_net(skb->sk));
 
/* Allocate table. */
err = ovs_flow_tbl_init(>table);
if (err)
-   goto err_free_dp;
+   goto err_destroy_dp;
 
-   dp->stats_percpu = netdev_alloc_pcpu_stats(struct dp_stats_percpu);
-   if (!dp->stats_percpu) {
-   err = -ENOMEM;
+   err = ovs_dp_stats_init(dp);
+   if (err)
goto err_destroy_table;
-   }
 
-   dp->ports = kmalloc_array(DP_VPORT_HASH_BUCKETS,
- sizeof(struct hlist_head),
- GFP_KERNEL);
-   if (!dp->ports) {
-   err = -ENOMEM;
-   goto err_destroy_percpu;
-   }
-
-   for (i = 0; i < DP_VPORT_HASH_BUCKETS; i++)
-   INIT_HLIST_HEAD(>ports[i]);
+   err = ovs_dp_vport_init(dp);
+   if (err)
+   goto err_destroy_stats;
 
err = ovs_meters_init(dp);
if (err)
-   goto err_destroy_ports_array;
+   goto err_destroy_ports;
 
/* Set up our datapath device. */
parms.name = nla_data(a[OVS_DP_ATTR_NAME]);
@@ -1764,15 +1780,15 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct 
genl_info *info)
 
 err_destroy_meters:
ovs_meters_exit(dp);
-err_destroy_ports_array:
+err_destroy_ports:
kfree(dp->ports);
-err_destroy_percpu:
+err_destroy_stats:
free_percpu(dp->stats_percpu);
 err_destroy_table:
ovs_flow_tbl_destroy(>table);
-err_free_dp:
+err_destroy_dp:
kfree(dp);
-err_free_reply:
+err_destroy_reply:
kfree_skb(reply);
 err:
return err;
-- 
2.17.1

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

[ovs-dev] [PATCH v2 03/24] datapath: do not update max_headroom if new headroom is equal to old headroom

2020-09-09 Thread Greg Rose 
From: Taehee Yoo 

Upstream commit:
commit 6b660c4177aaebdc73df7a3378f0e8b110aa4b51
Author: Taehee Yoo 
Date:   Sat Jul 6 01:08:09 2019 +0900

net: openvswitch: do not update max_headroom if new headroom is equal to 
old headroom

When a vport is deleted, the maximum headroom size would be changed.
If the vport which has the largest headroom is deleted,
the new max_headroom would be set.
But, if the new headroom size is equal to the old headroom size,
updating routine is unnecessary.

Signed-off-by: Taehee Yoo 
Tested-by: Greg Rose 
Reviewed-by: Greg Rose 
Signed-off-by: David S. Miller 

Cc: Taehee Yoo 
Signed-off-by: Greg Rose 
---
 datapath/datapath.c | 38 +++---
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 4c485c88a..2879f24ef 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -2072,10 +2072,9 @@ static struct vport *lookup_vport(struct net *net,
 
 }
 
-/* Called with ovs_mutex */
-static void update_headroom(struct datapath *dp)
+static unsigned int ovs_get_max_headroom(struct datapath *dp)
 {
-   unsigned dev_headroom, max_headroom = 0;
+   unsigned int dev_headroom, max_headroom = 0;
struct net_device *dev;
struct vport *vport;
int i;
@@ -2089,10 +2088,19 @@ static void update_headroom(struct datapath *dp)
}
}
 
-   dp->max_headroom = max_headroom;
+   return max_headroom;
+}
+
+/* Called with ovs_mutex */
+static void ovs_update_headroom(struct datapath *dp, unsigned int new_headroom)
+{
+   struct vport *vport;
+   int i;
+
+   dp->max_headroom = new_headroom;
for (i = 0; i < DP_VPORT_HASH_BUCKETS; i++)
hlist_for_each_entry_rcu(vport, >ports[i], dp_hash_node)
-   netdev_set_rx_headroom(vport->dev, max_headroom);
+   netdev_set_rx_headroom(vport->dev, new_headroom);
 }
 
 static int ovs_vport_cmd_new(struct sk_buff *skb, struct genl_info *info)
@@ -2103,6 +2111,7 @@ static int ovs_vport_cmd_new(struct sk_buff *skb, struct 
genl_info *info)
struct sk_buff *reply;
struct vport *vport;
struct datapath *dp;
+   unsigned int new_headroom;
u32 port_no;
int err;
 
@@ -2165,8 +2174,10 @@ restart:
  OVS_VPORT_CMD_NEW);
BUG_ON(err < 0);
 
-   if (netdev_get_fwd_headroom(vport->dev) > dp->max_headroom)
-   update_headroom(dp);
+   new_headroom = netdev_get_fwd_headroom(vport->dev);
+
+   if (new_headroom > dp->max_headroom)
+   ovs_update_headroom(dp, new_headroom);
else
netdev_set_rx_headroom(vport->dev, dp->max_headroom);
 
@@ -2235,11 +2246,12 @@ exit_unlock_free:
 
 static int ovs_vport_cmd_del(struct sk_buff *skb, struct genl_info *info)
 {
-   bool must_update_headroom = false;
+   bool update_headroom = false;
struct nlattr **a = info->attrs;
struct sk_buff *reply;
struct datapath *dp;
struct vport *vport;
+   unsigned int new_headroom;
int err;
 
reply = ovs_vport_cmd_alloc_info();
@@ -2265,13 +2277,17 @@ static int ovs_vport_cmd_del(struct sk_buff *skb, 
struct genl_info *info)
/* the vport deletion may trigger dp headroom update */
dp = vport->dp;
if (netdev_get_fwd_headroom(vport->dev) == dp->max_headroom)
-   must_update_headroom = true;
+   update_headroom = true;
+
netdev_reset_rx_headroom(vport->dev);
ovs_dp_detach_port(vport);
 
-   if (must_update_headroom)
-   update_headroom(dp);
+   if (update_headroom) {
+   new_headroom = ovs_get_max_headroom(dp);
 
+   if (new_headroom < dp->max_headroom)
+   ovs_update_headroom(dp, new_headroom);
+   }
ovs_unlock();
 
ovs_notify(_vport_genl_family, _dp_vport_multicast_group, reply, 
info);
-- 
2.17.1

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

[ovs-dev] [PATCH v2 10/24] datapath: add likely in flow_lookup

2020-09-09 Thread Greg Rose 
From: Tonghao Zhang 

Upstream commit:
commit 0a3e01371db17d753dd92ec4d0fc6247412d3b01
Author: Tonghao Zhang 
Date:   Fri Nov 1 22:23:51 2019 +0800

net: openvswitch: add likely in flow_lookup

The most case *index < ma->max, and flow-mask is not NULL.
We add un/likely for performance.

Signed-off-by: Tonghao Zhang 
Tested-by: Greg Rose 
Acked-by: William Tu 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Cc: Tonghao Zhang 
Reviewed-by: Tonghao Zhang 
Signed-off-by: Greg Rose 
---
 datapath/flow_table.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/datapath/flow_table.c b/datapath/flow_table.c
index 7efaa8044..ca2efe94d 100644
--- a/datapath/flow_table.c
+++ b/datapath/flow_table.c
@@ -541,7 +541,7 @@ static struct sw_flow *flow_lookup(struct flow_table *tbl,
struct sw_flow_mask *mask;
int i;
 
-   if (*index < ma->max) {
+   if (likely(*index < ma->max)) {
mask = rcu_dereference_ovsl(ma->masks[*index]);
if (mask) {
flow = masked_flow_lookup(ti, key, mask, n_mask_hit);
@@ -556,7 +556,7 @@ static struct sw_flow *flow_lookup(struct flow_table *tbl,
continue;
 
mask = rcu_dereference_ovsl(ma->masks[i]);
-   if (!mask)
+   if (unlikely(!mask))
break;
 
flow = masked_flow_lookup(ti, key, mask, n_mask_hit);
-- 
2.17.1

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

[ovs-dev] [PATCH v2 06/24] datapath: fix GFP flags in rtnl_net_notifyid()

2020-09-09 Thread Greg Rose 
From: Guillaume Nault 

Upstream commit:
commit d4e4fdf9e4a27c87edb79b1478955075be141f67
Author: Guillaume Nault 
Date:   Wed Oct 23 18:39:04 2019 +0200

netns: fix GFP flags in rtnl_net_notifyid()

In rtnl_net_notifyid(), we certainly can't pass a null GFP flag to
rtnl_notify(). A GFP_KERNEL flag would be fine in most circumstances,
but there are a few paths calling rtnl_net_notifyid() from atomic
context or from RCU critical sections. The later also precludes the use
of gfp_any() as it wouldn't detect the RCU case. Also, the nlmsg_new()
call is wrong too, as it uses GFP_KERNEL unconditionally.

Therefore, we need to pass the GFP flags as parameter and propagate it
through function calls until the proper flags can be determined.

In most cases, GFP_KERNEL is fine. The exceptions are:
  * openvswitch: ovs_vport_cmd_get() and ovs_vport_cmd_dump()
indirectly call rtnl_net_notifyid() from RCU critical section,

  * rtnetlink: rtmsg_ifinfo_build_skb() already receives GFP flags as
parameter.

Also, in ovs_vport_cmd_build_info(), let's change the GFP flags used
by nlmsg_new(). The function is allowed to sleep, so better make the
flags consistent with the ones used in the following
ovs_vport_cmd_fill_info() call.

Found by code inspection.

Fixes: 9a9634545c70 ("netns: notify netns id events")
Signed-off-by: Guillaume Nault 
Acked-by: Nicolas Dichtel 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Backport the datapath.c portion of this fix.

Cc:  Guillaume Nault 
Signed-off-by: Greg Rose 
---
 datapath/datapath.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 36f1e7894..1235c4e3d 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1990,7 +1990,7 @@ static struct genl_family dp_datapath_genl_family 
__ro_after_init = {
 /* Called with ovs_mutex or RCU read lock. */
 static int ovs_vport_cmd_fill_info(struct vport *vport, struct sk_buff *skb,
   struct net *net, u32 portid, u32 seq,
-  u32 flags, u8 cmd)
+  u32 flags, u8 cmd, gfp_t gfp)
 {
struct ovs_header *ovs_header;
struct ovs_vport_stats vport_stats;
@@ -2012,7 +2012,7 @@ static int ovs_vport_cmd_fill_info(struct vport *vport, 
struct sk_buff *skb,
 
 #ifdef HAVE_PEERNET2ID_ALLOC
if (!net_eq(net, dev_net(vport->dev))) {
-   int id = peernet2id_alloc(net, dev_net(vport->dev));
+   int id = peernet2id_alloc(net, dev_net(vport->dev), gfp);
 
if (nla_put_s32(skb, OVS_VPORT_ATTR_NETNSID, id))
goto nla_put_failure;
@@ -2054,11 +2054,12 @@ struct sk_buff *ovs_vport_cmd_build_info(struct vport 
*vport, struct net *net,
struct sk_buff *skb;
int retval;
 
-   skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+   skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
if (!skb)
return ERR_PTR(-ENOMEM);
 
-   retval = ovs_vport_cmd_fill_info(vport, skb, net, portid, seq, 0, cmd);
+   retval = ovs_vport_cmd_fill_info(vport, skb, net, portid, seq, 0, cmd,
+GFP_KERNEL);
BUG_ON(retval < 0);
 
return skb;
@@ -2200,7 +2201,7 @@ restart:
 
err = ovs_vport_cmd_fill_info(vport, reply, genl_info_net(info),
  info->snd_portid, info->snd_seq, 0,
- OVS_VPORT_CMD_NEW);
+ OVS_VPORT_CMD_NEW, GFP_KERNEL);
BUG_ON(err < 0);
 
new_headroom = netdev_get_fwd_headroom(vport->dev);
@@ -2260,7 +2261,7 @@ static int ovs_vport_cmd_set(struct sk_buff *skb, struct 
genl_info *info)
 
err = ovs_vport_cmd_fill_info(vport, reply, genl_info_net(info),
  info->snd_portid, info->snd_seq, 0,
- OVS_VPORT_CMD_SET);
+ OVS_VPORT_CMD_SET, GFP_KERNEL);
BUG_ON(err < 0);
ovs_unlock();
 
@@ -2300,7 +2301,7 @@ static int ovs_vport_cmd_del(struct sk_buff *skb, struct 
genl_info *info)
 
err = ovs_vport_cmd_fill_info(vport, reply, genl_info_net(info),
  info->snd_portid, info->snd_seq, 0,
- OVS_VPORT_CMD_DEL);
+ OVS_VPORT_CMD_DEL, GFP_KERNEL);
BUG_ON(err < 0);
 
/* the vport deletion may trigger dp headroom update */
@@ -2347,7 +2348,7 @@ static int ovs_vport_cmd_get(struct sk_buff *skb, struct 
genl_info *info)
goto exit_unlock_free;
err = ovs_vport_cmd_fill_info(vport, reply, genl_info_net(info),
  info->snd_portid, info->snd_seq, 0,
-

[ovs-dev] [PATCH v2 20/24] datapath: Distribute switch variables for initialization

2020-09-09 Thread Greg Rose 
From: Kees Cook 

Upstream commit:
commit 16a556eeb7ed2dc3709fe2c5be76accdfa4901ab
Author: Kees Cook 
Date:   Wed Feb 19 22:23:09 2020 -0800

openvswitch: Distribute switch variables for initialization

Variables declared in a switch statement before any case statements
cannot be automatically initialized with compiler instrumentation (as
they are not part of any execution flow). With GCC's proposed automatic
stack variable initialization feature, this triggers a warning (and they
don't get initialized). Clang's automatic stack variable initialization
(via CONFIG_INIT_STACK_ALL=y) doesn't throw a warning, but it also
doesn't initialize such variables[1]. Note that these warnings (or silent
skipping) happen before the dead-store elimination optimization phase,
so even when the automatic initializations are later elided in favor of
direct initializations, the warnings remain.

To avoid these problems, move such variables into the "case" where
they're used or lift them up into the main function body.

net/openvswitch/flow_netlink.c: In function ‘validate_set’:
net/openvswitch/flow_netlink.c:2711:29: warning: statement will never be 
executed [-Wswitch-unreachable]
 2711 |  const struct ovs_key_ipv4 *ipv4_key;
  | ^~~~

[1] https://bugs.llvm.org/show_bug.cgi?id=44916

Signed-off-by: Kees Cook 
Signed-off-by: David S. Miller 

Cc: Kees Cook 
Signed-off-by: Greg Rose 
---
 datapath/flow_netlink.c | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index d3fd77106..996041602 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -2700,10 +2700,6 @@ static int validate_set(const struct nlattr *a,
return -EINVAL;
 
switch (key_type) {
-   const struct ovs_key_ipv4 *ipv4_key;
-   const struct ovs_key_ipv6 *ipv6_key;
-   int err;
-
case OVS_KEY_ATTR_PRIORITY:
case OVS_KEY_ATTR_SKB_MARK:
case OVS_KEY_ATTR_CT_MARK:
@@ -2715,7 +2711,9 @@ static int validate_set(const struct nlattr *a,
return -EINVAL;
break;
 
-   case OVS_KEY_ATTR_TUNNEL:
+   case OVS_KEY_ATTR_TUNNEL: {
+   int err;
+
 #ifndef USE_UPSTREAM_TUNNEL
if (eth_p_mpls(eth_type))
return -EINVAL;
@@ -2728,8 +2726,10 @@ static int validate_set(const struct nlattr *a,
if (err)
return err;
break;
+   }
+   case OVS_KEY_ATTR_IPV4: {
+   const struct ovs_key_ipv4 *ipv4_key;
 
-   case OVS_KEY_ATTR_IPV4:
if (eth_type != htons(ETH_P_IP))
return -EINVAL;
 
@@ -2749,8 +2749,10 @@ static int validate_set(const struct nlattr *a,
return -EINVAL;
}
break;
+   }
+   case OVS_KEY_ATTR_IPV6: {
+   const struct ovs_key_ipv6 *ipv6_key;
 
-   case OVS_KEY_ATTR_IPV6:
if (eth_type != htons(ETH_P_IPV6))
return -EINVAL;
 
@@ -2777,7 +2779,7 @@ static int validate_set(const struct nlattr *a,
return -EINVAL;
 
break;
-
+   }
case OVS_KEY_ATTR_TCP:
if ((eth_type != htons(ETH_P_IP) &&
 eth_type != htons(ETH_P_IPV6)) ||
-- 
2.17.1

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

[ovs-dev] [PATCH v2 01/24] datapath: return an error instead of doing BUG_ON()

2020-09-09 Thread Greg Rose 
From: Eelco Chaudron 

Upstream commit:
commit a734d1f4c2fc962ef4daa179e216df84a8ec5f84
Author: Eelco Chaudron 
Date:   Thu May 2 16:12:38 2019 -0400

net: openvswitch: return an error instead of doing BUG_ON()

For all other error cases in queue_userspace_packet() the error is
returned, so it makes sense to do the same for these two error cases.

Reported-by: Davide Caratti 
Signed-off-by: Eelco Chaudron 
Acked-by: Flavio Leitner 
Signed-off-by: David S. Miller 

Cc: Eelco Chaudron 
Signed-off-by: Greg Rose 
---
 datapath/datapath.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 05c1e4274..d604bfd36 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -469,7 +469,8 @@ static int queue_userspace_packet(struct datapath *dp, 
struct sk_buff *skb,
upcall->dp_ifindex = dp_ifindex;
 
err = ovs_nla_put_key(key, key, OVS_PACKET_ATTR_KEY, false, user_skb);
-   BUG_ON(err);
+   if (err)
+   goto out;
 
if (upcall_info->userdata)
__nla_put(user_skb, OVS_PACKET_ATTR_USERDATA,
@@ -486,7 +487,9 @@ static int queue_userspace_packet(struct datapath *dp, 
struct sk_buff *skb,
}
err = ovs_nla_put_tunnel_info(user_skb,
  upcall_info->egress_tun_info);
-   BUG_ON(err);
+   if (err)
+   goto out;
+
nla_nest_end(user_skb, nla);
}
 
-- 
2.17.1

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

[ovs-dev] [PATCH v2 04/24] datapath: Print error when ovs_execute_actions() fails

2020-09-09 Thread Greg Rose 
From: Yifeng Sun 

Upstream commit:
commit aa733660dbd8d9192b8c528ae0f4b84f3fef74e4
Author: Yifeng Sun 
Date:   Sun Aug 4 19:56:11 2019 -0700

openvswitch: Print error when ovs_execute_actions() fails

Currently in function ovs_dp_process_packet(), return values of
ovs_execute_actions() are silently discarded. This patch prints out
an debug message when error happens so as to provide helpful hints
for debugging.
Acked-by: Pravin B Shelar 

Signed-off-by: David S. Miller 

Cc: Yifeng Sun 
Reviewed-by: Yifeng Sun 
Signed-off-by: Greg Rose 
---
 datapath/datapath.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 2879f24ef..c8c21d774 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -240,6 +240,7 @@ void ovs_dp_process_packet(struct sk_buff *skb, struct 
sw_flow_key *key)
struct dp_stats_percpu *stats;
u64 *stats_counter;
u32 n_mask_hit;
+   int error;
 
stats = this_cpu_ptr(dp->stats_percpu);
 
@@ -248,7 +249,6 @@ void ovs_dp_process_packet(struct sk_buff *skb, struct 
sw_flow_key *key)
 _mask_hit);
if (unlikely(!flow)) {
struct dp_upcall_info upcall;
-   int error;
 
memset(, 0, sizeof(upcall));
upcall.cmd = OVS_PACKET_CMD_MISS;
@@ -265,7 +265,10 @@ void ovs_dp_process_packet(struct sk_buff *skb, struct 
sw_flow_key *key)
 
ovs_flow_stats_update(flow, key->tp.flags, skb);
sf_acts = rcu_dereference(flow->sf_acts);
-   ovs_execute_actions(dp, skb, sf_acts, key);
+   error = ovs_execute_actions(dp, skb, sf_acts, key);
+   if (unlikely(error))
+   net_dbg_ratelimited("ovs: action execution error on datapath 
%s: %d\n",
+   ovs_dp_name(dp), error);
 
stats_counter = >n_hit;
 
-- 
2.17.1

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

[ovs-dev] [PATCH v2 00/24] Add support for Linux kernels up to 5.8.x

2020-09-09 Thread Greg Rose 
This patch set will add support for Linux kernels up to 5.9. In
addition there are quite a few patches for openvswitch on the Linux
kernel mailing list that have not been backported - here is a first
batch attempting to catch up on some of that technical debt.  There
will be a follow up batch of patches to this one but I didn't want
the patch bomb to get too large.

---
V2 - V2 of this patch set changes the NEWS as suggested by Ilya
   - Moves the acinclude patch for 5.8 support to the end of the
 patch series
   - Reduces targeted Linux kernel support to 5.8 since 5.9 is
 still not baked
   - Updates the travis kernel test list
   - Adds tags from authors from the first patch series.

Eelco Chaudron (1):
  datapath: return an error instead of doing BUG_ON()

Enrico Weigelt (1):
  datapath: drop unneeded likely() call around IS_ERR()

Greg Rose (3):
  acinclude: Enable builds up to Linux 5.8
  travis: Update kernel list as of 5.8
  Documentation: Update faq and NEWS for kernel 5.8

Guillaume Nault (1):
  datapath: fix GFP flags in rtnl_net_notifyid()

Jason A. Donenfeld (1):
  datapath: use skb_list_walk_safe helper for gso segments

Kees Cook (1):
  datapath: Distribute switch variables for initialization

Paolo Abeni (3):
  datapath: fix flow command message size
  datapath: drop unneeded BUG_ON() in ovs_flow_cmd_build_info()
  datapath: remove another BUG_ON()

Paul Blakey (1):
  datapath: Set OvS recirc_id from tc chain index

Taehee Yoo (1):
  datapath: do not update max_headroom if new headroom is equal to old
headroom

Tonghao Zhang (9):
  datapath: don't unlock mutex when changing the user_features fails
  datapath: optimize flow-mask looking up
  datapath: simplify the flow_hash
  datapath: add likely in flow_lookup
  datapath: fix possible memleak on destroy flow-table
  datapath: simplify the ovs_dp_cmd_new
  datapath: select vport upcall portid directly
  datapath: don't call pad_packet if not necessary
  datapath: use hlist_for_each_entry_rcu instead of hlist_for_each_entry

Yifeng Sun (1):
  datapath: Print error when ovs_execute_actions() fails

aaron conole (1):
  datapath: support asymmetric conntrack

 .travis.yml   |   4 +-
 Documentation/faq/releases.rst|   1 +
 NEWS  |   2 +
 acinclude.m4  |   7 +-
 datapath/conntrack.c  |  11 +
 datapath/datapath.c   | 220 --
 datapath/datapath.h   |   2 +
 datapath/flow.c   |  13 ++
 datapath/flow_netlink.c   |  18 +-
 datapath/flow_table.c | 214 +
 .../linux/compat/include/linux/compiler.h |  13 ++
 datapath/linux/compat/include/linux/skbuff.h  |   7 +
 .../linux/compat/include/linux/static_key.h   |   7 +
 datapath/vport.c  |   5 +-
 14 files changed, 332 insertions(+), 192 deletions(-)

-- 
2.17.1

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

[ovs-dev] [PATCH v2 02/24] datapath: drop unneeded likely() call around IS_ERR()

2020-09-09 Thread Greg Rose 
From: Enrico Weigelt 

Upstream commit:
commit b90f5aa4d6268e81dd1fd51e5ef89d2892bf040d
Author: Enrico Weigelt 
Date:   Wed Jun 5 23:06:40 2019 +0200

net: openvswitch: drop unneeded likely() call around IS_ERR()

IS_ERR() already calls unlikely(), so this extra likely() call
around the !IS_ERR() is not needed.

Signed-off-by: Enrico Weigelt 
Signed-off-by: David S. Miller 

Cc: Enrico Weigelt 
Signed-off-by: Greg Rose 
---
 datapath/datapath.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index d604bfd36..4c485c88a 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1402,7 +1402,7 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct 
genl_info *info)
>id, info, false, ufid_flags);
 
if (likely(reply)) {
-   if (likely(!IS_ERR(reply))) {
+   if (!IS_ERR(reply)) {
rcu_read_lock();/*To keep RCU checker happy. */
err = ovs_flow_cmd_fill_info(flow, 
ovs_header->dp_ifindex,
 reply, info->snd_portid,
-- 
2.17.1

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

[ovs-dev] [PATCH v2 17/24] datapath: remove another BUG_ON()

2020-09-09 Thread Greg Rose 
From: Paolo Abeni 

Upstream commit:
commit 8a574f86652a4540a2433946ba826ccb87f398cc
Author: Paolo Abeni 
Date:   Sun Dec 1 18:41:25 2019 +0100

openvswitch: remove another BUG_ON()

If we can't build the flow del notification, we can simply delete
the flow, no need to crash the kernel. Still keep a WARN_ON to
preserve debuggability.

Note: the BUG_ON() predates the Fixes tag, but this change
can be applied only after the mentioned commit.

v1 -> v2:
 - do not leak an skb on error

Fixes: aed067783e50 ("openvswitch: Minimize ovs_flow_cmd_del critical 
section.")
Signed-off-by: Paolo Abeni 
Signed-off-by: David S. Miller 

Cc: Paolo Abeni 
Signed-off-by: Greg Rose 
---
 datapath/datapath.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 877c8bdba..deffb3a60 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1414,7 +1414,10 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct 
genl_info *info)
 OVS_FLOW_CMD_DEL,
 ufid_flags);
rcu_read_unlock();
-   BUG_ON(err < 0);
+   if (WARN_ON_ONCE(err < 0)) {
+   kfree_skb(reply);
+   goto out_free;
+   }
ovs_notify(_flow_genl_family, 
_dp_flow_multicast_group, reply, info);
} else {
genl_set_err(_flow_genl_family, sock_net(skb->sk), 0,
@@ -1423,6 +1426,7 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct 
genl_info *info)
}
}
 
+out_free:
ovs_flow_free(flow, true);
return 0;
 unlock:
-- 
2.17.1

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

[ovs-dev] [PATCH v2 16/24] datapath: drop unneeded BUG_ON() in ovs_flow_cmd_build_info()

2020-09-09 Thread Greg Rose 
From: Paolo Abeni 

Upstream commit:
commit 8ffeb03fbba3b599690b361467bfd2373e8c450f
Author: Paolo Abeni 
Date:   Sun Dec 1 18:41:24 2019 +0100

openvswitch: drop unneeded BUG_ON() in ovs_flow_cmd_build_info()

All the callers of ovs_flow_cmd_build_info() already deal with
error return code correctly, so we can handle the error condition
in a more gracefull way. Still dump a warning to preserve
debuggability.

v1 -> v2:
 - clarify the commit message
 - clean the skb and report the error (DaveM)

Fixes: ccb1352e76cf ("net: Add Open vSwitch kernel components.")
Signed-off-by: Paolo Abeni 
Signed-off-by: David S. Miller 

Cc: Paolo Abeni 
Signed-off-by: Greg Rose 
---
 datapath/datapath.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 047b3312e..877c8bdba 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -946,7 +946,10 @@ static struct sk_buff *ovs_flow_cmd_build_info(const 
struct sw_flow *flow,
retval = ovs_flow_cmd_fill_info(flow, dp_ifindex, skb,
info->snd_portid, info->snd_seq, 0,
cmd, ufid_flags);
-   BUG_ON(retval < 0);
+   if (WARN_ON_ONCE(retval < 0)) {
+   kfree_skb(skb);
+   skb = ERR_PTR(retval);
+   }
return skb;
 }
 
-- 
2.17.1

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

[ovs-dev] [PATCH v2 19/24] datapath: use skb_list_walk_safe helper for gso segments

2020-09-09 Thread Greg Rose 
From: "Jason A. Donenfeld" 

Upstream commit:
commit 2cec4448db38758832c2edad439f99584bb8fa0d
Author: Jason A. Donenfeld 
Date:   Mon Jan 13 18:42:29 2020 -0500

net: openvswitch: use skb_list_walk_safe helper for gso segments

This is a straight-forward conversion case for the new function, keeping
the flow of the existing code as intact as possible.

Signed-off-by: Jason A. Donenfeld 
Signed-off-by: David S. Miller 

Cc: Jason A. Donenfeld 
Signed-off-by: Greg Rose 
---
 datapath/datapath.c  | 11 ---
 datapath/linux/compat/include/linux/skbuff.h |  7 +++
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index deffb3a60..4b71e2c46 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -343,8 +343,7 @@ static int queue_gso_packets(struct datapath *dp, struct 
sk_buff *skb,
}
 #endif
/* Queue all of the segments. */
-   skb = segs;
-   do {
+   skb_list_walk_safe(segs, skb, nskb) {
*OVS_CB(skb) = ovs_cb;
 #ifdef HAVE_SKB_GSO_UDP
if (gso_type & SKB_GSO_UDP && skb != segs)
@@ -354,17 +353,15 @@ static int queue_gso_packets(struct datapath *dp, struct 
sk_buff *skb,
if (err)
break;
 
-   } while ((skb = skb->next));
+   }
 
/* Free all of the segments. */
-   skb = segs;
-   do {
-   nskb = skb->next;
+   skb_list_walk_safe(segs, skb, nskb) {
if (err)
kfree_skb(skb);
else
consume_skb(skb);
-   } while ((skb = nskb));
+   }
return err;
 }
 
diff --git a/datapath/linux/compat/include/linux/skbuff.h 
b/datapath/linux/compat/include/linux/skbuff.h
index 6d248b3ed..204ce5497 100644
--- a/datapath/linux/compat/include/linux/skbuff.h
+++ b/datapath/linux/compat/include/linux/skbuff.h
@@ -487,4 +487,11 @@ static inline __u32 skb_get_hash_raw(const struct sk_buff 
*skb)
 }
 #endif
 
+#ifndef skb_list_walk_safe
+/* Iterate through singly-linked GSO fragments of an skb. */
+#define skb_list_walk_safe(first, skb, next_skb)   
\
+   for ((skb) = (first), (next_skb) = (skb) ? (skb)->next : NULL; (skb);  \
+(skb) = (next_skb), (next_skb) = (skb) ? (skb)->next : NULL)
+#endif
+
 #endif
-- 
2.17.1

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

[ovs-dev] [PATCH v2 14/24] datapath: don't call pad_packet if not necessary

2020-09-09 Thread Greg Rose 
From: Tonghao Zhang 

Upstream commit:
commit 61ca533c0e94104c35fcb7858a23ec9a05d78143
Author: Tonghao Zhang 
Date:   Thu Nov 14 23:51:08 2019 +0800

net: openvswitch: don't call pad_packet if not necessary

The nla_put_u16/nla_put_u32 makes sure that
*attrlen is align. The call tree is that:

nla_put_u16/nla_put_u32
  -> nla_putattrlen = sizeof(u16) or sizeof(u32)
  -> __nla_put  attrlen
  -> __nla_reserve  attrlen
  -> skb_put(skb, nla_total_size(attrlen))

nla_total_size returns the total length of attribute
including padding.

Cc: Joe Stringer 
Cc: William Tu 
Signed-off-by: Tonghao Zhang 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Cc: Tonghao Zhang 
Reviewed-by: Tonghao Zhang 
Signed-off-by: Greg Rose 
---
 datapath/datapath.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index f04ce210c..b9ac67635 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -512,23 +512,17 @@ static int queue_userspace_packet(struct datapath *dp, 
struct sk_buff *skb,
}
 
/* Add OVS_PACKET_ATTR_MRU */
-   if (upcall_info->mru) {
-   if (nla_put_u16(user_skb, OVS_PACKET_ATTR_MRU,
-   upcall_info->mru)) {
-   err = -ENOBUFS;
-   goto out;
-   }
-   pad_packet(dp, user_skb);
+   if (upcall_info->mru &&
+   nla_put_u16(user_skb, OVS_PACKET_ATTR_MRU, upcall_info->mru)) {
+   err = -ENOBUFS;
+   goto out;
}
 
/* Add OVS_PACKET_ATTR_LEN when packet is truncated */
-   if (cutlen > 0) {
-   if (nla_put_u32(user_skb, OVS_PACKET_ATTR_LEN,
-   skb->len)) {
-   err = -ENOBUFS;
-   goto out;
-   }
-   pad_packet(dp, user_skb);
+   if (cutlen > 0 &&
+   nla_put_u32(user_skb, OVS_PACKET_ATTR_LEN, skb->len)) {
+   err = -ENOBUFS;
+   goto out;
}
 
/* Add OVS_PACKET_ATTR_HASH */
-- 
2.17.1

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

[ovs-dev] [PATCH v2 05/24] datapath: Set OvS recirc_id from tc chain index

2020-09-09 Thread Greg Rose 
From: Paul Blakey 

Upstream commit:
commit 95a7233c452a58a4c2310c456c73997853b2ec46
Author: Paul Blakey 
Date:   Wed Sep 4 16:56:37 2019 +0300

net: openvswitch: Set OvS recirc_id from tc chain index

Offloaded OvS datapath rules are translated one to one to tc rules,
for example the following simplified OvS rule:

recirc_id(0),in_port(dev1),eth_type(0x0800),ct_state(-trk) 
actions:ct(),recirc(2)

Will be translated to the following tc rule:

$ tc filter add dev dev1 ingress \
prio 1 chain 0 proto ip \
flower tcp ct_state -trk \
action ct pipe \
action goto chain 2

Received packets will first travel though tc, and if they aren't stolen
by it, like in the above rule, they will continue to OvS datapath.
Since we already did some actions (action ct in this case) which might
modify the packets, and updated action stats, we would like to continue
the proccessing with the correct recirc_id in OvS (here recirc_id(2))
where we left off.

To support this, introduce a new skb extension for tc, which
will be used for translating tc chain to ovs recirc_id to
handle these miss cases. Last tc chain index will be set
by tc goto chain action and read by OvS datapath.

Signed-off-by: Paul Blakey 
Signed-off-by: Vlad Buslov 
Acked-by: Jiri Pirko 
Acked-by: Pravin B Shelar 
Signed-off-by: David S. Miller 

Backport the local datapath changes from this patch and add compat
layer fixup for the DECLARE_STATIC_KEY_FALSE macro.

Cc: Paul Blakey 
Signed-off-by: Greg Rose 
---
 acinclude.m4  |  3 ++
 datapath/datapath.c   | 34 ---
 datapath/datapath.h   |  2 ++
 datapath/flow.c   | 13 +++
 .../linux/compat/include/linux/static_key.h   |  7 
 5 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 84f344da0..3d56510a0 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -631,6 +631,9 @@ AC_DEFUN([OVS_CHECK_LINUX_COMPAT], [
   [OVS_DEFINE([HAVE_UPSTREAM_STATIC_KEY])])
   OVS_GREP_IFELSE([$KSRC/include/linux/jump_label.h], 
[DEFINE_STATIC_KEY_FALSE],
   [OVS_DEFINE([HAVE_DEFINE_STATIC_KEY])])
+  OVS_GREP_IFELSE([$KSRC/include/linux/jump_label.h],
+  [DECLARE_STATIC_KEY_FALSE],
+  [OVS_DEFINE([HAVE_DECLARE_STATIC_KEY])])
 
   OVS_GREP_IFELSE([$KSRC/include/linux/etherdevice.h], [eth_hw_addr_random])
   OVS_GREP_IFELSE([$KSRC/include/linux/etherdevice.h], [ether_addr_copy])
diff --git a/datapath/datapath.c b/datapath/datapath.c
index c8c21d774..36f1e7894 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1635,10 +1635,34 @@ static void ovs_dp_reset_user_features(struct sk_buff 
*skb, struct genl_info *in
dp->user_features = 0;
 }
 
-static void ovs_dp_change(struct datapath *dp, struct nlattr *a[])
+DEFINE_STATIC_KEY_FALSE(tc_recirc_sharing_support);
+
+static int ovs_dp_change(struct datapath *dp, struct nlattr *a[])
 {
-   if (a[OVS_DP_ATTR_USER_FEATURES])
-   dp->user_features = nla_get_u32(a[OVS_DP_ATTR_USER_FEATURES]);
+   u32 user_features = 0;
+
+   if (a[OVS_DP_ATTR_USER_FEATURES]) {
+   user_features = nla_get_u32(a[OVS_DP_ATTR_USER_FEATURES]);
+
+   if (user_features & ~(OVS_DP_F_VPORT_PIDS |
+ OVS_DP_F_UNALIGNED |
+ OVS_DP_F_TC_RECIRC_SHARING))
+   return -EOPNOTSUPP;
+
+#if !IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
+   if (user_features & OVS_DP_F_TC_RECIRC_SHARING)
+   return -EOPNOTSUPP;
+#endif
+   }
+
+   dp->user_features = user_features;
+
+   if (dp->user_features & OVS_DP_F_TC_RECIRC_SHARING)
+   static_branch_enable(_recirc_sharing_support);
+   else
+   static_branch_disable(_recirc_sharing_support);
+
+   return 0;
 }
 
 static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
@@ -1700,7 +1724,9 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct 
genl_info *info)
parms.port_no = OVSP_LOCAL;
parms.upcall_portids = a[OVS_DP_ATTR_UPCALL_PID];
 
-   ovs_dp_change(dp, a);
+   err = ovs_dp_change(dp, a);
+   if (err)
+   goto err_destroy_meters;
 
/* So far only local changes have been made, now need the lock. */
ovs_lock();
diff --git a/datapath/datapath.h b/datapath/datapath.h
index f99db1fde..c377e9b24 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -251,6 +251,8 @@ extern struct notifier_block ovs_dp_device_notifier;
 extern struct genl_family dp_vport_genl_family;
 extern const struct genl_multicast_group ovs_dp_vport_multicast_group;
 
+DECLARE_STATIC_KEY_FALSE(tc_recirc_sharing_support);
+
 void

Re: [ovs-dev] [PATCH v2] ofproto-dpif-xlate: Do not use zero-weight buckets in select groups.

2020-09-09 Thread Ilya Maximets 
On 6/8/19 1:28 AM, Ben Pfaff wrote:
> The OpenFlow specification says that buckets in select groups with a weight
> of zero should not be selected, but the ofproto-dpif implementation could
> select them in corner cases.  This fixes the problem.
> 
> Reported-by: ychen 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2019-May/359349.html
> Signed-off-by: Ben Pfaff 
> ---
> v1->v2: Simplify and fix figuring out whether a group is alive.

Hi, Ben.  I'm looking through old patches after the patchwork cleanup and this
one seems to be still valid.  It requires a minor rebase, but it's trivial.

Patch looks good to me in general and I could apply it.
What do you think?

It also looks like we will need to backport it.  Suggestions on how far it
should go are welcome.

Best regards, Ilya Maximets.

> 
>  ofproto/ofproto-dpif-xlate.c | 18 --
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 04d69ed06c20..1ace92d22019 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017 
> Nicira, Inc.
> +/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2019 
> Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -1864,8 +1864,8 @@ group_is_alive(const struct xlate_ctx *ctx, uint32_t 
> group_id, int depth)
>  #define MAX_LIVENESS_RECURSION 128 /* Arbitrary limit */
>  
>  static bool
> -bucket_is_alive(const struct xlate_ctx *ctx,
> -struct ofputil_bucket *bucket, int depth)
> +bucket_is_alive(const struct xlate_ctx *ctx, const struct group_dpif *group,
> +const struct ofputil_bucket *bucket, int depth)
>  {
>  if (depth >= MAX_LIVENESS_RECURSION) {
>  xlate_report_error(ctx, "bucket chaining exceeded %d links",
> @@ -1873,6 +1873,12 @@ bucket_is_alive(const struct xlate_ctx *ctx,
>  return false;
>  }
>  
> +/* In "select" groups, buckets with weight 0 are not used.
> + * In other kinds of groups, weight does not matter. */
> +if (group->up.type == OFPGT11_SELECT && bucket->weight == 0) {
> +return false;
> +}
> +
>  return (!ofputil_bucket_has_liveness(bucket)
>  || (bucket->watch_port != OFPP_ANY
> && odp_port_is_alive(ctx, bucket->watch_port))
> @@ -1910,7 +1916,7 @@ group_first_live_bucket(const struct xlate_ctx *ctx,
>  {
>  struct ofputil_bucket *bucket;
>  LIST_FOR_EACH (bucket, list_node, >up.buckets) {
> -if (bucket_is_alive(ctx, bucket, depth)) {
> +if (bucket_is_alive(ctx, group, bucket, depth)) {
>  return bucket;
>  }
>  xlate_report_bucket_not_live(ctx, bucket);
> @@ -1929,7 +1935,7 @@ group_best_live_bucket(const struct xlate_ctx *ctx,
>  
>  struct ofputil_bucket *bucket;
>  LIST_FOR_EACH (bucket, list_node, >up.buckets) {
> -if (bucket_is_alive(ctx, bucket, 0)) {
> +if (bucket_is_alive(ctx, group, bucket, 0)) {
>  uint32_t score =
>  (hash_int(bucket->bucket_id, basis) & 0x) * 
> bucket->weight;
>  if (score >= best_score) {
> @@ -4535,7 +4541,7 @@ pick_dp_hash_select_group(struct xlate_ctx *ctx, struct 
> group_dpif *group)
>  for (int i = 0; i <= hash_mask; i++) {
>  struct ofputil_bucket *b =
>  group->hash_map[(dp_hash + i) & hash_mask];
> -if (bucket_is_alive(ctx, b, 0)) {
> +if (bucket_is_alive(ctx, group, b, 0)) {
>  return b;
>  }
>  }
> 

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

[ovs-dev] FROM MRS SUSAN READ.

2020-09-09 Thread Mrs Susan Read via dev 


Greetings,

My name is Mrs. Susan Read, from United Kindom, I have been suffering from 
ovarian cancer disease and the doctor says that I have just a short time to 
live. And for the past Twelve years, I have being dealing on gold exportation, 
before this disease crippled me down. My late husband, Dr. Benoit Kabore, a 
retired diplomat and one time minister of mines and Power in the republic of 
Burkina Faso. Made a lot of money from the sales of Gold and cotton while he 
was a minister, but we had no child of our own.

later my husband realized through a powerful man of God that it was evil course 
instituted by his brother in other to inherit his wealth, but before then it 
was too late, I and my husband agreed that he should remarry another wife but 
our religion did not permit it, while planning this my husband’s brother heard 
it and they planned and killed my husband at the tender age of 56, he died in 
the month of September 2007. Now that I am very sick and according to the 
doctor, will not survive the sickness. The worst of it all is that I do not 
have any family members or children to inherit my wealth. I am writing this 
letter now through the help of the computer beside my sick is bed.

I have $6.7 Million US Dollars deposited here in Burkina Faso and I am willing 
to instruct my bank to transfer the said fund to you as my foreign Trustee. You 
will apply to the bank that there should release the fund to you, but you will 
assure me that you will take 40% of the fund and give 60% to the orphanages 
home in your country for my soul to rest after I have gone. Respond to me 
immediately for further details and instructions since I am in the end times of 
my life due to the ovarian cancer disease.

Hoping to receive your response as soon as possible.

My Regards,
Mrs Susan Read.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] ofproto: Preserve ofport number for failed ofport across restarts

2020-09-09 Thread Ilya Maximets 
>> 
>> I've been taking a look at this patch for the last few minutes.  It 
>> introduces a lot
>> of mechanism for the use case.  Did you consider any simpler mechanisms to
>> achieve the same effect?  What prevented them from working?
>> 
> I agree Ben. This change does bring some complexity to the code. A simpler 
> solution would
> be to leave the ofport number in ovsdb record of the interface (instead of 
> setting it to -1).
> But issue was when the port delete happens we do not see this record to be 
> able to free the
> ofport number. Is there a way to handle this?
> 
> Warm Regards,
> Vishal Ajmera

Hi, Vishal.  I'm looking through old patches after the patchwork cleanup and
it seems that this one in kind of undecided state.
Does this patch still valid?

Anyway, for the problem you described here:  Maybe it's possible to just use
'ofport_request' configuration option for interfaces?  It could be implemented
as picking a unique ofport before adding a new port and set 'ofport_request'
along with port creation.  Another way is to fix current ofport for interfaces
once OVS chose it, i.e. port add -> check ofport -> set 'ofport_request' with
that value.
In any case after reboot OVS will choose exactly same ofport as it was before
because it is stored in database.

What do you think?

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

Re: [ovs-dev] [PATCH v1] rhel: retain OVS_CTL_OPTS for systemd service files

2020-09-09 Thread Ilya Maximets 
On 9/9/20 5:39 PM, Gregory Rose wrote:
> 
> 
> On 9/9/2020 5:51 AM, Ilya Maximets wrote:
>>> On 1/31/2019 10:38 AM, Aaron Conole wrote:
 Martin Xu  writes:

> OVS init.d script calls ovs-ctl with $OVS_CTL_OPTS defined in the
> config file. This variable is replaced by OPTIONS in systemd service
> files. This patch addes $OVS_CTL_OPTS back to be passed along with 
> $OPTIONS
> for backward compatibility.
>
> VMware-BZ: #2036847
>
> Signed-off-by: Martin Xu 
> CC: Aaron Conole 
> ---
 I'm not sure why there should be two variables in the sysconfig file for
 this.  The following would preserve the old and not introduce a new
 variable (I think.. it's completely untested).  I guess this is because
 the debian-distro openvswitch-switch.template file doesn't match the
 rhel-distro template file, and we want to make a common set of systemd
 scripts? Otherwise I don't see what the purpose is - what is the
 migration path that this is addressing?
>>>
>>> Aaron,
>>>
>>> I owe you a response on this and will get to it but some fires need
>>> putting out at the moment.
>>
>> Hi, Greg, Martin.
>>
>> I'm looking through old patches after the patchwork cleanup and this
>> one seems to be never applied.  I'm assuming that it's not needed
>> anymore, however, I'd like to have some comment on it if possible.
>>
>> For now marking it as 'Not Applicable'.  Please, resubmit in case it's
>> still needed.
> 
> It appears to be no longer required so this is the right thing to do.

Great!  Thanks for the information.

> 
> Thanks,
> 
> - Greg
> 
> 
>>
>> Best regards, Ilya Maximets.
>>
>>>
>>> Thanks,
>>>
>>> - Greg
>>>

 ---
 diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
 b/rhel/usr_lib_systemd_system_ovsdb-server.service
 index 09f946bb1..660ec75ef 100644
 --- a/rhel/usr_lib_systemd_system_ovsdb-server.service
 +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
 @@ -12,6 +12,7 @@ EnvironmentFile=/etc/openvswitch/default.conf
    EnvironmentFile=-/etc/sysconfig/openvswitch
    ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch 
 /var/log/openvswitch
    ExecStartPre=/bin/sh -c 'rm -f /run/openvswitch/useropts; if [ 
 "$${OVS_USER_ID/:*/}" != "root" ]; then /usr/bin/echo 
 "OVSUSER=--ovs-user=${OVS_USER_ID}" > /run/openvswitch/useropts; fi'
 +ExecStartPre=/bin/sh -c 'if [ "${OVS_CTL_OPTS}" != "" -a "${OPTIONS}" == 
 "" ]; then /usr/bin/echo "OPTIONS=\"${OPTIONS} ${OVS_CTL_OPTS}\"" >> 
 /run/openvswitch/useropts; fi'
    EnvironmentFile=-/run/openvswitch/useropts
    ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
  --no-ovs-vswitchd --no-monitor --system-id=random \
 ---

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

Re: [ovs-dev] [PATCH] rhel: fix logrotate group when dpdk is enabled

2020-09-09 Thread Ilya Maximets 
On 4/30/19 7:10 PM, Jaime Caamaño Ruiz wrote:
> Otherwise logrotate will fail to generate the rotated log files.
> 
> Signed-off-by: Jaime Caamaño Ruiz 
> ---

Hi.  I'm looking through old patches after the patchwork cleanup and this
one seems to be still valid and applicable.

I could apply it if it still needed.  Jaime, what do you think?

Flavio, Aaron, could you, please, take a look at this patch?

Best regards, Ilya Maximets.

>  rhel/openvswitch-fedora.spec.in | 19 ++-
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
> index ce728b4f0..01401d5f1 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -366,18 +366,19 @@ exit 0
>  %post
>  %if %{with libcapng}
>  if [ $1 -eq 1 ]; then
> -sed -i 's:^#OVS_USER_ID=:OVS_USER_ID=:' /etc/sysconfig/openvswitch
> -sed -i 's:\(.*su\).*:\1 openvswitch openvswitch:' 
> %{_sysconfdir}/logrotate.d/openvswitch
> -
>  %if %{with dpdk}
> -sed -i \
> -
> 's@OVS_USER_ID="openvswitch:openvswitch"@OVS_USER_ID="openvswitch:hugetlbfs"@'\
> -/etc/sysconfig/openvswitch
> +%define gname hugetlbfs
> +%else
> +%define gname openvswitch
>  %endif
> +sed -i \
> +
> 's@^#OVS_USER_ID="openvswitch:openvswitch"@OVS_USER_ID="openvswitch:%{gname}"@'\
> +%{_sysconfdir}/sysconfig/openvswitch
> +sed -i 's:\(.*su\).*:\1 openvswitch %{gname}:' 
> %{_sysconfdir}/logrotate.d/openvswitch
>  
> -# In the case of upgrade, this is not needed.
> -chown -R openvswitch:openvswitch /etc/openvswitch
> -chown -R openvswitch:openvswitch /var/log/openvswitch
> +# In the case of upgrade, this is not needed
> +chown -R openvswitch:openvswitch %{_sysconfdir}/openvswitch
> +chown -R openvswitch:%{gname} %{_localstatedir}/log/openvswitch
>  fi
>  %endif
>  
> 

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

Re: [ovs-dev] [PATCH] vxlan: Make vxlan tunnel work in IP_TUNNEL_INFO_BRIDGE mode

2020-09-09 Thread Ilya Maximets 
On 3/30/19 6:28 AM, we...@ucloud.cn wrote:
> From: wenxu 
> 
> There is currently no support for the multicast/broadcast aspects
> of VXLAN in ovs. In the datapath flow the tun_dst must specific.
> But in the IP_TUNNEL_INFO_BRIDGE mode the tun_dst can not be specific.
> And the packet can forward through the fdb table of vxlan devcice. In
> this mode the broadcast/multicast packet can be sent through the
> following ways in ovs.
> 
> It adds an options allow_info_bridge for vxlan tunnel interface to
> permit to enable this mode for this vxlan tunnel.
> 
> ovs-vsctl add-port br0 vxlan -- set in vxlan type=vxlan \
> options:key=1000 options:remote_ip=flow
> ovs-vsctl set in vxlan options:allow_info_bridge=true
> ovs-ofctl add-flow br0 in_port=LOCAL,dl_dst=ff:ff:ff:ff:ff:ff, \
> action=output:vxlan
> 
> bridge fdb append ff:ff:ff:ff:ff:ff dev vxlan_sys_4789 dst 172.168.0.1 \
> src_vni 1000 vni 1000 self
> bridge fdb append ff:ff:ff:ff:ff:ff dev vxlan_sys_4789 dst 172.168.0.2 \
> src_vni 1000 vni 1000 self
> 
> Signed-off-by: wenxu 
> ---

Hi.  I'm looking through old patches after the patchwork cleanup and it
seems like this patch was never actually accepted or even reviewed.
However, the kernel part of this feature was accepted to kernel like
1.5 years ago.  It'll be good to have this feature support in userspace
code so it could be actually used.


I have a few comments for this implementation:

1. Comment in openvswitch.h differs from one that accepted to kernel, so
   it should be updated.

2. This implementation doens't implement support for the userspace datpath
   and this might produce issues if users will enable new configuration
   in this case.  We need an implementation for the userspace datpath, i.e.
   in lib/netdev-native-tnl.c, or firbid setting this new configuration
   for the case where netdev belongs to userspace datapath.  This also
   will need to be documanted.

3. Since this is a user-visible change, we need a NEWS entry for it.

4. It'll be good to have a system test for this feature to be sure that
   we will not break it in the future, especially if userspace/dummy
   datapaths doesn't support it.  See tests/system-traffic.at.

Anyway, this patch is very old and can not be applied cleanly, it'll be
great if  you could rebase it, address above issues and send v2.

What do you think?

Best regards, Ilya Maximets.

>  datapath/linux/compat/include/linux/openvswitch.h |  1 +
>  include/openvswitch/packets.h |  3 ++-
>  lib/netdev-vport.c| 20 +++-
>  lib/netdev.h  |  2 ++
>  lib/odp-util.c| 19 ---
>  lib/packets.h |  6 ++
>  ofproto/ofproto-dpif-xlate.c  |  3 ++-
>  ofproto/tunnel.c  |  5 +
>  vswitchd/vswitch.xml  |  9 +
>  9 files changed, 62 insertions(+), 6 deletions(-)
> 
> diff --git a/datapath/linux/compat/include/linux/openvswitch.h 
> b/datapath/linux/compat/include/linux/openvswitch.h
> index d5aa09d..3b23590 100644
> --- a/datapath/linux/compat/include/linux/openvswitch.h
> +++ b/datapath/linux/compat/include/linux/openvswitch.h
> @@ -400,6 +400,7 @@ enum ovs_tunnel_key_attr {
>   OVS_TUNNEL_KEY_ATTR_IPV6_DST,   /* struct in6_addr dst IPv6 
> address. */
>   OVS_TUNNEL_KEY_ATTR_PAD,
>   OVS_TUNNEL_KEY_ATTR_ERSPAN_OPTS,/* struct erspan_metadata */
> + OVS_TUNNEL_KEY_ATTR_IPV4_INFO_BRIDGE,   /* No argument.ipv4 info bridge 
> mode */
>   __OVS_TUNNEL_KEY_ATTR_MAX
>  };
>  
> diff --git a/include/openvswitch/packets.h b/include/openvswitch/packets.h
> index 925844e..bad5618 100644
> --- a/include/openvswitch/packets.h
> +++ b/include/openvswitch/packets.h
> @@ -43,7 +43,8 @@ struct flow_tnl {
>  uint32_t erspan_idx;
>  uint8_t erspan_dir;
>  uint8_t erspan_hwid;
> -uint8_t pad1[6]; /* Pad to 64 bits. */
> +uint8_t ipv4_info_bridge;
> +uint8_t pad1[5]; /* Pad to 64 bits. */
>  struct tun_metadata metadata;
>  };
>  
> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
> index 808a43f..43ca503 100644
> --- a/lib/netdev-vport.c
> +++ b/lib/netdev-vport.c
> @@ -553,6 +553,7 @@ set_tunnel_config(struct netdev *dev_, const struct smap 
> *args, char **errp)
>  bool needs_dst_port, has_csum, has_seq;
>  uint16_t dst_proto = 0, src_proto = 0;
>  struct netdev_tunnel_config tnl_cfg;
> +bool allow_info_bridge = false;
>  struct smap_node *node;
>  int err;
>  
> @@ -725,12 +726,25 @@ set_tunnel_config(struct netdev *dev_, const struct 
> smap *args, char **errp)
>  goto out;
>  }
>  }
> +} else if (!strcmp(node->key, "allow_info_bridge")) {
> +if (!strcmp(node->value, "true")) {
> +

[ovs-dev] [OVN] failed to connect 5K networks to 5K routers

2020-09-09 Thread Tony Liu 
Hi,

Here is what I did.

#1 Create 5K networks, 5K routers and 1 provider/physical network.
All resources are created in NB and translated to SB properly.

#2 Connect 5K routers to provider network.
All resources are created and updated in NB properly, but SB
is not fully updated.

I did 3 snapshots of NB, snapshot-init that has no resource,
snapshot-1 and snapshot-2 after each of above two steps.

I restored snapshot-init to NB, SB is not restored because northd
didn't work properly, so I manually clean up SB.

I restored snapshot-1 to NB, SB is fully updated fairly quick.

I restored snapshot-2 to NB, SB got partial update only.

I am still with 20.03...

When restore snapshot-1, does northd read all resources from NB
and translate them to SB as one-shot or there are multiple
transitions?

When restore snapshot-2, does northd take lots more time to
translate than snapshot-1?

When restore snapshot-2, controller on gateway chassis is also
involved to update local OVSDB and chassis port status in SB.
Could that be affecting the translation from NB to SB?

Is upgrading to 20.06 going to help on this?


Thanks!
Tony

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

Re: [ovs-dev] [PATCH v1] rhel: retain OVS_CTL_OPTS for systemd service files

2020-09-09 Thread Gregory Rose 




On 9/9/2020 5:51 AM, Ilya Maximets wrote:

On 1/31/2019 10:38 AM, Aaron Conole wrote:

Martin Xu  writes:


OVS init.d script calls ovs-ctl with $OVS_CTL_OPTS defined in the
config file. This variable is replaced by OPTIONS in systemd service
files. This patch addes $OVS_CTL_OPTS back to be passed along with $OPTIONS
for backward compatibility.

VMware-BZ: #2036847

Signed-off-by: Martin Xu 
CC: Aaron Conole 
---

I'm not sure why there should be two variables in the sysconfig file for
this.  The following would preserve the old and not introduce a new
variable (I think.. it's completely untested).  I guess this is because
the debian-distro openvswitch-switch.template file doesn't match the
rhel-distro template file, and we want to make a common set of systemd
scripts? Otherwise I don't see what the purpose is - what is the
migration path that this is addressing?


Aaron,

I owe you a response on this and will get to it but some fires need
putting out at the moment.


Hi, Greg, Martin.

I'm looking through old patches after the patchwork cleanup and this
one seems to be never applied.  I'm assuming that it's not needed
anymore, however, I'd like to have some comment on it if possible.

For now marking it as 'Not Applicable'.  Please, resubmit in case it's
still needed.


It appears to be no longer required so this is the right thing to do.

Thanks,

- Greg




Best regards, Ilya Maximets.



Thanks,

- Greg



---
diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
b/rhel/usr_lib_systemd_system_ovsdb-server.service
index 09f946bb1..660ec75ef 100644
--- a/rhel/usr_lib_systemd_system_ovsdb-server.service
+++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
@@ -12,6 +12,7 @@ EnvironmentFile=/etc/openvswitch/default.conf
   EnvironmentFile=-/etc/sysconfig/openvswitch
   ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch 
/var/log/openvswitch
   ExecStartPre=/bin/sh -c 'rm -f /run/openvswitch/useropts; if [ "$${OVS_USER_ID/:*/}" != 
"root" ]; then /usr/bin/echo "OVSUSER=--ovs-user=${OVS_USER_ID}" > 
/run/openvswitch/useropts; fi'
+ExecStartPre=/bin/sh -c 'if [ "${OVS_CTL_OPTS}" != "" -a "${OPTIONS}" == "" ]; then /usr/bin/echo 
"OPTIONS=\"${OPTIONS} ${OVS_CTL_OPTS}\"" >> /run/openvswitch/useropts; fi'
   EnvironmentFile=-/run/openvswitch/useropts
   ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
 --no-ovs-vswitchd --no-monitor --system-id=random \
---

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

[ovs-dev] FOESCO Informa

2020-09-09 Thread FOESCO 
Buenos días


Soy Alex Pons, director de FOESCO (Formación Estatal Continua).

Llegadas estas fechas y como cada año, recordamos a todas las empresas 
Españolas su derecho a consumir el Crédito de Formación del que disponen para 
la formación de sus empleados, antes de su caducidad a final de año.


Actualmente se encuentra abierto el plazo de inscripción para la Convocatoria 
SEPTIEMBRE 2020 de Cursos Bonificables con cargo al Crédito de Formación 2020.


Deseáis que os mandemos la información?


Quedamos a la espera de vuestra respuesta.


Saludos cordiales.


Alex Pons
Director FOESCO.

FOESCO Formación Estatal Continua.
Entidad Organizadora: B200592AA
www.foesco.com
e-mail: cur...@foesco.net
Tel: 910 323 794
(Horario de 9h a 15h y de 17h a 20h de Lunes a Viernes)

FOESCO ofrece formación a empresas y trabajadores en activo a través de cursos 
bonificados por la Fundación Estatal para la Formación en el Empleo (antiguo 
FORCEM) que gestiona las acciones formativas de FORMACIÓN CONTINUA para 
trabajadores y se rige por la ley 30/2015 de 9 de Septiembre.

Antes de imprimir este e-mail piense bien si es necesario hacerlo. Before 
printing this e-mail please think twice if you really need it. FOESCO Tfno: 910 
382 880 Email: cur...@foesco.com. La información transmitida en este mensaje 
está dirigida solamente a las personas o entidades que figuran en el 
encabezamiento y contiene información confidencial, por lo que, si usted lo 
recibiera por error, por favor destrúyalo sin copiarlo, usarlo ni distribuirlo, 
comunicándolo inmediatamente al emisor del mensaje. De conformidad con lo 
dispuesto en el Reglamento Europeo del 2016/679, del 27 de Abril de 2016, 
FOESCO le informa que los datos por usted suministrados serán tratados con las 
medidas de seguridad conformes a la normativa vigente que se requiere. Dichos 
datos serán empleados con fines de gestión. Para el ejercicio de sus derechos 
de transparencia, información, acceso, rectificación, supresión o derecho al 
olvido, limitación del tratamiento , portabilidad de datos y oposición de sus 
datos de carácter personal deberá dirigirse a la dirección del Responsable del 
tratamiento a C/ LAGUNA DEL MARQUESADO Nº10, 28021, MADRID, "PULSANDO AQUI" 
 y "ENVIAR" o a traves de la 
dirección de correo electrónico: ba...@foesco.com 

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

Re: [ovs-dev] [PATCH v7] dpif-netlink: distribute polling to discreet handlers

2020-09-09 Thread Mark Gray 
On 09/09/2020 14:49, Flavio Leitner wrote:
> 

>>  error:
>> -vport_del_channels(dpif, vport.port_no);
>> +vport_del_channel(dpif, port_no);
> 
> The port_no here is uint32_t but vport_del_channel() receives odp_port_t.
> That breaks build:
> https://travis-ci.org/github/fleitner/ovs/jobs/725370855
> 

I think I'll rename this variable to port_idx. The convention in the
file is port_no is usually of type odp_port_t. This contributed to me
missing it.

> Thanks!
> 

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

Re: [ovs-dev] [PATCH v7] dpif-netlink: distribute polling to discreet handlers

2020-09-09 Thread Flavio Leitner 



Hi Mark,

Found one issue, and then since another spin is needed I am
suggesting a stylish improvement.


On Tue, Sep 08, 2020 at 06:27:48PM +0100, Mark Gray wrote:
> From: Aaron Conole 
> 
> Currently, the channel handlers are polled globally.  On some
> systems, this causes a thundering herd issue where multiple
> handler threads become active, only to do no work and immediately
> sleep.
> 
> The approach here is to push the netlink socket channels to discreet
> handler threads to process, rather than polling on every thread.
> This will eliminate the need to wake multiple threads.
> 
> To check:
> 
>   ip netns add left
>   ip netns add right
>   ip link add center-left type veth peer name left0 netns left
>   ip link add center-right type veth peer name right0 netns right
>   ip link set center-left up
>   ip link set center-right up
>   ip -n left ip link set left0 up
>   ip -n left ip addr add 172.31.110.10/24 dev left0
>   ip -n right ip link set right0 up
>   ip -n right ip addr add 172.31.110.11/24 dev right0
> 
>   ovs-vsctl add-br br0
>   ovs-vsctl add-port br0 center-right
>   ovs-vsctl add-port br0 center-left
> 
>   # in one terminal
>   perf record -e sched:sched_wakeup,irq:softirq_entry -ag
> 
>   # in a separate terminal
>   ip netns exec left arping -I left0 -c 1 172.31.110.11
> 
>   # in the perf terminal after exiting
>   perf script
> 
> Look for the number of 'handler' threads which were made active.
> 
> Suggested-by: Ben Pfaff 
> Co-authored-by: Mark Gray 
> Reported-by: David Ahern 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2019-December/365857.html
> Cc: Matteo Croce 
> Cc: Flavio Leitner 
> Fixes: 69c51582f ("dpif-netlink: don't allocate per thread netlink sockets")
> Signed-off-by: Aaron Conole 
> Signed-off-by: Mark Gray 
> ---
> 
> 
> v2: Oops - forgot to commit my whitespace cleanups.
> v3: one port latency results as per Matteo's comments
> 
> Stock:
>   min/avg/max/mdev = 21.5/36.5/96.5/1.0 us
> With Patch:
>   min/avg/max/mdev = 5.3/9.7/98.4/0.5 us 
> v4: Oops - first email did not send
> v5:   min/avg/max/mdev = 9.3/10.4/33.6/2.2 us
> v6: Split out the AUTHORS patch and added a cover letter as
> per Ilya's suggestion.
> Fixed 0-day issues.
> v7: Merged patches as per Flavio's suggestion. This is
> no longer a series. Fixed some other small issues.
> 
>  lib/dpif-netlink.c | 325 +
>  lib/id-pool.c  |  10 ++
>  lib/id-pool.h  |   1 +
>  3 files changed, 195 insertions(+), 141 deletions(-)
> 
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 7da4fb54d..4ecf0c51c 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -37,6 +37,7 @@
>  #include "dpif-provider.h"
>  #include "fat-rwlock.h"
>  #include "flow.h"
> +#include "id-pool.h"
>  #include "netdev-linux.h"
>  #include "netdev-offload.h"
>  #include "netdev-provider.h"
> @@ -160,6 +161,9 @@ static void dpif_netlink_flow_to_dpif_flow(struct 
> dpif_flow *,
>  
>  /* One of the dpif channels between the kernel and userspace. */
>  struct dpif_channel {
> +struct hmap_node hmap_node;
> +struct dpif_handler *handler;
> +uint32_t port_idx;
>  struct nl_sock *sock;   /* Netlink socket. */
>  long long int last_poll;/* Last time this channel was polled. */
>  };
> @@ -183,6 +187,8 @@ struct dpif_handler {
>  int n_events; /* Num events returned by epoll_wait(). */
>  int event_offset; /* Offset into 'epoll_events'. */
>  
> +struct hmap channels; /* map of channels for each port. */
> +
>  #ifdef _WIN32
>  /* Pool of sockets. */
>  struct dpif_windows_vport_sock *vport_sock_pool;
> @@ -201,9 +207,8 @@ struct dpif_netlink {
>  struct fat_rwlock upcall_lock;
>  struct dpif_handler *handlers;
>  uint32_t n_handlers;   /* Num of upcall handlers. */
> -struct dpif_channel *channels; /* Array of channels for each port. */
> -int uc_array_size; /* Size of 'handler->channels' and */
> -   /* 'handler->epoll_events'. */
> +
> +struct id_pool *port_ids;  /* Set of added port ids */
>  
>  /* Change notification. */
>  struct nl_sock *port_notifier; /* vport multicast group subscriber. */
> @@ -287,6 +292,44 @@ close_nl_sock(struct nl_sock *sock)
>  #endif
>  }
>  
> +static size_t
> +dpif_handler_channels_count(const struct dpif_handler *handler)
> +{
> +return hmap_count(>channels);
> +}
> +
> +static struct dpif_channel *
> +dpif_handler_channel_find(struct dpif_handler *handler, uint32_t idx)
> +{
> +struct dpif_channel *channel;
> +
> +HMAP_FOR_EACH_WITH_HASH (channel, hmap_node, hash_int(idx, 0),
> + >channels) {
> +if (channel->port_idx == idx) {
> +return channel;
> +}
> +}
> +
> +return NULL;
> +}
> +
> +static void
> +dpif_channel_del(struct dpif_netlink

[ovs-dev] [PATCH] conntrack: add generic IP protocol support

2020-09-09 Thread Eelco Chaudron 
Currently, userspace conntrack only tracks TCP, UDP, and ICMP, and all
other IP protocols are discarded, and the +inv state is returned. This
is not in line with the kernel conntrack. Where if no L4 information can
be extracted it's treated as generic L3. The change below mimics the
behavior of the kernel.

Signed-off-by: Eelco Chaudron 
---
 lib/conntrack-private.h |3 +++
 lib/conntrack.c |   27 ++-
 tests/system-traffic.at |   29 +
 3 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index 9a8ca39..85329e8 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -59,6 +59,9 @@ struct conn_key {
 uint8_t nw_proto;
 };
 
+/* Verify that nw_proto stays uint8_t as it's used to index into l4_protos[] */
+BUILD_ASSERT_DECL(sizeof(((struct conn_key *)0)->nw_proto) == sizeof(uint8_t));
+
 /* This is used for alg expectations; an expectation is a
  * context created in preparation for establishing a data
  * connection. The expectation is created by the control
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 0cbc8f6..cb42f26 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -143,12 +143,7 @@ detect_ftp_ctl_type(const struct conn_lookup_ctx *ctx,
 static void
 expectation_clean(struct conntrack *ct, const struct conn_key *master_key);
 
-static struct ct_l4_proto *l4_protos[] = {
-[IPPROTO_TCP] = _proto_tcp,
-[IPPROTO_UDP] = _proto_other,
-[IPPROTO_ICMP] = _proto_icmp4,
-[IPPROTO_ICMPV6] = _proto_icmp6,
-};
+static struct ct_l4_proto *l4_protos[UINT8_MAX + 1];
 
 static void
 handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
@@ -296,6 +291,7 @@ ct_print_conn_info(const struct conn *c, const char 
*log_msg,
 struct conntrack *
 conntrack_init(void)
 {
+static struct ovsthread_once setup_l4_once = OVSTHREAD_ONCE_INITIALIZER;
 struct conntrack *ct = xzalloc(sizeof *ct);
 
 ovs_rwlock_init(>resources_lock);
@@ -322,6 +318,18 @@ conntrack_init(void)
 ct->clean_thread = ovs_thread_create("ct_clean", clean_thread_main, ct);
 ct->ipf = ipf_init();
 
+/* Initialize the l4 protocols. */
+if (ovsthread_once_start(_l4_once)) {
+for (int i = 0; i < ARRAY_SIZE(l4_protos); i++) {
+l4_protos[i] = _proto_other;
+}
+/* IPPROTO_UDP uses ct_proto_other, so no need to initialize it. */
+l4_protos[IPPROTO_TCP] = _proto_tcp;
+l4_protos[IPPROTO_ICMP] = _proto_icmp4;
+l4_protos[IPPROTO_ICMPV6] = _proto_icmp6;
+
+ovsthread_once_done(_l4_once);
+}
 return ct;
 }
 
@@ -1971,7 +1979,8 @@ extract_l4(struct conn_key *key, const void *data, size_t 
size, bool *related,
 validate_checksum))
&& extract_l4_icmp6(key, data, size, related);
 } else {
-return false;
+/* For all other protocols we do not have L4 keys, so keep them zero */
+return true;
 }
 }
 
@@ -2255,8 +2264,8 @@ nat_select_range_tuple(struct conntrack *ct, const struct 
conn *conn,
   conn->nat_info->nat_action & NAT_ACTION_SRC_PORT
   ? true : false;
 union ct_addr first_addr = ct_addr;
-bool pat_enabled = conn->key.nw_proto != IPPROTO_ICMP &&
-   conn->key.nw_proto != IPPROTO_ICMPV6;
+bool pat_enabled = conn->key.nw_proto == IPPROTO_TCP ||
+   conn->key.nw_proto == IPPROTO_UDP;
 
 while (true) {
 if (conn->nat_info->nat_action & NAT_ACTION_SRC) {
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 3ed03d9..b7aca93 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -2341,6 +2341,35 @@ NXST_FLOW reply:
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([conntrack - generic IP protocol])
+CHECK_CONNTRACK()
+OVS_TRAFFIC_VSWITCHD_START()
+AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg 
ofproto_dpif_upcall:dbg])
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
+
+AT_DATA([flows.txt], [dnl
+table=0, priority=1,action=drop
+table=0, priority=10,arp,action=normal
+table=0, priority=100,ip,action=ct(table=1)
+table=1, priority=100,in_port=1,ip,ct_state=+trk+new,action=ct(commit)
+table=1, priority=100,in_port=1,ct_state=+trk+est,action=normal
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 
packet=01005e125e000101080045c00028ff7019cdc0a8001ee012210164010001ba52c0a80001
 actions=resubmit(,0)"])
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep 
"orig=.src=192\.168\.0\.30,"], [], [dnl
+112,orig=(src=192.168.0.30,dst=224.0.0.18,sport=0,dport=0),reply=(src=224.0.0.18,dst=192.168.0.30,sport=0,dport=0)
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([conntrack - ICMP related])
 AT_SKIP_IF([test $HAVE_NC = no])

Re: [ovs-dev] [PATCH ovn v2 0/9] Incremental processing for flow installation.

2020-09-09 Thread Mark Michelson 

Thanks for addressing my concerns on patch 9.

Acked-by: Mark Michelson 

Warning: I'm also ACKing Numan's patch series that also adds some 
lflow-level caching (but he is caching expressions). So whoever loses 
this merge race may have some work to do in order to apply their patch 
cleanly.


On 9/7/20 2:45 AM, Han Zhou wrote:

Incremental processing has been implementation in ovn-controller, but we were
still doing full comparison between desired flow table and installed flow table
every time to figure out the changes need to be pushed to OVS. This series is
mainly to utilize the incremental processing information to figure out flow
changes to OVS without full table scanning, to further reduce CPU of
ovn-controller.

In ovn-scale-test with 3000 HVs and 30k lports, the end-to-end latency between
the moment a lflow is updated in SB DB and the moment when all the 3K HVs
complete OVS flow updating has reduced around 60% (from 1s to 400ms). perf
report also shows ~40% of CPU reduced in ovn-controller.

Another important change of this series is the fix of the conjunction handling
problem.

v1 -> v2:

- Addressed Mark's comments for defining different data types for desired flows
   and installed flows, and related refactoring. (patch 6/9)
- Updated comments for the cross reference structures between desired flows and
   SB UUIDs with a diagram to help understanding. (patch 4/9)

Han Zhou (9):
   ofctrl: change ofctrl_dup_flow to module internal function
   ovn.at: Fix AT for conjunction case.
   lflow.c: No need to remove flows for adding new datapath.
   ovn-controller: Fix conjunction handling with incremental processing.
   ovn.at: Add test case for duplicated flow handling.
   ofctrl.c: Maintain references between installed flows and desired flows.
   ofctrl.c: Refactor - move openflow msg construction to functions.
   ofctrl: Incremental processing for flow installation by tracking.
   ofctrl.c: Merge opposite changes of tracked flows before installing.

  controller/lflow.c  |  106 --
  controller/ofctrl.c | 1041 +--
  controller/ofctrl.h |   39 +-
  tests/ovn.at|  225 ++-
  4 files changed, 1164 insertions(+), 247 deletions(-)



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

Re: [ovs-dev] [PATCH ovn v2 3/4] expr: Evaluate the condition expression in a separate step.

2020-09-09 Thread Mark Michelson 

For the whole series:

Acked-by: Mark Michelson 

However, there are a few nits for this particular patch that I think 
should be addressed before committing:


1) The commit message says expr_simply() instead of expr_simplify()
2) The comment above expr_normalize() in expr.c needs to be updated to 
mention that 'expr' must have already been simplified and had conditions 
evaluated.
3) In expr_normalize(), the comment above the EXPR_T_CONDITION case 
needs to be altered to say that expr_evaluate_condition resolves 
conditions instead of expr_simplify.


These are all simple enough that you can just update these and then merge.

Warning: I'm also ACK-ing Han's patch series that adds some lflow-level 
caching (but his patch series caches ovn_flows). So there is a chance 
that whoever loses this merge race will have some extra work to get 
their patch to apply cleanly.


On 9/9/20 6:09 AM, num...@ovn.org wrote:

From: Numan Siddique 

A similar patch was committed earlier [1] and then reverted [2].
This patch is similar to [1], but not exactly same.

A new function is added - expr_evaluate_condition() which evaluates
the condition expression - is_chassis_resident. expr_simply() will
no longer evaluates this condition. expr_normalize() is still expected
to be called after expr_evaluate_condition(). Otherwise it will assert
if 'is_chassis_resident()' is not evaluated.

An upcoming commit needs this in order to cache the logical flow expressions
for logical flows having 'is_chassis_resident()' condition in their match.
The expr tree after expr_simplify()  for such logical flows is cached. Since
we can't cache the is_chassis_resident condition, it is separated out.
(For logical flows which do not have this condition in their match, the expr 
matches
will be cached.)

[1] - 99e3a145927("expr: Evaluate the condition expression in a separate step.")
[2] - 065bcf46218("ovn-controller: Revert lflow expr caching")

Signed-off-by: Numan Siddique 
---
  controller/lflow.c|  3 ++-
  include/ovn/expr.h| 10 +
  lib/expr.c| 50 +--
  tests/test-ovn.c  | 11 +-
  utilities/ovn-trace.c |  6 --
  5 files changed, 57 insertions(+), 23 deletions(-)

diff --git a/controller/lflow.c b/controller/lflow.c
index c2f939f90f..c6e1586283 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -684,7 +684,8 @@ consider_logical_flow(const struct sbrec_logical_flow 
*lflow,
  .lflow = lflow,
  .lfrr = l_ctx_out->lfrr
  };
-expr = expr_simplify(expr, is_chassis_resident_cb, _aux);
+expr = expr_simplify(expr);
+expr = expr_evaluate_condition(expr, is_chassis_resident_cb, _aux);
  expr = expr_normalize(expr);
  uint32_t n_conjs = expr_to_matches(expr, lookup_port_cb, ,
 );
diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index b34fb0e813..ed7b054144 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -432,10 +432,12 @@ void expr_destroy(struct expr *);
  
  struct expr *expr_annotate(struct expr *, const struct shash *symtab,

 char **errorp);
-struct expr *expr_simplify(struct expr *,
-   bool (*is_chassis_resident)(const void *c_aux,
-   const char *port_name),
-   const void *c_aux);
+struct expr *expr_simplify(struct expr *);
+struct expr *expr_evaluate_condition(
+struct expr *,
+bool (*is_chassis_resident)(const void *c_aux,
+const char *port_name),
+const void *c_aux);
  struct expr *expr_normalize(struct expr *);
  
  bool expr_honors_invariants(const struct expr *);

diff --git a/lib/expr.c b/lib/expr.c
index 6fb96757ad..bff48dfd20 100644
--- a/lib/expr.c
+++ b/lib/expr.c
@@ -2063,10 +2063,10 @@ expr_simplify_relational(struct expr *expr)
  
  /* Resolves condition and replaces the expression with a boolean. */

  static struct expr *
-expr_simplify_condition(struct expr *expr,
-bool (*is_chassis_resident)(const void *c_aux,
+expr_evaluate_condition__(struct expr *expr,
+  bool (*is_chassis_resident)(const void *c_aux,
  const char *port_name),
-const void *c_aux)
+  const void *c_aux)
  {
  bool result;
  
@@ -2084,13 +2084,41 @@ expr_simplify_condition(struct expr *expr,

  return expr_create_boolean(result);
  }
  
+struct expr *

+expr_evaluate_condition(struct expr *expr,
+bool (*is_chassis_resident)(const void *c_aux,
+const char *port_name),
+const void *c_aux)
+{
+struct expr *sub, *next;
+
+switch (expr->type) {
+case EXPR_T_AND:
+case EXPR_T_OR:
+ LIST_FOR_EACH_SAFE (sub, next, node, >andor) {
+

Re: [ovs-dev] [PATCH v1] rhel: retain OVS_CTL_OPTS for systemd service files

2020-09-09 Thread Ilya Maximets 
> On 1/31/2019 10:38 AM, Aaron Conole wrote:
>> Martin Xu  writes:
>>
>>> OVS init.d script calls ovs-ctl with $OVS_CTL_OPTS defined in the
>>> config file. This variable is replaced by OPTIONS in systemd service
>>> files. This patch addes $OVS_CTL_OPTS back to be passed along with $OPTIONS
>>> for backward compatibility.
>>>
>>> VMware-BZ: #2036847
>>>
>>> Signed-off-by: Martin Xu 
>>> CC: Aaron Conole 
>>> ---
>> I'm not sure why there should be two variables in the sysconfig file for
>> this.  The following would preserve the old and not introduce a new
>> variable (I think.. it's completely untested).  I guess this is because
>> the debian-distro openvswitch-switch.template file doesn't match the
>> rhel-distro template file, and we want to make a common set of systemd
>> scripts? Otherwise I don't see what the purpose is - what is the
>> migration path that this is addressing?
> 
> Aaron,
> 
> I owe you a response on this and will get to it but some fires need 
> putting out at the moment.

Hi, Greg, Martin.

I'm looking through old patches after the patchwork cleanup and this
one seems to be never applied.  I'm assuming that it's not needed
anymore, however, I'd like to have some comment on it if possible.

For now marking it as 'Not Applicable'.  Please, resubmit in case it's
still needed.

Best regards, Ilya Maximets.

> 
> Thanks,
> 
> - Greg
> 
>>
>> ---
>> diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
>> b/rhel/usr_lib_systemd_system_ovsdb-server.service
>> index 09f946bb1..660ec75ef 100644
>> --- a/rhel/usr_lib_systemd_system_ovsdb-server.service
>> +++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
>> @@ -12,6 +12,7 @@ EnvironmentFile=/etc/openvswitch/default.conf
>>   EnvironmentFile=-/etc/sysconfig/openvswitch
>>   ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch 
>> /var/log/openvswitch
>>   ExecStartPre=/bin/sh -c 'rm -f /run/openvswitch/useropts; if [ 
>> "$${OVS_USER_ID/:*/}" != "root" ]; then /usr/bin/echo 
>> "OVSUSER=--ovs-user=${OVS_USER_ID}" > /run/openvswitch/useropts; fi'
>> +ExecStartPre=/bin/sh -c 'if [ "${OVS_CTL_OPTS}" != "" -a "${OPTIONS}" == "" 
>> ]; then /usr/bin/echo "OPTIONS=\"${OPTIONS} ${OVS_CTL_OPTS}\"" >> 
>> /run/openvswitch/useropts; fi'
>>   EnvironmentFile=-/run/openvswitch/useropts
>>   ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
>> --no-ovs-vswitchd --no-monitor --system-id=random \
>> ---
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] Update tutorial for newer versions of Faucet and Open vSwitch.

2020-09-09 Thread Ilya Maximets 
On 9/12/18 1:53 AM, Brad Cowie wrote:
> Newer versions of Faucet use a dynamic OpenFlow pipeline based on what
> features are enabled in the configuration file. Update log output, flow
> table dumps and explanations to be consistent with newer Faucet versions.
> 
> Remove mentions of bugs that we have since fixed in Faucet since the
> tutorial was originally written.
> 
> Adds documentation on changes to Open vSwitch commands to recommend
> using a version that is compatible with the features of the tutorial.
> 
> Reported-by: Matthias Ableidinger 
> Reported-at: 
> https://mail.openvswitch.org/pipermail/ovs-discuss/2018-August/047180.html
> Signed-off-by: Brad Cowie 
> ---

Hi.  I'm looking through old patches after the patchwork cleanup and
this one seems to be never applied.  I do not have any experience with
Faucet, but the patch looks ok and it's still applicable.

Brad, Ben, if it still valid I could apply it.  What do you think?

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

Re: [ovs-dev] [PATCH ovn] Replace ds_put_* evaluating to constant expressions

2020-09-09 Thread Anton Ivanov 

On 09/09/2020 12:20, Ilya Maximets wrote:

On 9/9/20 11:02 AM, anton.iva...@cambridgegreys.com wrote:

From: Anton Ivanov 

Replaces ds_put_cstr and ds_put_format which evaluate to
constant expressions with string constants

Signed-off-by: Anton Ivanov 
---


Hi.  Thanks for the fix!

One general comment to almost all of your patches:
Please, use the following format for the patch subject:
   : Summary.
i.e. add an : part that should represent a piece of code this patch
is targeted to.  For example, it should be 'ovn-northd:' for this patch.
And it's also better to end the patch subject with a period.  You already
starting the summary with a capital letter, so this  part is fine.

https://docs.ovn.org/en/latest/internals/contributing/submitting-patches.html#email-subject

Some comments for the patch inline.


Cool. I will address them in the next revision. I found one more case of 
constant format as well.




Best regards, Ilya Maximets.


  northd/ovn-northd.c | 56 ++---
  1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 3de71612b..c681d5cbf 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -1928,6 +1928,8 @@ update_dynamic_addresses(struct dynamic_address_update 
*update)
  
  struct ds new_addr = DS_EMPTY_INITIALIZER;

  ds_put_format(_addr, ETH_ADDR_FMT, ETH_ADDR_ARGS(mac));
+
+


This is totally unrelated and not needed.


  ipam_insert_mac(, true);
  
  if (ip4) {

@@ -6950,24 +6952,24 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
  struct ds action = DS_EMPTY_INITIALIZER;
  
  ds_clear();


No need to clear.


-ds_put_cstr(, "udp.dst == 53");
+const char *dns_match = "udp.dst == 53";


It might be better to just pass this string directly  to the ovn_lflow_add.


  ds_put_format(,
REGBIT_DNS_LOOKUP_RESULT" = dns_lookup(); next;");
  ovn_lflow_add(lflows, od, S_SWITCH_IN_DNS_LOOKUP, 100,
-  ds_cstr(), ds_cstr());
+  dns_match, ds_cstr());
  ds_clear();
-ds_put_cstr(, " && "REGBIT_DNS_LOOKUP_RESULT);
-ds_put_format(, "eth.dst <-> eth.src; ip4.src <-> ip4.dst; "
+
+const char *dns_match2 = "udp.dst == 53 && "REGBIT_DNS_LOOKUP_RESULT;


Please, declare a single variable once for both cases at the top of the block.


+const char *dns_action = "eth.dst <-> eth.src; ip4.src <-> ip4.dst; "


And this block is not big, so , please, declare this variable at the top of the
block too.


"udp.dst = udp.src; udp.src = 53; outport = inport; "
-  "flags.loopback = 1; output;");
+  "flags.loopback = 1; output;";
  ovn_lflow_add(lflows, od, S_SWITCH_IN_DNS_RESPONSE, 100,
-  ds_cstr(), ds_cstr());
-ds_clear();
-ds_put_format(, "eth.dst <-> eth.src; ip6.src <-> ip6.dst; "
+  dns_match2, dns_action);
+dns_action = "eth.dst <-> eth.src; ip6.src <-> ip6.dst; "
"udp.dst = udp.src; udp.src = 53; outport = inport; "
-  "flags.loopback = 1; output;");
+  "flags.loopback = 1; output;";
  ovn_lflow_add(lflows, od, S_SWITCH_IN_DNS_RESPONSE, 100,
-  ds_cstr(), ds_cstr());
+  dns_match2, dns_action);
  ds_destroy();
  }
  
@@ -7839,10 +7841,10 @@ add_ecmp_symmetric_reply_flows(struct hmap *lflows,

  _route->header_);
  
  ds_clear();


No more need to clear.


-ds_put_cstr(, "eth.dst = ct_label.ecmp_reply_eth; next;");
+const char *action = "eth.dst = ct_label.ecmp_reply_eth; next;";


It might be better to just pass this string directly  to the function below,
i.e. without any intermediate variables.


  ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_ARP_RESOLVE,
  200, ds_cstr(_reply),
-ds_cstr(), _route->header_);
+action, _route->header_);
  }
  
  static void

@@ -8734,9 +8736,9 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
  
  /* TTL discard */

  ds_clear();


No more need to clear.


-ds_put_cstr(, "ip4 && ip.ttl == {0, 1}");
+const char *ttl_match = "ip4 && ip.ttl == {0, 1}";


Ditto.


  ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 30,
-  ds_cstr(), "drop;");
+  ttl_match, "drop;");
  
  /* Pass other traffic not already handled to the next table for

   * routing. */
@@ -8778,14 +8780,13 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
  ds_put_cstr(, " && icmp4.type == 8 && icmp4.code == 0");
  
  ds_clear();


No more need to clear.


-ds_put_format(,
-

Re: [ovs-dev] [Windows] Update documentation and build

2020-09-09 Thread Ilya Maximets 
On 9/8/20 10:48 PM, Alin Gabriel Serdean wrote:
> The following patchset contains small updates in the build process and
> corresponding documentation.
> 
> Alin Serdean (8):
>   windows: Remove unused variable
>   windows: Add default value for VSTUDIO_CONFIG
>   datapath_windows: Add datapath_windows target
>   windows, documentation: Recommend latest VS and WDK version
>   windows, installer: Bundle latest runtime version
>   windows: Update build with latest pthread project
>   windows,installer: Bundle Windows 10 driver
>   appveyor: Bump outdated links and add artifacts
> 

Hi Alin.  Thanks for working on this!

This patch set breaks the linux build, at least on patch #2.
You might noticed the reply from 0-day robot.  Also, all our
CI systems failed:
  https://travis-ci.org/github/ovsrobot/ovs/jobs/725379551#L3191

Please, take a look.

And in case you missed my reply for the previous version of patch #8,
I had a question if we could use openssl 1.1.1 instead of 1.0.2 since
1.0.2 is not recommended to use.

Best regards, Ilya Maximets.

>  Documentation/intro/install/windows.rst   | 13 +++--
>  appveyor.yml  | 54 +++
>  datapath-windows/automake.mk  |  4 ++
>  m4/openvswitch.m4 | 21 ++--
>  windows/automake.mk   |  9 ++--
>  .../ovs-windows-installer/Driver/.gitignore   |  1 +
>  .../Driver/Win10/.gitignore   |  3 ++
>  windows/ovs-windows-installer/Product.wxs | 23 ++--
>  8 files changed, 79 insertions(+), 49 deletions(-)
>  create mode 100644 windows/ovs-windows-installer/Driver/Win10/.gitignore
> 

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

Re: [ovs-dev] [PATCH ovn] Replace ds_put_* evaluating to constant expressions

2020-09-09 Thread Ilya Maximets 
On 9/9/20 11:02 AM, anton.iva...@cambridgegreys.com wrote:
> From: Anton Ivanov 
> 
> Replaces ds_put_cstr and ds_put_format which evaluate to
> constant expressions with string constants
> 
> Signed-off-by: Anton Ivanov 
> ---

Hi.  Thanks for the fix!

One general comment to almost all of your patches:
Please, use the following format for the patch subject:
  : Summary.
i.e. add an : part that should represent a piece of code this patch
is targeted to.  For example, it should be 'ovn-northd:' for this patch.
And it's also better to end the patch subject with a period.  You already
starting the summary with a capital letter, so this  part is fine.

https://docs.ovn.org/en/latest/internals/contributing/submitting-patches.html#email-subject

Some comments for the patch inline.

Best regards, Ilya Maximets.

>  northd/ovn-northd.c | 56 ++---
>  1 file changed, 28 insertions(+), 28 deletions(-)
> 
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 3de71612b..c681d5cbf 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -1928,6 +1928,8 @@ update_dynamic_addresses(struct dynamic_address_update 
> *update)
>  
>  struct ds new_addr = DS_EMPTY_INITIALIZER;
>  ds_put_format(_addr, ETH_ADDR_FMT, ETH_ADDR_ARGS(mac));
> +
> +

This is totally unrelated and not needed.

>  ipam_insert_mac(, true);
>  
>  if (ip4) {
> @@ -6950,24 +6952,24 @@ build_lswitch_flows(struct hmap *datapaths, struct 
> hmap *ports,
>  struct ds action = DS_EMPTY_INITIALIZER;
>  
>  ds_clear();

No need to clear.

> -ds_put_cstr(, "udp.dst == 53");
> +const char *dns_match = "udp.dst == 53";

It might be better to just pass this string directly  to the ovn_lflow_add.

>  ds_put_format(,
>REGBIT_DNS_LOOKUP_RESULT" = dns_lookup(); next;");
>  ovn_lflow_add(lflows, od, S_SWITCH_IN_DNS_LOOKUP, 100,
> -  ds_cstr(), ds_cstr());
> +  dns_match, ds_cstr());
>  ds_clear();
> -ds_put_cstr(, " && "REGBIT_DNS_LOOKUP_RESULT);
> -ds_put_format(, "eth.dst <-> eth.src; ip4.src <-> ip4.dst; "
> +
> +const char *dns_match2 = "udp.dst == 53 && "REGBIT_DNS_LOOKUP_RESULT;

Please, declare a single variable once for both cases at the top of the block.

> +const char *dns_action = "eth.dst <-> eth.src; ip4.src <-> ip4.dst; "

And this block is not big, so , please, declare this variable at the top of the
block too.

>"udp.dst = udp.src; udp.src = 53; outport = inport; "
> -  "flags.loopback = 1; output;");
> +  "flags.loopback = 1; output;";
>  ovn_lflow_add(lflows, od, S_SWITCH_IN_DNS_RESPONSE, 100,
> -  ds_cstr(), ds_cstr());
> -ds_clear();
> -ds_put_format(, "eth.dst <-> eth.src; ip6.src <-> ip6.dst; "
> +  dns_match2, dns_action);
> +dns_action = "eth.dst <-> eth.src; ip6.src <-> ip6.dst; "
>"udp.dst = udp.src; udp.src = 53; outport = inport; "
> -  "flags.loopback = 1; output;");
> +  "flags.loopback = 1; output;";
>  ovn_lflow_add(lflows, od, S_SWITCH_IN_DNS_RESPONSE, 100,
> -  ds_cstr(), ds_cstr());
> +  dns_match2, dns_action);
>  ds_destroy();
>  }
>  
> @@ -7839,10 +7841,10 @@ add_ecmp_symmetric_reply_flows(struct hmap *lflows,
>  _route->header_);
>  
>  ds_clear();

No more need to clear.

> -ds_put_cstr(, "eth.dst = ct_label.ecmp_reply_eth; next;");
> +const char *action = "eth.dst = ct_label.ecmp_reply_eth; next;";

It might be better to just pass this string directly  to the function below,
i.e. without any intermediate variables.

>  ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_ARP_RESOLVE,
>  200, ds_cstr(_reply),
> -ds_cstr(), _route->header_);
> +action, _route->header_);
>  }
>  
>  static void
> @@ -8734,9 +8736,9 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
> *ports,
>  
>  /* TTL discard */
>  ds_clear();

No more need to clear.

> -ds_put_cstr(, "ip4 && ip.ttl == {0, 1}");
> +const char *ttl_match = "ip4 && ip.ttl == {0, 1}";

Ditto.

>  ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 30,
> -  ds_cstr(), "drop;");
> +  ttl_match, "drop;");
>  
>  /* Pass other traffic not already handled to the next table for
>   * routing. */
> @@ -8778,14 +8780,13 @@ build_lrouter_flows(struct hmap *datapaths, struct 
> hmap *ports,
>  ds_put_cstr(, " && icmp4.type == 8 && icmp4.code == 0");
>  
>  ds_clear();

No more need to clear.

> -ds_put_format(,
> -"ip4.dst <-> ip4.src; "

Re: [ovs-dev] [PATCH ovn v2] ovn-northd: Optionally skip the check of lsp_is_up.

2020-09-09 Thread Numan Siddique 
On Wed, Sep 9, 2020 at 2:52 AM Han Zhou  wrote:
>
> The checking of lsp "up" status before adding ARP responder flows was
> added to avoid confusion in cases when a network admin sees ARP response
> for VM/containers that is not up on chassis yet. However, this check
> introduces an extra round of flow change in SB and triggers computes
> on hypervisors which is unnecessary cost in most cases, especially for
> large scale environment. To improve this, this patch provides an option
> ignore_lsp_down to disable the check for the use cases when ARP reponse for a
> port that is "down" isn't considered harmful.
>
> Acked-by: Numan Siddique 
> Signed-off-by: Han Zhou 
> ---
> v1 -> v2: change the option from "check_lsp_is_up" to "ignore_lsp_down" as
>   suggested by Numan.


Hi Han,

The patch LGTM.

Thanks
Numan

>
>  northd/ovn-northd.8.xml | 15 ++-
>  northd/ovn-northd.c |  7 ++-
>  ovn-nb.xml  | 13 +
>  tests/ovn-northd.at | 14 ++
>  4 files changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index d15245a..a275442 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -763,9 +763,11 @@ output;
>
>  
>These flows are omitted for logical ports (other than router ports 
> or
> -  localport ports) that are down, for logical ports of
> -  type virtual and for logical ports with 'unknown'
> -  address set.
> +  localport ports) that are down (unless 
> +  ignore_lsp_down is configured as true in 
> options
> +  column of NB_Global table of the 
> Northbound
> +  database), for logical ports of type virtual and for
> +  logical ports with 'unknown' address set.
>  
>
>
> @@ -812,8 +814,11 @@ nd_na_router {
>
>  
>These flows are omitted for logical ports (other than router ports 
> or
> -  localport ports) that are down and for logical ports 
> of
> -  type virtual.
> +  localport ports) that are down (unless 
> +  ignore_lsp_down is configured as true in 
> options
> +  column of NB_Global table of the 
> Northbound
> +  database), for logical ports of type virtual and for
> +  logical ports with 'unknown' address set.
>  
>
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index bebb25a..100d9dc 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -87,6 +87,8 @@ static struct eth_addr mac_prefix;
>
>  static bool controller_event_en;
>
> +static bool check_lsp_is_up;
> +
>  /* MAC allocated for service monitor usage. Just one mac is allocated
>   * for this purpose and ovn-controller's on each chassis will make use
>   * of this mac when sending out the packets to monitor the services
> @@ -6741,7 +6743,8 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
> *ports,
>   *  - port type is router or
>   *  - port type is localport
>   */
> -if (!lsp_is_up(op->nbsp) && strcmp(op->nbsp->type, "router") &&
> +if (check_lsp_is_up &&
> +!lsp_is_up(op->nbsp) && strcmp(op->nbsp->type, "router") &&
>  strcmp(op->nbsp->type, "localport")) {
>  continue;
>  }
> @@ -11831,6 +11834,8 @@ ovnnb_db_run(struct northd_context *ctx,
>
>  controller_event_en = smap_get_bool(>options,
>  "controller_event", false);
> +check_lsp_is_up = !smap_get_bool(>options,
> + "ignore_lsp_down", false);
>
>  build_datapaths(ctx, datapaths, lr_list);
>  build_ports(ctx, sbrec_chassis_by_name, datapaths, ports);
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 109ffcf..0bfe626 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -167,6 +167,19 @@
>  
>
>
> +  
> +
> +  If set to false, ARP/ND reply flows for logical switch ports will 
> be
> +  installed only if the port is up, i.e. claimed by a Chassis. If set
> +  to true, these flows are installed regardless of the status of the
> +  port, which can result in a situation that ARP request to an IP is
> +  resolved even before the relevant VM/container is running. For
> +  environments where this is not an issue, setting it to
> +  true can reduce the load and latency of the control
> +  plane. The default value is false.
> +
> +  
> +
>
>  
>These options control how routes are advertised between OVN
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index c6d9ae11..b89c585 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -1991,3 +1991,17 @@ AT_CHECK([ovn-sbctl lflow-list | grep 
> "ls_out_pre_lb.*priority=100" | grep reg0
>  ])
>
>  AT_CLEANUP
> +
> +AT_SETUP([ovn --

Re: [ovs-dev] [PATCH ovn] Replace ds_put_* evaluating to constant expressions

2020-09-09 Thread Numan Siddique 
On Wed, Sep 9, 2020 at 2:33 PM  wrote:
>
> From: Anton Ivanov 
>
> Replaces ds_put_cstr and ds_put_format which evaluate to
> constant expressions with string constants
>
> Signed-off-by: Anton Ivanov 

Acked-by: Numan Siddique 

Thanks
Numan

> ---
>  northd/ovn-northd.c | 56 ++---
>  1 file changed, 28 insertions(+), 28 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 3de71612b..c681d5cbf 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -1928,6 +1928,8 @@ update_dynamic_addresses(struct dynamic_address_update 
> *update)
>
>  struct ds new_addr = DS_EMPTY_INITIALIZER;
>  ds_put_format(_addr, ETH_ADDR_FMT, ETH_ADDR_ARGS(mac));
> +
> +
>  ipam_insert_mac(, true);
>
>  if (ip4) {
> @@ -6950,24 +6952,24 @@ build_lswitch_flows(struct hmap *datapaths, struct 
> hmap *ports,
>  struct ds action = DS_EMPTY_INITIALIZER;
>
>  ds_clear();
> -ds_put_cstr(, "udp.dst == 53");
> +const char *dns_match = "udp.dst == 53";
>  ds_put_format(,
>REGBIT_DNS_LOOKUP_RESULT" = dns_lookup(); next;");
>  ovn_lflow_add(lflows, od, S_SWITCH_IN_DNS_LOOKUP, 100,
> -  ds_cstr(), ds_cstr());
> +  dns_match, ds_cstr());
>  ds_clear();
> -ds_put_cstr(, " && "REGBIT_DNS_LOOKUP_RESULT);
> -ds_put_format(, "eth.dst <-> eth.src; ip4.src <-> ip4.dst; "
> +
> +const char *dns_match2 = "udp.dst == 53 && "REGBIT_DNS_LOOKUP_RESULT;
> +const char *dns_action = "eth.dst <-> eth.src; ip4.src <-> ip4.dst; "
>"udp.dst = udp.src; udp.src = 53; outport = inport; "
> -  "flags.loopback = 1; output;");
> +  "flags.loopback = 1; output;";
>  ovn_lflow_add(lflows, od, S_SWITCH_IN_DNS_RESPONSE, 100,
> -  ds_cstr(), ds_cstr());
> -ds_clear();
> -ds_put_format(, "eth.dst <-> eth.src; ip6.src <-> ip6.dst; "
> +  dns_match2, dns_action);
> +dns_action = "eth.dst <-> eth.src; ip6.src <-> ip6.dst; "
>"udp.dst = udp.src; udp.src = 53; outport = inport; "
> -  "flags.loopback = 1; output;");
> +  "flags.loopback = 1; output;";
>  ovn_lflow_add(lflows, od, S_SWITCH_IN_DNS_RESPONSE, 100,
> -  ds_cstr(), ds_cstr());
> +  dns_match2, dns_action);
>  ds_destroy();
>  }
>
> @@ -7839,10 +7841,10 @@ add_ecmp_symmetric_reply_flows(struct hmap *lflows,
>  _route->header_);
>
>  ds_clear();
> -ds_put_cstr(, "eth.dst = ct_label.ecmp_reply_eth; next;");
> +const char *action = "eth.dst = ct_label.ecmp_reply_eth; next;";
>  ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_ARP_RESOLVE,
>  200, ds_cstr(_reply),
> -ds_cstr(), _route->header_);
> +action, _route->header_);
>  }
>
>  static void
> @@ -8734,9 +8736,9 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
> *ports,
>
>  /* TTL discard */
>  ds_clear();
> -ds_put_cstr(, "ip4 && ip.ttl == {0, 1}");
> +const char *ttl_match = "ip4 && ip.ttl == {0, 1}";
>  ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 30,
> -  ds_cstr(), "drop;");
> +  ttl_match, "drop;");
>
>  /* Pass other traffic not already handled to the next table for
>   * routing. */
> @@ -8778,14 +8780,13 @@ build_lrouter_flows(struct hmap *datapaths, struct 
> hmap *ports,
>  ds_put_cstr(, " && icmp4.type == 8 && icmp4.code == 0");
>
>  ds_clear();
> -ds_put_format(,
> -"ip4.dst <-> ip4.src; "
> -"ip.ttl = 255; "
> -"icmp4.type = 0; "
> -"flags.loopback = 1; "
> -"next; ");
> +const char * icmp_actions = "ip4.dst <-> ip4.src; "
> +"ip.ttl = 255; "
> +"icmp4.type = 0; "
> +"flags.loopback = 1; "
> +"next; ";
>  ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
> -ds_cstr(), ds_cstr(),
> +ds_cstr(), icmp_actions,
>  >nbrp->header_);
>  }
>
> @@ -9184,14 +9185,14 @@ build_lrouter_flows(struct hmap *datapaths, struct 
> hmap *ports,
>  ds_put_cstr(, " && icmp6.type == 128 && icmp6.code == 0");
>
>  ds_clear();
> -ds_put_cstr(,
> +const char *lrp_actions =
>  "ip6.dst <-> ip6.src; "
>  "ip.ttl = 255; "
>

Re: [ovs-dev] [PATCH v1 1/1] DPDK: Remove support for vhost-user zero-copy.

2020-09-09 Thread Kevin Traynor 
On 08/09/2020 16:26, Ian Stokes wrote:
> Support for vhost-user dequeue zero-copy was deprecated in OVS 2.14 with
> the aim of removing it for OVS 2.15. Support for zero-copy will also be
> removed from DPDK 20.11. As such remove support from OVS.
> 

Maybe you can add some links to the DPDK patches and OVS discussions
seen as they are being requested on the ML.

> Signed-off-by: Ian Stokes 
> ---
>  Documentation/topics/dpdk/vhost-user.rst | 72 
> 
>  NEWS |  2 +
>  lib/netdev-dpdk.c| 25 ---
>  vswitchd/vswitch.xml | 11 -
>  4 files changed, 2 insertions(+), 108 deletions(-)
> 
> diff --git a/Documentation/topics/dpdk/vhost-user.rst 
> b/Documentation/topics/dpdk/vhost-user.rst
> index 4af738d11..595e40cde 100644
> --- a/Documentation/topics/dpdk/vhost-user.rst
> +++ b/Documentation/topics/dpdk/vhost-user.rst
> @@ -553,78 +553,6 @@ shown with::
>  
>$ ovs-vsctl get Interface dpdkvhostclient0 statistics:ovs_tx_retries
>  
> -vhost-user Dequeue Zero Copy (experimental)
> 
> -
> -.. warning::
> -
> -   vhost-user Dequeue Zero Copy is deprecated in OVS and will be removed in
> -   the next release.
> -
> -Normally when dequeuing a packet from a vHost User device, a memcpy operation
> -must be used to copy that packet from guest address space to host address
> -space. This memcpy can be removed by enabling dequeue zero-copy like so::
> -
> -$ ovs-vsctl add-port br0 dpdkvhostuserclient0 -- set Interface \
> -dpdkvhostuserclient0 type=dpdkvhostuserclient \
> -options:vhost-server-path=/tmp/dpdkvhostclient0 \
> -options:dq-zero-copy=true
> -
> -With this feature enabled, a reference (pointer) to the packet is passed to
> -the host, instead of a copy of the packet. Removing this memcpy can give a
> -performance improvement for some use cases, for example switching large 
> packets
> -between different VMs. However additional packet loss may be observed.
> -
> -Note that the feature is disabled by default and must be explicitly enabled
> -by setting the ``dq-zero-copy`` option to ``true`` while specifying the
> -``vhost-server-path`` option as above. If you wish to split out the command
> -into multiple commands as below, ensure ``dq-zero-copy`` is set before
> -``vhost-server-path``::
> -
> -$ ovs-vsctl set Interface dpdkvhostuserclient0 options:dq-zero-copy=true
> -$ ovs-vsctl set Interface dpdkvhostuserclient0 \
> -options:vhost-server-path=/tmp/dpdkvhostclient0
> -
> -The feature is only available to ``dpdkvhostuserclient`` port types.
> -
> -A limitation exists whereby if packets from a vHost port with
> -``dq-zero-copy=true`` are destined for a ``dpdk`` type port, the number of tx
> -descriptors (``n_txq_desc``) for that port must be reduced to a smaller 
> number,
> -128 being the recommended value. This can be achieved by issuing the 
> following
> -command::
> -
> -$ ovs-vsctl set Interface dpdkport options:n_txq_desc=128
> -
> -Note: The sum of the tx descriptors of all ``dpdk`` ports the VM will send to
> -should not exceed 128. For example, in case of a bond over two physical ports
> -in balance-tcp mode, one must divide 128 by the number of links in the bond.
> -
> -Refer to :ref:`dpdk-queues-sizes` for more information.
> -
> -The reason for this limitation is due to how the zero copy functionality is
> -implemented. The vHost device's 'tx used vring', a virtio structure used for
> -tracking used ie. sent descriptors, will only be updated when the NIC frees
> -the corresponding mbuf. If we don't free the mbufs frequently enough, that
> -vring will be starved and packets will no longer be processed. One way to
> -ensure we don't encounter this scenario, is to configure ``n_txq_desc`` to a
> -small enough number such that the 'mbuf free threshold' for the NIC will be 
> hit
> -more often and thus free mbufs more frequently. The value of 128 is 
> suggested,
> -but values of 64 and 256 have been tested and verified to work too, with
> -differing performance characteristics. A value of 512 can be used too, if the
> -virtio queue size in the guest is increased to 1024 (available to configure 
> in
> -QEMU versions v2.10 and greater). This value can be set like so::
> -
> -$ qemu-system-x86_64 ... -chardev socket,id=char1,path=,server
> -  -netdev type=vhost-user,id=mynet1,chardev=char1,vhostforce
> -  -device virtio-net-pci,mac=00:00:00:00:00:01,netdev=mynet1,
> -  tx_queue_size=1024
> -
> -Because of this limitation, this feature is considered 'experimental'.
> -
> -.. note::
> -
> -   Post-copy Live Migration is not compatible with dequeue zero copy.

There's a reference to this in the post-copy section too that can be
removed.

> -
>  Further information can be found in the
>  `DPDK documentation
>  `__
> diff

Re: [ovs-dev] [PATCH ovn] Remove duplicate include file

2020-09-09 Thread Numan Siddique 
On Wed, Sep 9, 2020 at 3:36 PM Yi Li  wrote:
>
> Signed-off-by: Yi Li 

Thanks. I applied this patch to master.

Numan'

> ---
>  controller/bfd.c | 1 -
>  controller/pinctrl.c | 1 -
>  2 files changed, 2 deletions(-)
>
> diff --git a/controller/bfd.c b/controller/bfd.c
> index b305eb15..cf011e38 100644
> --- a/controller/bfd.c
> +++ b/controller/bfd.c
> @@ -17,7 +17,6 @@
>  #include "bfd.h"
>  #include "encaps.h"
>  #include "lport.h"
> -#include "ovn-controller.h"
>
>  #include "lib/hash.h"
>  #include "lib/sset.h"
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index bb7bb41b..0a702053 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -27,7 +27,6 @@
>  #include "ha-chassis.h"
>  #include "lport.h"
>  #include "nx-match.h"
> -#include "ovn-controller.h"
>  #include "latch.h"
>  #include "lib/packets.h"
>  #include "lib/sset.h"
> --
> 2.25.3
>
>
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn 3/4] expr: Evaluate the condition expression in a separate step.

2020-09-09 Thread Numan Siddique 
On Wed, Sep 9, 2020 at 1:32 AM Mark Michelson  wrote:
>
> I know this is weird because I ACKed this exact patch once in the past,
> but I found a small issue. See below:
>
> On 9/2/20 2:46 PM, num...@ovn.org wrote:
> > From: Numan Siddique 
> >
> > This patch was committed earlier [1] and then reverted [2].
> >
> > A new function is added - expr_evaluate_condition() which evaluates
> > the condition expression - is_chassis_resident. expr_simply() will
> > no longer evaluates this condition.
> >
> > An upcoming commit needs this in order to cache the logical flow expressions
> > so that every lflow_*() function which calls consider_logical_flow() doesn't
> > need to convert the logical flow match to an expression. Instead the expr 
> > tree
> > for the logical flow is cached. Since we can't cache the is_chassis_resident
> > condition, it is separated out.
> >
> > [1] - 99e3a145927("expr: Evaluate the condition expression in a separate 
> > step.")
> > [2] - 065bcf46218("ovn-controller: Revert lflow expr caching")
> >
> > Signed-off-by: Numan Siddique 
> > ---
> >   controller/lflow.c|  3 ++-
> >   include/ovn/expr.h| 10 +
> >   lib/expr.c| 50 +--
> >   tests/test-ovn.c  | 11 +-
> >   utilities/ovn-trace.c |  6 --
> >   5 files changed, 57 insertions(+), 23 deletions(-)
> >
> > diff --git a/controller/lflow.c b/controller/lflow.c
> > index c2f939f90f..63df5611ec 100644
> > --- a/controller/lflow.c
> > +++ b/controller/lflow.c
> > @@ -684,8 +684,9 @@ consider_logical_flow(const struct sbrec_logical_flow 
> > *lflow,
> >   .lflow = lflow,
> >   .lfrr = l_ctx_out->lfrr
> >   };
> > -expr = expr_simplify(expr, is_chassis_resident_cb, _aux);
> > +expr = expr_simplify(expr);
> >   expr = expr_normalize(expr);
>
> expr_normalize() has the following in it:
>
>  /* Should not hit expression type condition, since expr_normalize
> is
>
>
>   * only called after expr_simplify, which resolves all conditions.
> */
>
>
>  case EXPR_T_CONDITION:
>
>
>
>  default:
>
>
>
>  OVS_NOT_REACHED();
>
>
>
>  }
>
> expr_normalize() assumes that expr_simplify has evaluated all
> conditions, but now that is not the case. As it turns out, as long as
> is_chassis_resident() is part of an AND or OR expression, then things
> still work out ok since expr_normalize() is not called recursively.
>
> However, if for some reason we ever decided to create an expression that
> was just a bare is_chassis_resident("blah"), then this would abort.
>
> Even though I'm doubtful we would ever create an expression that was
> just is_chassis_resident("blah"), I think expr_normalize() should be
> fixed just in case. If nothing else, change the comment since it is
> incorrect now.

Thanks Mark for the review and comments. Excellent catch.
I found it a bit hard to allow EXPR_T_CONDITION in expr_normalize().
I think it could complicate the processing. Hence I changed the approach a bit
in v2. For logical flows having is_chassis_resident() match, we cache
the simplified expr
tree now. So after retrieving the simplified expr tree from the cache,
we clone the expr,
evaluate it and then normalize it. This would definitely cost a bit
more. But I think this
is better to guarantee accuracy.

We can revisit later if we want to allow EXPR_T_CONDITION in expr_normalize().
I think for now it should be ok since we cache the expr matches for
the majority of logical flows.

Please take a look at v2 -
https://patchwork.ozlabs.org/project/ovn/list/?series=200461
https://mail.openvswitch.org/pipermail/ovs-dev/2020-September/374704.html

Thanks
Numan


>
> > +expr = expr_evaluate_condition(expr, is_chassis_resident_cb, 
> > _aux);
> >   uint32_t n_conjs = expr_to_matches(expr, lookup_port_cb, ,
> >  );
> >   expr_destroy(expr);
> > diff --git a/include/ovn/expr.h b/include/ovn/expr.h
> > index b34fb0e813..ed7b054144 100644
> > --- a/include/ovn/expr.h
> > +++ b/include/ovn/expr.h
> > @@ -432,10 +432,12 @@ void expr_destroy(struct expr *);
> >
> >   struct expr *expr_annotate(struct expr *, const struct shash *symtab,
> >  char **errorp);
> > -struct expr *expr_simplify(struct expr *,
> > -   bool (*is_chassis_resident)(const void *c_aux,
> > -   const char 
> > *port_name),
> > -   const void *c_aux);
> > +struct expr *expr_simplify(struct expr *);
> > +struct expr *expr_evaluate_condition(
> > +struct expr *,
> > +bool (*is_chassis_resident)(const void *c_aux,
> > +const char *port_name),
> > +const void *c_aux);
> >   struct expr *expr_normalize(struct expr *);
> >
> >   bool expr_honors_invariants(const struct expr *);
> > diff --git a/lib/expr.c b/lib/expr.c
> > index 6fb96757ad..bff48dfd20 100644
> > ---

[ovs-dev] [PATCH ovn v2 3/4] expr: Evaluate the condition expression in a separate step.

2020-09-09 Thread numans 
From: Numan Siddique 

A similar patch was committed earlier [1] and then reverted [2].
This patch is similar to [1], but not exactly same.

A new function is added - expr_evaluate_condition() which evaluates
the condition expression - is_chassis_resident. expr_simply() will
no longer evaluates this condition. expr_normalize() is still expected
to be called after expr_evaluate_condition(). Otherwise it will assert
if 'is_chassis_resident()' is not evaluated.

An upcoming commit needs this in order to cache the logical flow expressions
for logical flows having 'is_chassis_resident()' condition in their match.
The expr tree after expr_simplify()  for such logical flows is cached. Since
we can't cache the is_chassis_resident condition, it is separated out.
(For logical flows which do not have this condition in their match, the expr 
matches
will be cached.)

[1] - 99e3a145927("expr: Evaluate the condition expression in a separate step.")
[2] - 065bcf46218("ovn-controller: Revert lflow expr caching")

Signed-off-by: Numan Siddique 
---
 controller/lflow.c|  3 ++-
 include/ovn/expr.h| 10 +
 lib/expr.c| 50 +--
 tests/test-ovn.c  | 11 +-
 utilities/ovn-trace.c |  6 --
 5 files changed, 57 insertions(+), 23 deletions(-)

diff --git a/controller/lflow.c b/controller/lflow.c
index c2f939f90f..c6e1586283 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -684,7 +684,8 @@ consider_logical_flow(const struct sbrec_logical_flow 
*lflow,
 .lflow = lflow,
 .lfrr = l_ctx_out->lfrr
 };
-expr = expr_simplify(expr, is_chassis_resident_cb, _aux);
+expr = expr_simplify(expr);
+expr = expr_evaluate_condition(expr, is_chassis_resident_cb, _aux);
 expr = expr_normalize(expr);
 uint32_t n_conjs = expr_to_matches(expr, lookup_port_cb, ,
);
diff --git a/include/ovn/expr.h b/include/ovn/expr.h
index b34fb0e813..ed7b054144 100644
--- a/include/ovn/expr.h
+++ b/include/ovn/expr.h
@@ -432,10 +432,12 @@ void expr_destroy(struct expr *);
 
 struct expr *expr_annotate(struct expr *, const struct shash *symtab,
char **errorp);
-struct expr *expr_simplify(struct expr *,
-   bool (*is_chassis_resident)(const void *c_aux,
-   const char *port_name),
-   const void *c_aux);
+struct expr *expr_simplify(struct expr *);
+struct expr *expr_evaluate_condition(
+struct expr *,
+bool (*is_chassis_resident)(const void *c_aux,
+const char *port_name),
+const void *c_aux);
 struct expr *expr_normalize(struct expr *);
 
 bool expr_honors_invariants(const struct expr *);
diff --git a/lib/expr.c b/lib/expr.c
index 6fb96757ad..bff48dfd20 100644
--- a/lib/expr.c
+++ b/lib/expr.c
@@ -2063,10 +2063,10 @@ expr_simplify_relational(struct expr *expr)
 
 /* Resolves condition and replaces the expression with a boolean. */
 static struct expr *
-expr_simplify_condition(struct expr *expr,
-bool (*is_chassis_resident)(const void *c_aux,
+expr_evaluate_condition__(struct expr *expr,
+  bool (*is_chassis_resident)(const void *c_aux,
 const char *port_name),
-const void *c_aux)
+  const void *c_aux)
 {
 bool result;
 
@@ -2084,13 +2084,41 @@ expr_simplify_condition(struct expr *expr,
 return expr_create_boolean(result);
 }
 
+struct expr *
+expr_evaluate_condition(struct expr *expr,
+bool (*is_chassis_resident)(const void *c_aux,
+const char *port_name),
+const void *c_aux)
+{
+struct expr *sub, *next;
+
+switch (expr->type) {
+case EXPR_T_AND:
+case EXPR_T_OR:
+ LIST_FOR_EACH_SAFE (sub, next, node, >andor) {
+ovs_list_remove(>node);
+struct expr *e = expr_evaluate_condition(sub, is_chassis_resident,
+ c_aux);
+e = expr_fix(e);
+expr_insert_andor(expr, next, e);
+}
+return expr_fix(expr);
+
+case EXPR_T_CONDITION:
+return expr_evaluate_condition__(expr, is_chassis_resident, c_aux);
+
+case EXPR_T_CMP:
+case EXPR_T_BOOLEAN:
+return expr;
+}
+
+OVS_NOT_REACHED();
+}
+
 /* Takes ownership of 'expr' and returns an equivalent expression whose
  * EXPR_T_CMP nodes use only tests for equality (EXPR_R_EQ). */
 struct expr *
-expr_simplify(struct expr *expr,
-  bool (*is_chassis_resident)(const void *c_aux,
-const char *port_name),
-  const void *c_aux)
+expr_simplify(struct expr *expr)
 {
 struct expr *sub, *next;
 
@@ -2105,8 +2133,7 @@

[ovs-dev] [PATCH ovn v2 4/4] ovn-controller: Cache logical flow expr matches.

2020-09-09 Thread numans 
From: Numan Siddique 

This patch is a second attempt in caching the expr tree of logical flows.
Earlier attemp [1] was reverted.

In this approach, we do the following

  - If a logical flow match has any port group/address set references, we don't
cache expr match or expr tree.

  - If a logical flow match doesn't have a port group/address set reference and
expr condition match - is_chassis_resident, we cache the expr matches
(which is hmap of struct expr_match *).

  - If a logical flow match has expr condition match - is_chassis_resident we
cache the simplified 'expr' tree.

Caching is configurable and can be disabled if desired. The second patch of 
this series
added this support.

In my testing, a complete lflow_run() took around 5 seconds for around 90k 
logical flows,
where as it took around 2 seconds with caching.

Signed-off-by: Numan Siddique 
---
 controller/lflow.c| 459 --
 include/ovn/expr.h|   2 +-
 lib/expr.c|  17 +-
 tests/test-ovn.c  |   5 +-
 utilities/ovn-trace.c |   2 +-
 5 files changed, 327 insertions(+), 158 deletions(-)

diff --git a/controller/lflow.c b/controller/lflow.c
index c6e1586283..59b32ab803 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -269,14 +269,34 @@ lflow_resource_destroy_lflow(struct lflow_resource_ref 
*lfrr,
 free(lfrn);
 }
 
+enum lflow_cache_type {
+LCACHE_T_NO_CACHE,
+LCACHE_T_MATCHES,
+LCACHE_T_EXPR,
+};
+
 /* Represents an lflow cache which
  *  - stores the conjunction id offset if the lflow matches
  *results in conjunctive OpenvSwitch flows.
+ *
+ *  - Caches
+ * (1) Nothing if the logical flow has port group/address set references.
+ * (2) expr tree if the logical flow has is_chassis_resident() match.
+ * (3) expr matches if (1) and (2) are false.
  */
 struct lflow_cache {
 struct hmap_node node;
 struct uuid lflow_uuid; /* key */
 uint32_t conj_id_ofs;
+
+enum lflow_cache_type type;
+union {
+struct {
+struct hmap *expr_matches;
+size_t n_conjs;
+};
+struct expr *expr;
+};
 };
 
 static struct lflow_cache *
@@ -286,6 +306,7 @@ lflow_cache_add(struct hmap *lflow_cache_map,
 struct lflow_cache *lc = xmalloc(sizeof *lc);
 lc->lflow_uuid = lflow->header_.uuid;
 lc->conj_id_ofs = 0;
+lc->type = LCACHE_T_NO_CACHE;
 hmap_insert(lflow_cache_map, >node, uuid_hash(>lflow_uuid));
 return lc;
 }
@@ -312,6 +333,12 @@ lflow_cache_delete(struct hmap *lflow_cache_map,
 struct lflow_cache *lc = lflow_cache_get(lflow_cache_map, lflow);
 if (lc) {
 hmap_remove(lflow_cache_map, >node);
+if (lc->type == LCACHE_T_MATCHES) {
+expr_matches_destroy(lc->expr_matches);
+free(lc->expr_matches);
+} else if (lc->type == LCACHE_T_EXPR) {
+expr_destroy(lc->expr);
+}
 free(lc);
 }
 }
@@ -327,6 +354,12 @@ lflow_cache_destroy(struct hmap *lflow_cache_map)
 {
 struct lflow_cache *lc;
 HMAP_FOR_EACH_POP (lc, node, lflow_cache_map) {
+if (lc->type == LCACHE_T_MATCHES) {
+expr_matches_destroy(lc->expr_matches);
+free(lc->expr_matches);
+} else if (lc->type == LCACHE_T_EXPR) {
+expr_destroy(lc->expr);
+}
 free(lc);
 }
 
@@ -567,6 +600,159 @@ update_conj_id_ofs(uint32_t *conj_id_ofs, uint32_t 
n_conjs)
 return true;
 }
 
+static void
+add_matches_to_flow_table(const struct sbrec_logical_flow *lflow,
+  struct hmap *matches, size_t conj_id_ofs,
+  uint8_t ptable, uint8_t output_ptable,
+  struct ofpbuf *ovnacts,
+  bool ingress, struct lflow_ctx_in *l_ctx_in,
+  struct lflow_ctx_out *l_ctx_out)
+{
+struct lookup_port_aux aux = {
+.sbrec_multicast_group_by_name_datapath
+= l_ctx_in->sbrec_multicast_group_by_name_datapath,
+.sbrec_port_binding_by_name = l_ctx_in->sbrec_port_binding_by_name,
+.dp = lflow->logical_datapath
+};
+
+/* Encode OVN logical actions into OpenFlow. */
+uint64_t ofpacts_stub[1024 / 8];
+struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
+struct ovnact_encode_params ep = {
+.lookup_port = lookup_port_cb,
+.tunnel_ofport = tunnel_ofport_cb,
+.aux = ,
+.is_switch = datapath_is_switch(lflow->logical_datapath),
+.group_table = l_ctx_out->group_table,
+.meter_table = l_ctx_out->meter_table,
+.lflow_uuid = lflow->header_.uuid,
+
+.pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS,
+.ingress_ptable = OFTABLE_LOG_INGRESS_PIPELINE,
+.egress_ptable = OFTABLE_LOG_EGRESS_PIPELINE,
+.output_ptable = output_ptable,
+.mac_bind_ptable = OFTABLE_MAC_BINDING,
+.mac_lookup_ptable = OFTABLE_MAC_LOOKUP,
+};

[ovs-dev] [PATCH ovn v2 1/4] I-P engine: Provide the option to store client data in engine ctx.

2020-09-09 Thread numans 
From: Numan Siddique 

There can be some client specific data which could change from one engine run
to another. Adding a 'void *' data in 'struct engine_ctx' will be useful.
One such usecase is to provide a config option in ovn-controller to turn on or
off logical flow expr caching. And this config knob can be stored in the 
engine_ctx.
An upcoming patch will make use of this.

Signed-off-by: Numan Siddique 
---
 lib/inc-proc-eng.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h
index e25bcb29c6..80de75029e 100644
--- a/lib/inc-proc-eng.h
+++ b/lib/inc-proc-eng.h
@@ -66,6 +66,7 @@
 struct engine_context {
 struct ovsdb_idl_txn *ovs_idl_txn;
 struct ovsdb_idl_txn *ovnsb_idl_txn;
+void *client_ctx;
 };
 
 /* Arguments to be passed to the engine at engine_init(). */
-- 
2.26.2

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

[ovs-dev] [PATCH ovn v2 2/4] ovn-controller: Persist the conjunction ids allocated for conjuctive matches.

2020-09-09 Thread numans 
From: Numan Siddique 

For a logical flow which results in conjuctive OF matches, we are not persisting
the allocated conjunction ids for it. There are few side effects because of 
this.
  - When a port group or address set gets modified, the logical flows which 
references
these port groups/address sets gets reprocessed again and the resulting 
OpenvSwitch flows
with conjunctive matches gets modified in the vswitchd if the conjunction 
id changes.

  - And because of this there is small probability of a packet getting dropped 
when the
OF flows gets updated with different conjunction ids.

This patch fixes this issue by persisting the conjunction ids. Earlier, logical 
flow caching
support was added [1] to ovn-controller and then reverted [2] later due to some 
issues.

This patch takes the lflow caching approach to persist the conjunction ids. But 
it only
creates the cache for logical flows which result in conjunctive matches. And it 
doesn't
cache the expr tree.

The lflow caching is made configurable and enabled by default. Any user can 
disable caching
by setting 'ovn-enable-lflow-cache' to 'false' in the OVS db.
 - ovs-vsctl set open . external_ids:ovn-enable-lflow-cache=false

Note: Changing the option 'ovn-enable-lflow-cache' doesn't result in full 
recompute of
I-P engine. But ovn-controller updates the chassis.other_config column. And 
when it
receives the update2/update3 message, this results in full recompute (as any 
chassis
changes results in full recompute). This is the case with all the config 
options in OVS db.

An upcoming patch series, will attempt to add back the expr caching (addressing 
all the issues.)

[1] - 8795bec737b("ovn-controller: Cache logical flow expr tree for each lflow.)
[2] - 065bcf46218("ovn-controller: Revert lflow expr caching")

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1858878
Signed-off-by: Numan Siddique 
---
 controller/chassis.c|  22 +-
 controller/lflow.c  | 118 ++-
 controller/lflow.h  |   8 ++-
 controller/ovn-controller.c |  97 +++---
 tests/ovn.at| 135 
 5 files changed, 363 insertions(+), 17 deletions(-)

diff --git a/controller/chassis.c b/controller/chassis.c
index 45e018e385..8e6ad2d459 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -86,6 +86,7 @@ struct ovs_chassis_cfg {
 const char *cms_options;
 const char *monitor_all;
 const char *chassis_macs;
+const char *enable_lflow_cache;
 
 /* Set of encap types parsed from the 'ovn-encap-type' external-id. */
 struct sset encap_type_set;
@@ -165,6 +166,12 @@ get_monitor_all(const struct smap *ext_ids)
 return smap_get_def(ext_ids, "ovn-monitor-all", "false");
 }
 
+static const char *
+get_enable_lflow_cache(const struct smap *ext_ids)
+{
+return smap_get_def(ext_ids, "ovn-enable-lflow-cache", "true");
+}
+
 static const char *
 get_encap_csum(const struct smap *ext_ids)
 {
@@ -286,6 +293,7 @@ chassis_parse_ovs_config(const struct 
ovsrec_open_vswitch_table *ovs_table,
 ovs_cfg->cms_options = get_cms_options(>external_ids);
 ovs_cfg->monitor_all = get_monitor_all(>external_ids);
 ovs_cfg->chassis_macs = get_chassis_mac_mappings(>external_ids);
+ovs_cfg->enable_lflow_cache = get_enable_lflow_cache(>external_ids);
 
 if (!chassis_parse_ovs_encap_type(encap_type, _cfg->encap_type_set)) {
 return false;
@@ -312,12 +320,14 @@ static void
 chassis_build_other_config(struct smap *config, const char *bridge_mappings,
const char *datapath_type, const char *cms_options,
const char *monitor_all, const char *chassis_macs,
-   const char *iface_types, bool is_interconn)
+   const char *iface_types,
+   const char *enable_lflow_cache, bool is_interconn)
 {
 smap_replace(config, "ovn-bridge-mappings", bridge_mappings);
 smap_replace(config, "datapath-type", datapath_type);
 smap_replace(config, "ovn-cms-options", cms_options);
 smap_replace(config, "ovn-monitor-all", monitor_all);
+smap_replace(config, "ovn-enable-lflow-cache", enable_lflow_cache);
 smap_replace(config, "iface-types", iface_types);
 smap_replace(config, "ovn-chassis-mac-mappings", chassis_macs);
 smap_replace(config, "is-interconn", is_interconn ? "true" : "false");
@@ -332,6 +342,7 @@ chassis_other_config_changed(const char *bridge_mappings,
  const char *cms_options,
  const char *monitor_all,
  const char *chassis_macs,
+ const char *enable_lflow_cache,
  const struct ds *iface_types,
  bool is_interconn,
  const struct sbrec_chassis *chassis_rec)
@@ -364,6 +375,13 @@

[ovs-dev] [PATCH ovn v2 0/4] Another attempt on lflow expr caching.

2020-09-09 Thread numans 
From: Numan Siddique 

This patch series adds caching of logical flow expr matches/expr tree
so that whenever a logical flow is processed, we can reuse the cached
expr tree of expr matches.

The expr matches/tree is not cached for a logical flow referencing
port group/address sets as it is tricky to handle any port group changes
if cached.

The patch 2 of this series was submitted separately earlier. Patch 2
uses the lflow caching approach to cache the conjunction ids so that
these ids are persistent across recomputes.


v1 -> v2

 * Resolved conflicts in patch 2.
 * Addressed review comments from Mark in patch 3 in a different way
   than suggested. expr_evaluate_condition() should be called before
   expr_normalize().
 * In patch 4 - Changed the expr caching to cache simplified expr tree
   for lflows having condition - is_chassis_resident(). The cached
   expr tree is evaluated and normalized when processing such lflows
   later.

Numan Siddique (4):
  I-P engine: Provide the option to store client data in engine ctx.
  ovn-controller: Persist the conjunction ids allocated for conjuctive
matches.
  expr: Evaluate the condition expression in a separate step.
  ovn-controller: Cache logical flow expr matches.

 controller/chassis.c|  22 +-
 controller/lflow.c  | 520 +++-
 controller/lflow.h  |   8 +-
 controller/ovn-controller.c |  97 ++-
 include/ovn/expr.h  |  10 +-
 lib/expr.c  |  57 +++-
 lib/inc-proc-eng.h  |   1 +
 tests/ovn.at| 135 ++
 tests/test-ovn.c|  12 +-
 utilities/ovn-trace.c   |   6 +-
 10 files changed, 709 insertions(+), 159 deletions(-)

-- 
2.26.2

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

[ovs-dev] [PATCH ovn] Remove duplicate include file

2020-09-09 Thread Yi Li 
Signed-off-by: Yi Li 
---
 controller/bfd.c | 1 -
 controller/pinctrl.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/controller/bfd.c b/controller/bfd.c
index b305eb15..cf011e38 100644
--- a/controller/bfd.c
+++ b/controller/bfd.c
@@ -17,7 +17,6 @@
 #include "bfd.h"
 #include "encaps.h"
 #include "lport.h"
-#include "ovn-controller.h"
 
 #include "lib/hash.h"
 #include "lib/sset.h"
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index bb7bb41b..0a702053 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -27,7 +27,6 @@
 #include "ha-chassis.h"
 #include "lport.h"
 #include "nx-match.h"
-#include "ovn-controller.h"
 #include "latch.h"
 #include "lib/packets.h"
 #include "lib/sset.h"
-- 
2.25.3



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

Re: [ovs-dev] [PATCH] Remove duplicate include file

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Yi Li, 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.


git-am:
error: sha1 information is lacking or useless (controller/bfd.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 Remove duplicate include file
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


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] Remove duplicate include file

2020-09-09 Thread Yi Li 
Signed-off-by: Yi Li 
---
 controller/bfd.c | 1 -
 controller/pinctrl.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/controller/bfd.c b/controller/bfd.c
index b305eb15..cf011e38 100644
--- a/controller/bfd.c
+++ b/controller/bfd.c
@@ -17,7 +17,6 @@
 #include "bfd.h"
 #include "encaps.h"
 #include "lport.h"
-#include "ovn-controller.h"
 
 #include "lib/hash.h"
 #include "lib/sset.h"
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index bb7bb41b..0a702053 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -27,7 +27,6 @@
 #include "ha-chassis.h"
 #include "lport.h"
 #include "nx-match.h"
-#include "ovn-controller.h"
 #include "latch.h"
 #include "lib/packets.h"
 #include "lib/sset.h"
-- 
2.25.3



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

[ovs-dev] [PATCH ovn] Replace ds_put_* evaluating to constant expressions

2020-09-09 Thread anton . ivanov 
From: Anton Ivanov 

Replaces ds_put_cstr and ds_put_format which evaluate to
constant expressions with string constants

Signed-off-by: Anton Ivanov 
---
 northd/ovn-northd.c | 56 ++---
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 3de71612b..c681d5cbf 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -1928,6 +1928,8 @@ update_dynamic_addresses(struct dynamic_address_update 
*update)
 
 struct ds new_addr = DS_EMPTY_INITIALIZER;
 ds_put_format(_addr, ETH_ADDR_FMT, ETH_ADDR_ARGS(mac));
+
+
 ipam_insert_mac(, true);
 
 if (ip4) {
@@ -6950,24 +6952,24 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
 struct ds action = DS_EMPTY_INITIALIZER;
 
 ds_clear();
-ds_put_cstr(, "udp.dst == 53");
+const char *dns_match = "udp.dst == 53";
 ds_put_format(,
   REGBIT_DNS_LOOKUP_RESULT" = dns_lookup(); next;");
 ovn_lflow_add(lflows, od, S_SWITCH_IN_DNS_LOOKUP, 100,
-  ds_cstr(), ds_cstr());
+  dns_match, ds_cstr());
 ds_clear();
-ds_put_cstr(, " && "REGBIT_DNS_LOOKUP_RESULT);
-ds_put_format(, "eth.dst <-> eth.src; ip4.src <-> ip4.dst; "
+
+const char *dns_match2 = "udp.dst == 53 && "REGBIT_DNS_LOOKUP_RESULT;
+const char *dns_action = "eth.dst <-> eth.src; ip4.src <-> ip4.dst; "
   "udp.dst = udp.src; udp.src = 53; outport = inport; "
-  "flags.loopback = 1; output;");
+  "flags.loopback = 1; output;";
 ovn_lflow_add(lflows, od, S_SWITCH_IN_DNS_RESPONSE, 100,
-  ds_cstr(), ds_cstr());
-ds_clear();
-ds_put_format(, "eth.dst <-> eth.src; ip6.src <-> ip6.dst; "
+  dns_match2, dns_action);
+dns_action = "eth.dst <-> eth.src; ip6.src <-> ip6.dst; "
   "udp.dst = udp.src; udp.src = 53; outport = inport; "
-  "flags.loopback = 1; output;");
+  "flags.loopback = 1; output;";
 ovn_lflow_add(lflows, od, S_SWITCH_IN_DNS_RESPONSE, 100,
-  ds_cstr(), ds_cstr());
+  dns_match2, dns_action);
 ds_destroy();
 }
 
@@ -7839,10 +7841,10 @@ add_ecmp_symmetric_reply_flows(struct hmap *lflows,
 _route->header_);
 
 ds_clear();
-ds_put_cstr(, "eth.dst = ct_label.ecmp_reply_eth; next;");
+const char *action = "eth.dst = ct_label.ecmp_reply_eth; next;";
 ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_ARP_RESOLVE,
 200, ds_cstr(_reply),
-ds_cstr(), _route->header_);
+action, _route->header_);
 }
 
 static void
@@ -8734,9 +8736,9 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 
 /* TTL discard */
 ds_clear();
-ds_put_cstr(, "ip4 && ip.ttl == {0, 1}");
+const char *ttl_match = "ip4 && ip.ttl == {0, 1}";
 ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_INPUT, 30,
-  ds_cstr(), "drop;");
+  ttl_match, "drop;");
 
 /* Pass other traffic not already handled to the next table for
  * routing. */
@@ -8778,14 +8780,13 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 ds_put_cstr(, " && icmp4.type == 8 && icmp4.code == 0");
 
 ds_clear();
-ds_put_format(,
-"ip4.dst <-> ip4.src; "
-"ip.ttl = 255; "
-"icmp4.type = 0; "
-"flags.loopback = 1; "
-"next; ");
+const char * icmp_actions = "ip4.dst <-> ip4.src; "
+"ip.ttl = 255; "
+"icmp4.type = 0; "
+"flags.loopback = 1; "
+"next; ";
 ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
-ds_cstr(), ds_cstr(),
+ds_cstr(), icmp_actions,
 >nbrp->header_);
 }
 
@@ -9184,14 +9185,14 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
*ports,
 ds_put_cstr(, " && icmp6.type == 128 && icmp6.code == 0");
 
 ds_clear();
-ds_put_cstr(,
+const char *lrp_actions =
 "ip6.dst <-> ip6.src; "
 "ip.ttl = 255; "
 "icmp6.type = 129; "
 "flags.loopback = 1; "
-"next; ");
+"next; ";
 ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
-ds_cstr(), ds_cstr(),
+

Re: [ovs-dev] [PATCH ovn v2] ovn-ctl: Handle cluster db upgrades for run_(nb/sb)_ovsdb

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings Numan Siddique, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 91 characters long (recommended limit is 79)
#32 FILE: utilities/ovn-ctl:293:
if test X$detach = Xno && test $mode = cluster && test -z 
"$cluster_remote_addr" ; then

Lines checked: 61, Warnings: 1, Errors: 0


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

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

Re: [ovs-dev] [PATCH v1] NEWS: Add external ip based NAT support

2020-09-09 Thread Numan Siddique 
On Wed, Sep 9, 2020 at 1:28 AM Ankur Sharma  wrote:
>
> Signed-off-by: Ankur Sharma 

Thanks. I applied the patch to master.

I think you forgot to add the "ovn" tag in the subject prefix :). Now
that we have a separate patchwork for OVN,
please add this tag for future patches.

Thanks
Numan

> ---
>  NEWS | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/NEWS b/NEWS
> index a1ce4e8..8e2869b 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -11,6 +11,8 @@ Post-v20.06.0
>   called Chassis_Private now contains the nb_cfg column which is updated
>   by incrementing the value in the NB_Global table, CMSes relying on
>   this mechanism should update their code to use this new table.
> +   - Added support for external ip based NAT. Now, besides the logical ip
> + external ips will also decide if a packet will be NATed or not.
>
>  OVN v20.06.0
>  --
> --
> 1.8.3.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] ovn-ctl: Handle cluster db upgrades for run_(nb/sb)_ovsdb

2020-09-09 Thread Numan Siddique 
On Wed, Sep 9, 2020 at 2:31 AM Mark Michelson  wrote:
>
> This needs a rebase because commit
> 413cf9864024c4ef253ea177435161af198d5784 changed the way that the ovsdb
> server is started.

Thanks for the review. I submitted v2 -
https://patchwork.ozlabs.org/project/ovn/patch/20200909071939.5095-1-num...@ovn.org/

Numan

>
>
> On 9/3/20 9:04 AM, num...@ovn.org wrote:
> > From: Numan Siddique 
> >
> > when ovn-ctl run_(nb_sb)_ovsdb is called, the ovsdb-server is started 
> > without
> > passing --detach and --monoitor and the process is exec'd.
> >
> > For cluster mode, upgrade_cluster is never called and hence the dbs are not 
> > upraded
> > to new schema. CMS has to handle the db upgrade separately.
> >
> > This patch handles the db upgrade by starting ovsdb-server in background 
> > and then
> > waits on ovsdb-server to complete.
> >
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1868392
> > Signed-off-by: Numan Siddique 
> > ---
> >   utilities/ovn-ctl | 18 +-
> >   1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl
> > index 8afe68a0ad..dee58d9a36 100755
> > --- a/utilities/ovn-ctl
> > +++ b/utilities/ovn-ctl
> > @@ -288,7 +288,19 @@ $cluster_remote_port
> >   set "$@" --sync-from=`cat $active_conf_file`
> >   fi
> >
> > -"$@" "$file"
> > +local run_ovsdb_in_bg="no"
> > +local process_id=
> > +if test X$detach = Xno && test $mode = cluster && test -z 
> > "$cluster_remote_addr" ; then
> > +# When detach is no (for run_nb_ovsdb/run_sb_ovsdb commands)
> > +# we want to run ovsdb-server in background rather than running it 
> > in foreground
> > +# so that the OVN dbs are upgraded for the cluster mode.
> > +# Otherwise, CMS has to take the responsibility of upgrading the 
> > dbs.
> > +run_ovsdb_in_bg="yes"
> > +"$@" $file &
> > +process_id=$!
> > +else
> > +"$@" $file
> > +fi
> >
> >   # Initialize the database if it's NOT joining a cluster.
> >   if test -z "$cluster_remote_addr"; then
> > @@ -298,6 +310,10 @@ $cluster_remote_port
> >   if test $mode = cluster; then
> >   upgrade_cluster "$schema" "unix:$sock"
> >   fi
> > +
> > +if test $run_ovsdb_in_bg = yes; then
> > +wait $process_id
> > +fi
> >   }
> >
> >   start_nb_ovsdb() {
> >
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH ovn v2] ovn-ctl: Handle cluster db upgrades for run_(nb/sb)_ovsdb

2020-09-09 Thread numans 
From: Numan Siddique 

when ovn-ctl run_(nb_sb)_ovsdb is called, the ovsdb-server is started without
passing --detach and --monoitor and the process is exec'd.

For cluster mode, upgrade_cluster is never called and hence the dbs are not 
upraded
to new schema. CMS has to handle the db upgrade separately.

This patch handles the db upgrade by starting ovsdb-server in background and 
then
waits on ovsdb-server to complete.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1868392
Signed-off-by: Numan Siddique 
---
 v1 -> v2
 -
  * Rebased to resovle the conflicts.

 utilities/ovn-ctl | 20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/utilities/ovn-ctl b/utilities/ovn-ctl
index af095ea1bd..c44201ccfb 100755
--- a/utilities/ovn-ctl
+++ b/utilities/ovn-ctl
@@ -288,7 +288,21 @@ $cluster_remote_port
 set "$@" --sync-from=`cat $active_conf_file`
 fi
 
-start_wrapped_daemon "$wrapper" ovsdb-$db "" "$@" "$file"
+local run_ovsdb_in_bg="no"
+local process_id=
+if test X$detach = Xno && test $mode = cluster && test -z 
"$cluster_remote_addr" ; then
+# When detach is no (for run_nb_ovsdb/run_sb_ovsdb commands)
+# we want to run ovsdb-server in background rather than running it in
+# foreground so that the OVN dbs are upgraded for the cluster mode.
+# Otherwise, CMS has to take the responsibility of upgrading the dbs.
+# Note: We run only the ovsdb-server in backgroud which created the
+# cluster (i.e cluster_remote_addr is not set.).
+run_ovsdb_in_bg="yes"
+"$@" $file &
+process_id=$!
+else
+start_wrapped_daemon "$wrapper" ovsdb-$db "" "$@" "$file"
+fi
 
 # Initialize the database if it's NOT joining a cluster.
 if test -z "$cluster_remote_addr"; then
@@ -298,6 +312,10 @@ $cluster_remote_port
 if test $mode = cluster; then
 upgrade_cluster "$schema" "unix:$sock"
 fi
+
+if test $run_ovsdb_in_bg = yes; then
+wait $process_id
+fi
 }
 
 start_nb_ovsdb() {
-- 
2.26.2

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

Re: [ovs-dev] [PATCH v2] userspace: fix bad UDP performance issue of veth

2020-09-09 Thread 0-day Robot 
Bleep bloop.  Greetings yang_y_yi, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 80 characters long (recommended limit is 79)
#204 FILE: Documentation/howto/userspace-udp-performance-tunning.rst:48:
iperf3: OUT OF ORDER - incoming packet = 14 and received packet = 123 AND SP = 5

WARNING: Line is 80 characters long (recommended limit is 79)
#205 FILE: Documentation/howto/userspace-udp-performance-tunning.rst:49:
iperf3: OUT OF ORDER - incoming packet = 15 and received packet = 125 AND SP = 5

WARNING: Line is 80 characters long (recommended limit is 79)
#206 FILE: Documentation/howto/userspace-udp-performance-tunning.rst:50:
iperf3: OUT OF ORDER - incoming packet = 78 and received packet = 137 AND SP = 5

WARNING: Line is 80 characters long (recommended limit is 79)
#207 FILE: Documentation/howto/userspace-udp-performance-tunning.rst:51:
iperf3: OUT OF ORDER - incoming packet = 79 and received packet = 137 AND SP = 5

WARNING: Line is 80 characters long (recommended limit is 79)
#208 FILE: Documentation/howto/userspace-udp-performance-tunning.rst:52:
iperf3: OUT OF ORDER - incoming packet = 80 and received packet = 139 AND SP = 5

WARNING: Line is 80 characters long (recommended limit is 79)
#209 FILE: Documentation/howto/userspace-udp-performance-tunning.rst:53:
iperf3: OUT OF ORDER - incoming packet = 82 and received packet = 172 AND SP = 5

WARNING: Line is 80 characters long (recommended limit is 79)
#210 FILE: Documentation/howto/userspace-udp-performance-tunning.rst:54:
iperf3: OUT OF ORDER - incoming packet = 83 and received packet = 173 AND SP = 5

WARNING: Line is 86 characters long (recommended limit is 79)
#223 FILE: Documentation/howto/userspace-udp-performance-tunning.rst:67:
$ sudo ip netns exec ns03 iperf3 -t 10 -i 1 -u -b 10G -c 10.15.2.3 
--get-server-output

WARNING: Line is 84 characters long (recommended limit is 79)
#238 FILE: Documentation/howto/userspace-udp-performance-tunning.rst:82:
[ ID] Interval   Transfer Bandwidth   JitterLost/Total 
Datagrams

WARNING: Line is 84 characters long (recommended limit is 79)
#246 FILE: Documentation/howto/userspace-udp-performance-tunning.rst:90:
[ ID] Interval   Transfer Bandwidth   JitterLost/Total 
Datagrams

WARNING: Line is 85 characters long (recommended limit is 79)
#278 FILE: Documentation/howto/userspace-udp-performance-tunning.rst:122:
$ sudo ip netns exec ns03 iperf3 -t 10 -i 1 -u -b 3G -c 10.15.2.3 
--get-server-output

WARNING: Line is 84 characters long (recommended limit is 79)
#293 FILE: Documentation/howto/userspace-udp-performance-tunning.rst:137:
[ ID] Interval   Transfer Bandwidth   JitterLost/Total 
Datagrams

WARNING: Line is 84 characters long (recommended limit is 79)
#301 FILE: Documentation/howto/userspace-udp-performance-tunning.rst:145:
[ ID] Interval   Transfer Bandwidth   JitterLost/Total 
Datagrams

WARNING: Line is 90 characters long (recommended limit is 79)
#350 FILE: Documentation/howto/userspace-udp-performance-tunning.rst:194:
   $ sudo ovs-vsctl set Open_vSwitch . 
other_config:userspace-sock-buf-size=1073741823

WARNING: Comment with 'xxx' marker
#506 FILE: lib/userspace-sock-buf-size.c:31:
 * other_config:userspace_sock_buf_size = 

Lines checked: 602, Warnings: 15, Errors: 0


build:
reading sources... [ 87%] topics/language-bindings
reading sources... [ 88%] topics/networking-namespaces
reading sources... [ 89%] topics/openflow
reading sources... [ 90%] topics/ovs-extensions
reading sources... [ 91%] topics/ovsdb-replication
reading sources... [ 92%] topics/porting
reading sources... [ 93%] topics/testing
reading sources... [ 93%] topics/tracing
reading sources... [ 94%] topics/userspace-tso
reading sources... [ 95%] topics/windows
reading sources... [ 96%] tutorials/faucet
reading sources... [ 97%] tutorials/index
reading sources... [ 98%] tutorials/ipsec
reading sources... [ 99%] tutorials/ovs-advanced
reading sources... [100%] tutorials/ovs-conntrack

deprecation warning: io.FileInput() argument `handle_io_errors` is ignored 
since "Docutils 0.10 (2012-12-16)" and will soon be removed.deprecation 
warning: io.FileInput() argument `handle_io_errors` is ignored since "Docutils 
0.10 (2012-12-16)" and will soon be removed.deprecation warning: io.FileInput() 
argument `handle_io_errors` is ignored since "Docutils 0.10 (2012-12-16)" and 
will soon be removed.deprecation warning: io.FileInput() argument 
`handle_io_errors` is ignored since "Docutils 0.10 (2012-12-16)" and will soon 
be removed.deprecation warning: io.FileInput() argument `handle_io_errors` is 
ignored since "Docutils 0.10 (2012-12-16)" and will soon be removed.deprecation 
warning: io.FileInput() argument `handle_io_errors` is ignored since "Docutils 
0.10 (2012-12-16)" and will soon be removed.deprecation warning:

[ovs-dev] [PATCH v2] userspace: fix bad UDP performance issue of veth

2020-09-09 Thread yang_y_yi 
From: Yi Yang 

iperf3 UDP performance of veth to veth case is
very very bad because of too many packet loss,
the root cause is rmem_default and wmem_default
are just 212992, but iperf3 UDP test used 8K
UDP size which resulted in many UDP fragment in
case that MTU size is 1500, one 8K UDP send would
enqueue 6 UDP fragments to socket receive queue,
the default small socket buffer size can't cache
so many packets that many packets are lost.

This commit fixed packet loss issue, it allows
users to set socket receive and send buffer size
per their own system environment to proper value,
therefore there will not be packet loss.

Users can set system interface socket buffer size
by command lines:

  $ sudo sh -c "1073741823 > /proc/sys/net/core/wmem_max"
  $ sudo sh -c "1073741823 > /proc/sys/net/core/rmem_max"

or

  $ sudo ovs-vsctl set Open_vSwitch . \
other_config:userspace-sock-buf-size=1073741823

But final socket buffer size is minimum one among of them.
Possible value range is 212992 to 1073741823. Current
default value for other_config:userspace-sock-buf-size is
212992, users need to increase it to improve UDP
performance, the changed value will take effect after
restarting ovs-vswitchd. More details about it is in the
document
Documentation/howto/userspace-udp-performance-tunning.rst.

By the way, big socket buffer doesn't mean it will
allocate big buffer on creating socket, actually
it won't alocate any extra buffer compared to default
socket buffer size, it just means more skbuffs can
be enqueued to socket receive queue and send queue,
therefore there will not be packet loss.

The below is for your reference.

The result before apply this commit
===
$ ip netns exec ns02 iperf3 -t 5 -i 1 -u -b 100M -c 10.15.2.6 
--get-server-output -A 5
Connecting to host 10.15.2.6, port 5201
[  4] local 10.15.2.2 port 59053 connected to 10.15.2.6 port 5201
[ ID] Interval   Transfer Bandwidth   Total Datagrams
[  4]   0.00-1.00   sec  10.8 MBytes  90.3 Mbits/sec  1378
[  4]   1.00-2.00   sec  11.9 MBytes   100 Mbits/sec  1526
[  4]   2.00-3.00   sec  11.9 MBytes   100 Mbits/sec  1526
[  4]   3.00-4.00   sec  11.9 MBytes   100 Mbits/sec  1526
[  4]   4.00-5.00   sec  11.9 MBytes   100 Mbits/sec  1526
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval   Transfer Bandwidth   JitterLost/Total 
Datagrams
[  4]   0.00-5.00   sec  58.5 MBytes  98.1 Mbits/sec  0.047 ms  357/531 (67%)
[  4] Sent 531 datagrams

Server output:
---
Accepted connection from 10.15.2.2, port 60314
[  5] local 10.15.2.6 port 5201 connected to 10.15.2.2 port 59053
[ ID] Interval   Transfer Bandwidth   JitterLost/Total 
Datagrams
[  5]   0.00-1.00   sec  1.36 MBytes  11.4 Mbits/sec  0.047 ms  357/531 (67%)
[  5]   1.00-2.00   sec  0.00 Bytes  0.00 bits/sec  0.047 ms  0/0 (-nan%)
[  5]   2.00-3.00   sec  0.00 Bytes  0.00 bits/sec  0.047 ms  0/0 (-nan%)
[  5]   3.00-4.00   sec  0.00 Bytes  0.00 bits/sec  0.047 ms  0/0 (-nan%)
[  5]   4.00-5.00   sec  0.00 Bytes  0.00 bits/sec  0.047 ms  0/0 (-nan%)

iperf Done.

The result after apply this commit
===
$ sudo ip netns exec ns02 iperf3 -t 5 -i 1 -u -b 4G -c 10.15.2.6 
--get-server-output -A 5
Connecting to host 10.15.2.6, port 5201
[  4] local 10.15.2.2 port 48547 connected to 10.15.2.6 port 5201
[ ID] Interval   Transfer Bandwidth   Total Datagrams
[  4]   0.00-1.00   sec   440 MBytes  3.69 Gbits/sec  56276
[  4]   1.00-2.00   sec   481 MBytes  4.04 Gbits/sec  61579
[  4]   2.00-3.00   sec   474 MBytes  3.98 Gbits/sec  60678
[  4]   3.00-4.00   sec   480 MBytes  4.03 Gbits/sec  61452
[  4]   4.00-5.00   sec   480 MBytes  4.03 Gbits/sec  61441
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval   Transfer Bandwidth   JitterLost/Total 
Datagrams
[  4]   0.00-5.00   sec  2.30 GBytes  3.95 Gbits/sec  0.024 ms  0/301426 (0%)
[  4] Sent 301426 datagrams

Server output:
---
Accepted connection from 10.15.2.2, port 60320
[  5] local 10.15.2.6 port 5201 connected to 10.15.2.2 port 48547
[ ID] Interval   Transfer Bandwidth   JitterLost/Total 
Datagrams
[  5]   0.00-1.00   sec   209 MBytes  1.75 Gbits/sec  0.021 ms  0/26704 (0%)
[  5]   1.00-2.00   sec   258 MBytes  2.16 Gbits/sec  0.025 ms  0/32967 (0%)
[  5]   2.00-3.00   sec   258 MBytes  2.16 Gbits/sec  0.022 ms  0/32987 (0%)
[  5]   3.00-4.00   sec   257 MBytes  2.16 Gbits/sec  0.023 ms  0/32954 (0%)
[  5]   4.00-5.00   sec   257 MBytes  2.16 Gbits/sec  0.021 ms  0/32937 (0%)
[  5]   5.00-6.00   sec   255 MBytes  2.14 Gbits/sec  0.026 ms  0/32685 (0%)
[  5]   6.00-7.00   sec   254 MBytes  2.13 Gbits/sec  0.025 ms  0/32453 (0%)
[  5]   7.00-8.00   sec   255 MBytes  2.14 Gbits/sec  0.026 ms  0/32679 (0%)
[  5]   8.00-9.00   sec   255 MBytes  2.14