Re: [ovs-dev] [PATCH 1/1] netdev-offload-dpdk: Support vxlan encap offload with load actions

[Sriharsha Basavapatna via dev]

On Sun, Sep 6, 2020 at 5:51 PM Eli Britstein  wrote:
>
> ping
>
> On 7/30/2020 1:58 PM, Eli Britstein wrote:
> > From: Lei Wang 
> >
> > Struct match has the tunnel values/masks in
> > match->flow.tunnel/match->wc.masks.tunnel.
> > Load actions such as load:0xa566c10->NXM_NX_TUN_IPV4_DST[],
> > load:0xbba->NXM_NX_TUN_ID[] are utilizing the tunnel masks fields,
> > but those should not be used for matching.
> > Offloading fails if masks is not clear. Clear it if no tunnel used.
> >
> > Signed-off-by: Lei Wang 
> > Reviewed-by: Eli Britstein 
> > Reviewed-by: Gaetan Rivet 
> > Signed-off-by: Eli Britstein 
> > ---
> >   lib/netdev-offload-dpdk.c | 4 
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> > index de6101e4d..0d23e4879 100644
> > --- a/lib/netdev-offload-dpdk.c
> > +++ b/lib/netdev-offload-dpdk.c
> > @@ -682,6 +682,10 @@ parse_flow_match(struct flow_patterns *patterns,
> >
> >   consumed_masks = &match->wc.masks;
> >
> > +if (!flow_tnl_dst_is_set(&match->flow.tunnel)) {
> > +memset(&match->wc.masks.tunnel, 0, sizeof match->wc.masks.tunnel);
> > +}
> > +
This fix looks good to me.  Did you take a look to see if this can be
fixed in the code that generates these invalid masks in the first
place ?

Thanks,
-Harsha
> >   memset(&consumed_masks->in_port, 0, sizeof consumed_masks->in_port);
> >   /* recirc id must be zero. */
> >   if (match->wc.masks.recirc_id & match->flow.recirc_id) {
> ___
> 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] Von Bintou

[Bintou Deme via dev]

Von: Bintou Deme
Liebste,
Guten Tag und vielen Dank für Ihre Aufmerksamkeit. Bitte ich möchte, dass Sie 
meine E-Mail sorgfältig lesen und mir bei der Bearbeitung dieses Projekts 
behilflich sind. Ich bin Miss Bintou Deme und ich schreibe demütig, um Ihre 
Partnerschaft und Unterstützung bei der Übertragung und Investition meiner 
Erbschaftsfonds in Höhe von 6.500.000,00 USD (sechs Millionen 
fünfhunderttausend US-Dollar) zu erbitten, die mein verstorbener geliebter 
Vater vor seinem Tod bei einer Bank hinterlegt hat.

Ich möchte Ihnen versichern, dass dieser Fonds von meinem verstorbenen Vater 
legal erworben wurde und keinen kriminellen Hintergrund hat. Mein Vater hat 
diesen Fonds legal durch ein legitimes Geschäft erworben, bevor er während 
seiner Geschäftsreise zu Tode vergiftet wurde. Der Tod meines Vaters wurde 
vermutlich von seinen Verwandten geplant, die während dieser Zeit seiner 
Geschäftsreise mit ihm reisten. Denn nach 3 Monaten des Todes meines Vaters 
begannen seine Verwandten, alle Grundstücke meines verstorbenen Vaters zu 
beanspruchen und zu verkaufen.

Die Verwandten meines verstorbenen Vaters wissen nichts von den US $ 
6.500.000,00 (sechs Millionen fünfhunderttausend US-Dollar), die mein 
verstorbener Vater bei der Bank hinterlegt hat, und mein verstorbener Vater hat 
mir vor seinem Tod heimlich gesagt, dass ich in einem Land einen ausländischen 
Partner suchen soll meiner Wahl, wo ich dieses Geld für meine eigenen Zwecke 
überweisen werde.

Bitte helfen Sie mir, dieses Geld für geschäftliche Zwecke in Ihrem Land auf 
Ihr Konto zu überweisen. Ich habe diese Entscheidung getroffen, weil ich viele 
Demütigungen von den Verwandten meines verstorbenen Vaters erlitten habe. 
Gegenwärtig hatte ich Kommunikation mit dem Direktor der Bank, bei der mein 
verstorbener Vater dieses Geld eingezahlt hat. Ich habe dem Direktor der Bank 
erklärt, wie dringend es ist, sicherzustellen, dass der Fonds ins Ausland 
transferiert wird, damit ich dieses Land zu meiner Sicherheit verlassen kann. 
Der Direktor der Bank hat mir versichert, dass der Fonds übertragen wird, 
sobald ich jemanden vorstelle, der ehrlich ist, den Fonds in meinem Namen zu 
diesem Zweck zu erhalten.

Bitte seien Sie versichert, dass die Bank den Geldbetrag auf Ihr Konto 
überweisen wird und es kein Problem gibt. Diese Transaktion ist 100% risikofrei 
und legitim. Ich bin bereit, Ihnen 30% des Gesamtbetrags als Ausgleich für Ihre 
Bemühungen / Beiträge nach der erfolgreichen Überweisung dieses Fonds auf Ihr 
Konto anzubieten. Sie werden mir auch helfen, 10% für 
Wohltätigkeitsorganisationen und mutterlose Babys in Ihrem Land zu spenden.

Ich möchte nur, dass Sie für mich als mein ausländischer Partner auftreten, 
damit die Bank diesen Fonds auf Ihr Konto überweist, damit ich in diesem Land 
leben kann. Bitte, ich werde Ihre dringende Hilfe wegen meines gegenwärtigen 
Zustands jetzt brauchen. Wenn Sie sich bereit erklären, mit mir in Bezug auf 
diesen Zweck zusammenzuarbeiten, geben Sie mir bitte Ihr Interesse, indem Sie 
mir antworten, damit ich Ihnen die erforderlichen Informationen und 
Einzelheiten zur weiteren Vorgehensweise geben kann. Ich biete Ihnen 30% des 
Geldes für Ihre Hilfe an und Unterstützung, um damit umzugehen.

Ihre dringende Antwort wird geschätzt.
Freundliche Grüße
Bintou Deme
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn] controller: IPv6 Prefix-Delegation: introduce RENEW/REBIND msg support

[Lorenzo Bianconi]

> On 10/9/20 2:59 PM, 0-day Robot wrote:
> > Bleep bloop.  Greetings Lorenzo Bianconi, I am a robot and I have tried out 
> > your patch.
> > Thanks for your contribution.
> > 
> > I encountered some error that I wasn't expecting.  See the details below.
> > 
> > 
> > build:
> > mv -f $depbase.Tpo $depbase.Po
> > depbase=`echo controller/lport.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
> > gcc -std=gnu99 -DHAVE_CONFIG_H -I.   -I ./include  -I ./include -I ./ovn -I 
> > ./include -I 
> > /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include
> >  -I 
> > /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include
> >  -I 
> > /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib
> >  -I 
> > /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib
> >  -I 
> > /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR
> >  -I 
> > /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR
> >  -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
> > -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
> > -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
> > -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing 
> > -Wshadow -Werror -Werror  -g -O2 -MT controller/lport.o -MD -MP -MF 
> > $depbase.Tpo -c -o controller/lport.o controller/lport.c &&\
> > mv -f $depbase.Tpo $depbase.Po
> > depbase=`echo controller/ofctrl.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
> > gcc -std=gnu99 -DHAVE_CONFIG_H -I.   -I ./include  -I ./include -I ./ovn -I 
> > ./include -I 
> > /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include
> >  -I 
> > /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include
> >  -I 
> > /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib
> >  -I 
> > /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib
> >  -I 
> > /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR
> >  -I 
> > /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR
> >  -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
> > -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
> > -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
> > -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing 
> > -Wshadow -Werror -Werror  -g -O2 -MT controller/ofctrl.o -MD -MP -MF 
> > $depbase.Tpo -c -o controller/ofctrl.o controller/ofctrl.c &&\
> > mv -f $depbase.Tpo $depbase.Po
> > depbase=`echo controller/pinctrl.o | sed 's|[^/]*$|.deps/&|;s|\.o$||'`;\
> > gcc -std=gnu99 -DHAVE_CONFIG_H -I.   -I ./include  -I ./include -I ./ovn -I 
> > ./include -I 
> > /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include
> >  -I 
> > /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/include
> >  -I 
> > /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib
> >  -I 
> > /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR/lib
> >  -I 
> > /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR
> >  -I 
> > /var/lib/jenkins/jobs/0day_robot_upstream_build_ovn_from_pw/workspace/OVSDIR
> >  -I ./lib -I ./lib-Wstrict-prototypes -Wall -Wextra -Wno-sign-compare 
> > -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter 
> > -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition 
> > -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing 
> > -Wshadow -Werror -Werror  -g -O2 -MT controller/pinctrl.o -MD -MP -MF 
> > $depbase.Tpo -c -o controller/pinctrl.o controller/pinctrl.c &&\
> > mv -f $depbase.Tpo $depbase.Po
> > controller/pinctrl.c: In function ‘pinctrl_parse_dhcpv6_reply’:
> > controller/pinctrl.c:832:12: error: missing initializer for field ‘__in6_u’ 
> > of ‘struct in6_addr’ [-Werror=missing-field-initializers]
> >  struct in6_addr ipv6 = {};
> 
> I guess, you need to use 'in6addr_any' as initializer, if you want to
> avoid compiler warnings.
> 
> Best regards, Ilya Maximets.

ack, thx I will fix it in v2.

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

Re: [ovs-dev] [PATCH 1/1] netdev-offload-dpdk: Preserve HW statistics for modified flows

[Sriharsha Basavapatna via dev]

On Sun, Sep 6, 2020 at 5:52 PM Eli Britstein  wrote:
>
> ping
>
> On 7/30/2020 4:52 PM, Eli Britstein wrote:
> > In case of a flow modification, preserve the HW statistics of the old HW
> > flow to the new one.
> >
> > Signed-off-by: Eli Britstein 
> > Reviewed-by: Gaetan Rivet 

Update fixes: tag
> > ---
> >   lib/netdev-offload-dpdk.c | 28 +---
> >   1 file changed, 21 insertions(+), 7 deletions(-)
> >
> > diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> > index de6101e4d..960acb2da 100644
> > --- a/lib/netdev-offload-dpdk.c
> > +++ b/lib/netdev-offload-dpdk.c
> > @@ -78,7 +78,7 @@ ufid_to_rte_flow_data_find(const ovs_u128 *ufid)
> >   return NULL;
> >   }
> >
> > -static inline void
> > +static inline struct ufid_to_rte_flow_data *
> >   ufid_to_rte_flow_associate(const ovs_u128 *ufid,
> >  struct rte_flow *rte_flow, bool 
> > actions_offloaded)
> >   {
> > @@ -103,6 +103,7 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid,
> >
> >   cmap_insert(&ufid_to_rte_flow,
> >   CONST_CAST(struct cmap_node *, &data->node), hash);
> > +return data;
> >   }
> >
> >   static inline void
> > @@ -1407,7 +1408,7 @@ out:
> >   return flow;
> >   }
> >
> > -static int
> > +static struct ufid_to_rte_flow_data *
> >   netdev_offload_dpdk_add_flow(struct netdev *netdev,
> >struct match *match,
> >struct nlattr *nl_actions,
> > @@ -1416,6 +1417,7 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
> >struct offload_info *info)
> >   {
> >   struct flow_patterns patterns = { .items = NULL, .cnt = 0 };
> > +struct ufid_to_rte_flow_data *flows_data = NULL;
> >   bool actions_offloaded = true;
> >   struct rte_flow *flow;
> >   int ret = 0;

We can eliminate 'ret' with this change, since it is only being used
to catch the return value of parse_flow_match().
> > @@ -1442,13 +1444,13 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
> >   ret = -1;

Relates to the above comment.
> >   goto out;
> >   }
> > -ufid_to_rte_flow_associate(ufid, flow, actions_offloaded);
> > +flows_data = ufid_to_rte_flow_associate(ufid, flow, actions_offloaded);
> >   VLOG_DBG("%s: installed flow %p by ufid "UUID_FMT,
> >netdev_get_name(netdev), flow, UUID_ARGS((struct uuid 
> > *)ufid));
> >
> >   out:
> >   free_flow_patterns(&patterns);
> > -return ret;
> > +return flows_data;
> >   }
> >
> >   static int
> > @@ -1482,14 +1484,19 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, 
> > struct match *match,
> >struct dpif_flow_stats *stats)
> >   {
> >   struct ufid_to_rte_flow_data *rte_flow_data;
> > +struct dpif_flow_stats old_stats;
> > +bool modification = false;
> >   int ret;
> >
> >   /*
> >* If an old rte_flow exists, it means it's a flow modification.
> >* Here destroy the old rte flow first before adding a new one.
> > + * Keep the stats for the newly created rule.
> >*/
> >   rte_flow_data = ufid_to_rte_flow_data_find(ufid);
> >   if (rte_flow_data && rte_flow_data->rte_flow) {
> > +old_stats = rte_flow_data->stats;
> > +modification = true;
> >   ret = netdev_offload_dpdk_destroy_flow(netdev, ufid,
> >  rte_flow_data->rte_flow);
> >   if (ret < 0) {
> > @@ -1497,11 +1504,18 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, 
> > struct match *match,
> >   }
> >   }
> >
> > +rte_flow_data = netdev_offload_dpdk_add_flow(netdev, match, actions,
> > + actions_len, ufid, info);
> > +if (!rte_flow_data) {
> > +return -1;
> > +}
> > +if (modification) {
> > +rte_flow_data->stats = old_stats;
> > +}
> >   if (stats) {
> > -memset(stats, 0, sizeof *stats);

What if it is a new flow, don't we need to clear the stats ?
> > +*stats = rte_flow_data->stats;
> >   }
> > -return netdev_offload_dpdk_add_flow(netdev, match, actions,
> > -actions_len, ufid, info);
> > +return 0;
> >   }
> >
> >   static int
> ___
> 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] netdev-offload: add offload-delay option to delay offload the datapath flows

[wenxu]

From: wenxu 

Add offload-delay option to delay offload the datapath flow.
Sometimes there is no need for offload the short connection flows which
overload the add/del flows in the HW. It is better to offload persistent
connection.

enable it as following:
ovs-vsctl set Open_Vswitch . other-config:offload-delay=1

Signed-off-by: wenxu 
---
 lib/dpif-netlink.c|  8 
 lib/dpif.h|  4 +++-
 lib/netdev-offload.c  | 18 ++
 lib/netdev-offload.h  |  1 +
 ofproto/ofproto-dpif-upcall.c | 33 +++--
 vswitchd/vswitch.xml  | 21 +
 6 files changed, 78 insertions(+), 7 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index 2f881e4..094a015 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -2060,6 +2060,10 @@ parse_flow_put(struct dpif_netlink *dpif, struct 
dpif_flow_put *put)
 return EOPNOTSUPP;
 }
 
+if (put->flags & DPIF_FP_NO_OFFLOAD) {
+return EOPNOTSUPP;
+}
+
 err = parse_key_and_mask_to_match(put->key, put->key_len, put->mask,
   put->mask_len, &match);
 if (err) {
@@ -2148,6 +2152,10 @@ parse_flow_put(struct dpif_netlink *dpif, struct 
dpif_flow_put *put)
 (oor_netdev ? oor_netdev->name : dev->name));
 }
 
+if (put->flags & DPIF_FP_TRY_OFFLOAD) {
+return 0;
+}
+
 out:
 if (err && err != EEXIST && (put->flags & DPIF_FP_MODIFY)) {
 /* Modified rule can't be offloaded, try and delete from HW */
diff --git a/lib/dpif.h b/lib/dpif.h
index 2d52f01..95d4693 100644
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -529,7 +529,9 @@ enum dpif_flow_put_flags {
 DPIF_FP_CREATE = 1 << 0,/* Allow creating a new flow. */
 DPIF_FP_MODIFY = 1 << 1,/* Allow modifying an existing flow. */
 DPIF_FP_ZERO_STATS = 1 << 2, /* Zero the stats of an existing flow. */
-DPIF_FP_PROBE = 1 << 3  /* Suppress error messages, if any. */
+DPIF_FP_PROBE = 1 << 3, /* Suppress error messages, if any. */
+DPIF_FP_NO_OFFLOAD = 1 << 4,  /* Don't try send to netdev */
+DPIF_FP_TRY_OFFLOAD = 1 << 5  /* Only try send to netdev */
 };
 
 bool dpif_probe_feature(struct dpif *, const char *name,
diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c
index 2da3bc7..731141b 100644
--- a/lib/netdev-offload.c
+++ b/lib/netdev-offload.c
@@ -621,6 +621,7 @@ netdev_ifindex_to_odp_port(int ifindex)
 }
 
 static bool netdev_offload_rebalance_policy = false;
+static unsigned netdev_offload_delay = 0;
 
 bool
 netdev_is_offload_rebalance_policy_enabled(void)
@@ -628,6 +629,20 @@ netdev_is_offload_rebalance_policy_enabled(void)
 return netdev_offload_rebalance_policy;
 }
 
+unsigned
+netdev_is_offload_delay(void)
+{
+return netdev_offload_delay;
+}
+
+static void
+netdev_set_offload_delay(unsigned delay)
+{
+if (delay >= 1 && delay <= 6) {
+netdev_offload_delay = delay;
+}
+}
+
 static void
 netdev_ports_flow_init(void)
 {
@@ -660,6 +675,9 @@ netdev_set_flow_api_enabled(const struct smap 
*ovs_other_config)
 netdev_offload_rebalance_policy = true;
 }
 
+netdev_set_offload_delay(smap_get_int(ovs_other_config,
+  "offload-delay", 0));
+
 netdev_ports_flow_init();
 
 ovsthread_once_done(&once);
diff --git a/lib/netdev-offload.h b/lib/netdev-offload.h
index 4c0ed2a..9d0dd00 100644
--- a/lib/netdev-offload.h
+++ b/lib/netdev-offload.h
@@ -103,6 +103,7 @@ bool netdev_any_oor(void);
 bool netdev_is_flow_api_enabled(void);
 void netdev_set_flow_api_enabled(const struct smap *ovs_other_config);
 bool netdev_is_offload_rebalance_policy_enabled(void);
+unsigned netdev_is_offload_delay(void);
 
 struct dpif_port;
 int netdev_ports_insert(struct netdev *, const char *dpif_type,
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index e022fde..e873dd7 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -310,6 +310,7 @@ struct udpif_key {
 
 uint32_t key_recirc_id;   /* Non-zero if reference is held by the ukey. */
 struct recirc_refs recircs;  /* Action recirc IDs with references held. */
+bool try_offload; /* Delay offload flags for flow */
 
 #define OFFL_REBAL_INTVL_MSEC  3000/* dynamic offload rebalance freq */
 struct netdev *in_netdev;  /* in_odp_port's netdev */
@@ -1582,8 +1583,13 @@ handle_upcalls(struct udpif *udpif, struct upcall 
*upcalls,
 struct udpif_key *ukey = upcall->ukey;
 
 if (ukey_install(udpif, ukey)) {
+enum dpif_flow_put_flags flags = DPIF_FP_CREATE;
+
 upcall->ukey_persists = true;
-put_op_init(&ops[n_ops++], ukey, DPIF_FP_CREATE);
+if (netdev_is_offload_delay()) {
+flags |= DPIF_FP_NO_OFFLOAD;
+}
+

[ovs-dev] [PATCH v2 ovn] controller: IPv6 Prefix-Delegation: introduce RENEW/REBIND msg support

[Lorenzo Bianconi]

Introduce RENEW/REBIND message support to OVN IPv6 PD support
according to RFC-3633 [0] in order to renew IPv6 prefixes when
T1/T2 are elapsed

[0] https://tools.ietf.org/html/rfc3633

Acked-by: Mark Michelson 
Signed-off-by: Lorenzo Bianconi 
---
Changes since v1:
- fixed compilation warning
---
 controller/pinctrl.c | 131 +--
 lib/ovn-l7.h |   3 +
 tests/system-ovn.at  |  18 --
 3 files changed, 118 insertions(+), 34 deletions(-)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 0a7020533..c1ef1610f 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -573,11 +573,22 @@ enum {
 PREFIX_REQUEST,
 PREFIX_PENDING,
 PREFIX_DONE,
+PREFIX_RENEW,
+PREFIX_REBIND,
 };
 
 struct ipv6_prefixd_state {
 long long int next_announce;
+long long int last_complete;
 long long int last_used;
+/* IPv6 PD server info */
+struct in6_addr server_addr;
+struct eth_addr sa;
+/* server_id_info */
+struct {
+uint8_t *data;
+uint8_t len;
+} uuid;
 struct in6_addr ipv6_addr;
 struct eth_addr ea;
 struct eth_addr cmac;
@@ -781,20 +792,26 @@ out:
 static void
 pinctrl_prefixd_state_handler(const struct flow *ip_flow,
   struct in6_addr addr, unsigned aid,
+  struct eth_addr sa, struct in6_addr server_addr,
   char prefix_len, unsigned t1, unsigned t2,
-  unsigned plife_time, unsigned vlife_time)
+  unsigned plife_time, unsigned vlife_time,
+  uint8_t *uuid, uint8_t uuid_len)
 {
 struct ipv6_prefixd_state *pfd;
 
 pfd = pinctrl_find_prefixd_state(ip_flow, aid);
 if (pfd) {
 pfd->state = PREFIX_PENDING;
-pfd->plife_time = plife_time;
-pfd->vlife_time = vlife_time;
+pfd->server_addr = server_addr;
+pfd->sa = sa;
+pfd->uuid.data = uuid;
+pfd->uuid.len = uuid_len;
+pfd->plife_time = plife_time * 1000;
+pfd->vlife_time = vlife_time * 1000;
 pfd->plen = prefix_len;
 pfd->prefix = addr;
-pfd->t1 = t1;
-pfd->t2 = t2;
+pfd->t1 = t1 * 1000;
+pfd->t2 = t2 * 1000;
 notify_pinctrl_main();
 }
 }
@@ -804,19 +821,21 @@ pinctrl_parse_dhcpv6_reply(struct dp_packet *pkt_in,
const struct flow *ip_flow)
 OVS_REQUIRES(pinctrl_mutex)
 {
+struct eth_header *eth = dp_packet_eth(pkt_in);
+struct ip6_hdr *in_ip = dp_packet_l3(pkt_in);
 struct udp_header *udp_in = dp_packet_l4(pkt_in);
 unsigned char *in_dhcpv6_data = (unsigned char *)(udp_in + 1);
 size_t dlen = MIN(ntohs(udp_in->udp_len), dp_packet_l4_size(pkt_in));
 unsigned t1 = 0, t2 = 0, vlife_time = 0, plife_time = 0;
-uint8_t *end = (uint8_t *)udp_in + dlen;
-uint8_t prefix_len = 0;
-struct in6_addr ipv6;
+uint8_t *end = (uint8_t *)udp_in + dlen, *uuid = NULL;
+uint8_t prefix_len = 0, uuid_len = 0;
+struct in6_addr ipv6 = in6addr_any;
 bool status = false;
 unsigned aid = 0;
 
-memset(&ipv6, 0, sizeof (struct in6_addr));
 /* skip DHCPv6 common header */
 in_dhcpv6_data += 4;
+
 while (in_dhcpv6_data < end) {
 struct dhcpv6_opt_header *in_opt =
  (struct dhcpv6_opt_header *)in_dhcpv6_data;
@@ -867,14 +886,20 @@ pinctrl_parse_dhcpv6_reply(struct dp_packet *pkt_in,
 }
 break;
 }
+case DHCPV6_OPT_SERVER_ID_CODE:
+uuid_len = ntohs(in_opt->len);
+uuid = xmalloc(uuid_len);
+memcpy(uuid, in_opt + 1, uuid_len);
+break;
 default:
 break;
 }
 in_dhcpv6_data += opt_len;
 }
 if (status) {
-pinctrl_prefixd_state_handler(ip_flow, ipv6, aid, prefix_len,
-  t1, t2, plife_time, vlife_time);
+pinctrl_prefixd_state_handler(ip_flow, ipv6, aid, eth->eth_src,
+  in_ip->ip6_src, prefix_len, t1, t2,
+  plife_time, vlife_time, uuid, uuid_len);
 }
 }
 
@@ -904,27 +929,42 @@ pinctrl_handle_dhcp6_server(struct rconn *swconn, const 
struct flow *ip_flow,
 }
 
 static void
-compose_prefixd_solicit(struct dp_packet *b,
-struct ipv6_prefixd_state *pfd,
-const struct eth_addr eth_dst,
-const struct in6_addr *ipv6_dst)
+compose_prefixd_packet(struct dp_packet *b, struct ipv6_prefixd_state *pfd)
 {
-eth_compose(b, eth_dst, pfd->ea, ETH_TYPE_IPV6, IPV6_HEADER_LEN);
+struct in6_addr ipv6_dst;
+struct eth_addr eth_dst;
 
 int payload = sizeof(struct dhcpv6_opt_server_id) +
   sizeof(struct dhcpv6_opt_ia_na);
+if (pfd->uuid.len) {
+payload += pfd->uuid.len + sizeof(struct dhcpv6_opt_header)

[ovs-dev] [PATCH ovn] vtep-controller: extract mac address in building umr

[numans]

From: Vladislav Odintsov 

port_binding_rec->mac array contains items with addresses (MAC, IP, ...)
delimited by whitespace.
In Unicast_Macs_Remote record should be only one mac address.
Now mac address extracts from port_binding_rec->mac[i].

Submitted-at: https://github.com/ovn-org/ovn/pull/48
Signed-off-by: Vladislav Odintsov 
Signed-off-by: Numan Siddique 
---
 controller-vtep/vtep.c   | 12 ++--
 tests/ovn-controller-vtep.at | 16 
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/controller-vtep/vtep.c b/controller-vtep/vtep.c
index e2baca5a7..5538c7d6a 100644
--- a/controller-vtep/vtep.c
+++ b/controller-vtep/vtep.c
@@ -20,6 +20,7 @@
 #include "lib/hash.h"
 #include "openvswitch/hmap.h"
 #include "openvswitch/shash.h"
+#include "lib/ovn-util.h"
 #include "lib/smap.h"
 #include "lib/sset.h"
 #include "lib/util.h"
@@ -366,7 +367,13 @@ vtep_macs_run(struct ovsdb_idl_txn *vtep_idl_txn, struct 
shash *ucast_macs_rmts,
 for (i = 0; i < port_binding_rec->n_mac; i++) {
 const struct vteprec_ucast_macs_remote *umr;
 const struct sbrec_port_binding *conflict;
-char *mac = port_binding_rec->mac[i];
+struct lport_addresses laddrs;
+
+if (!extract_lsp_addresses(port_binding_rec->mac[i], &laddrs)) {
+continue;
+};
+
+char *mac = laddrs.ea_s;
 
 /* Checks for duplicate MAC in the same vtep logical switch. */
 conflict = shash_find_data(&ls_node->added_macs, mac);
@@ -384,7 +391,7 @@ vtep_macs_run(struct ovsdb_idl_txn *vtep_idl_txn, struct 
shash *ucast_macs_rmts,
 tnl_key);
 umr = shash_find_data(ucast_macs_rmts, mac_ip_tnlkey);
 /* If finds the 'umr' entry for the mac, ip, and tnl_key, deletes
- * the entry from shash so that it is not gargage collected.
+ * the entry from shash so that it is not garbage collected.
  *
  * If not found, creates a new 'umr' entry. */
 if (umr && umr->logical_switch == ls_node->vtep_ls) {
@@ -395,6 +402,7 @@ vtep_macs_run(struct ovsdb_idl_txn *vtep_idl_txn, struct 
shash *ucast_macs_rmts,
 vteprec_ucast_macs_remote_set_locator(new_umr, pl);
 }
 free(mac_ip_tnlkey);
+destroy_lport_addresses(&laddrs);
 }
 }
 
diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at
index 3092dd5ca..f0598fa4d 100644
--- a/tests/ovn-controller-vtep.at
+++ b/tests/ovn-controller-vtep.at
@@ -383,6 +383,22 @@ AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | 
cut -d ':' -f2- | tr -
 "f0:ab:cd:ef:01:03"
 ])
 
+# adds MAC-IP pair to logical switch port.
+AT_CHECK([ovn-nbctl lsp-set-addresses vif0 "f0:ab:cd:ef:01:04 192.168.0.1"])
+OVS_WAIT_UNTIL([test -n "`vtep-ctl list Ucast_Macs_Remote | grep 
'f0:ab:cd:ef:01:04'`"])
+AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':' -f2- | tr 
-d ' ' | sort], [0], [dnl
+"f0:ab:cd:ef:01:04"
+])
+
+# adds another MAC-IP pair to logical switch port.
+AT_CHECK([ovn-nbctl lsp-set-addresses vif0 "f0:ab:cd:ef:01:04 192.168.0.1" 
"f0:ab:cd:ef:01:05 192.168.0.2"])
+OVS_WAIT_UNTIL([test -n "`vtep-ctl list Ucast_Macs_Remote | grep 
'f0:ab:cd:ef:01:05'`"])
+AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':' -f2- | tr 
-d ' ' | sort], [0], [dnl
+
+"f0:ab:cd:ef:01:04"
+"f0:ab:cd:ef:01:05"
+])
+
 # removes one mac to logical switch port.
 AT_CHECK([ovn-nbctl lsp-set-addresses vif0 f0:ab:cd:ef:01:03])
 OVS_WAIT_UNTIL([test -z "`vtep-ctl --columns=MAC list Ucast_Macs_Remote | grep 
02`"])
-- 
2.26.2

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

Re: [ovs-dev] [PATCH ovn] vtep-controller: extract mac address in building umr

[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: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Numan Siddique 
Lines checked: 92, 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 ovn v4] ofctrl: Add a predictable resolution for conflicting flow actions.

[Dumitru Ceara]

On 10/11/20 9:40 PM, Han Zhou wrote:
> 
> 
> On Sun, Oct 11, 2020 at 5:14 AM Dumitru Ceara  > wrote:
>> 
>> On 10/6/20 10:30 AM, Dumitru Ceara wrote:
>>> On 10/2/20 10:28 PM, Han Zhou wrote:
 
 Hi Dumitru,
 
>>> 
>>> Hi Han,
>>> 
 Thanks for the revision. It looks good overall. The major
 problems left are: 1. it missed updating the active
 desired_flow in the installed_flow. 2. when a tracked flow
 deletion is handled, if there are other desired flows linked to
 the same installed flow, it should use the same criteria to
 decide which flow should become active from the candidate
 flows.
 
>>> 
>>> I see, you're right, thanks. I'll fix it in the next revision.
>>> 
 I also noticed 2 problems of the existing code while reviewing
 this patch. I submitted 2 patches: 1. 
 https://patchwork.ozlabs.org/project/ovn/patch/1601663136-19111-1-git-send-email-hz...@ovn.org/
>>
 
>> 2.
 https://patchwork.ozlabs.org/project/ovn/patch/20201002200504.2954064-1-hz...@ovn.org/
>>
 
>> 
 1) is required for your solution to work properly. 2) is not
 directly related but will cause a merge conflict. Please help
 review both since they are closely related to your patch.
 
>>> 
>>> I'll try to review your patches as soon as possible and I'll
>>> respin mine afterwards.
>>> 
 Please see more detailed comments below.
 
 On Wed, Sep 30, 2020 at 12:39 AM Dumitru Ceara
 mailto:dce...@redhat.com> 
 >> wrote:
> 
> Until now, in case the ACL configuration generates openflows
> that have the same match but different actions,
> ovn-controller was using the following approach: 1. If the
> flow being added contains conjunctive actions, merge its 
> actions with the already existing flow. 2. Otherwise, if the
> flow is being added incrementally 
> (update_installed_flows_by_track), don't install the new flow
> but instead keep the old one. 3. Otherwise,
> (update_installed_flows_by_compare), don't install the new
> flow but instead keep the old one.
> 
> Even though one can argue that having an ACL with a match
> that includes the match of another ACL is a misconfiguration,
> it can happen that the users provision OVN like this.
> Depending on the order of reading and installing the logical
> flows, the above operations can yield unpredictable results,
> e.g., allow specific traffic but then after ovn-controller is
> restarted (or a recompute happens) that specific traffic
> starts being dropped.
> 
> A simple example of ACL configuration is: ovn-nbctl acl-add
> ls to-lport 3 '(ip4.src==10.0.0.1 || ip4.src==10.0.0.2) &&
> (ip4.dst == 10.0.0.3 || ip4.dst == 10.0.0.4)' allow ovn-nbctl
> acl-add ls to-lport 3 'ip4.src==10.0.0.1' allow ovn-nbctl
> acl-add ls to-lport 2 'arp' allow ovn-nbctl acl-add ls
> to-lport 1 'ip4' drop
> 
> This is a pattern used by most CMSs: - define a default deny
> policy. - punch holes in the default deny policy based on
> user specific configurations.
> 
> Without this commit the behavior for traffic from 10.0.0.1 to
> 10.0.0.5 is unpredictable. Depending on the order of
> operations traffic might be dropped or allowed.
> 
> It's also quite hard to force the CMS to ensure that such
> match overlaps never occur.
> 
> To address this issue we now resolve conflicts between flows
> with the same match and different actions by giving
> precedence to less restrictive flows. This means that if the
> installed flow has action "conjunction" and the desired flow
> doesn't then we prefer the desired flow. Similarly, if the
> desired flow has action "conjunction" and the installed flow
> doesn't then we prefer the already installed flow.
> 
> CC: Daniel Alvarez    >> CC: Han Zhou    >> Reported-at:
> https://bugzilla.redhat.com/1871931 Acked-by: Mark Michelson
> mailto:mmich...@redhat.com>
 >>
> Signed-off-by: Dumitru Ceara  
 >>
> --- v4: - Address Han's comments: - make sure only flows with
> action conjunction are combined. v3: - Add Mark's ack. - Add
> last missing pcap check in the test. v2: - Address Han's
> comments: - Do not delete desired flow that cannot be merged,
> it might be installed later. - Fix typos in the commit log. -
> Update the test to check the OVS flows. --- 
> controller/ofctrl.c | 163
> --- tests/ovn.at
>     

[ovs-dev] Product inquiry,

[Anne Weber]

Greetings,

This is Anne from VULCAN SUPPLY & TRADING SÁRL
VS&T COMMODITIES SÁRL.

We would like you to order and supply us your products this time., Kindly 
please give us more information about your company through your Catalog, We are 
interested in your esteemed product prices, delivery time and are willing to 
purchase from your company if price is competitive.

Stay Safe

I await your feedback.

Best regards

Purchase Manager
 
Anne Weber 


VULCAN SUPPLY & TRADING SÁRL

VS&T COMMODITIES SÁRL

Chemin de Blandonnet 8

1214 Vernier, Switzerland
VS&T


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

Re: [ovs-dev] [PATCH 1/1] netdev-offload-dpdk: Preserve HW statistics for modified flows

[Eli Britstein]

On 10/12/2020 11:45 AM, Sriharsha Basavapatna wrote:

On Sun, Sep 6, 2020 at 5:52 PM Eli Britstein  wrote:

ping

On 7/30/2020 4:52 PM, Eli Britstein wrote:

In case of a flow modification, preserve the HW statistics of the old HW
flow to the new one.

Signed-off-by: Eli Britstein 
Reviewed-by: Gaetan Rivet 

Update fixes: tag

I'll put 3c7330ebf036 netdev-offload-dpdk: Support offload of output action.
As it's the first commit that does full offload. Before that there are 
no HW rules that count. What do you think?

---
   lib/netdev-offload-dpdk.c | 28 +---
   1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index de6101e4d..960acb2da 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -78,7 +78,7 @@ ufid_to_rte_flow_data_find(const ovs_u128 *ufid)
   return NULL;
   }

-static inline void
+static inline struct ufid_to_rte_flow_data *
   ufid_to_rte_flow_associate(const ovs_u128 *ufid,
  struct rte_flow *rte_flow, bool actions_offloaded)
   {
@@ -103,6 +103,7 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid,

   cmap_insert(&ufid_to_rte_flow,
   CONST_CAST(struct cmap_node *, &data->node), hash);
+return data;
   }

   static inline void
@@ -1407,7 +1408,7 @@ out:
   return flow;
   }

-static int
+static struct ufid_to_rte_flow_data *
   netdev_offload_dpdk_add_flow(struct netdev *netdev,
struct match *match,
struct nlattr *nl_actions,
@@ -1416,6 +1417,7 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
struct offload_info *info)
   {
   struct flow_patterns patterns = { .items = NULL, .cnt = 0 };
+struct ufid_to_rte_flow_data *flows_data = NULL;
   bool actions_offloaded = true;
   struct rte_flow *flow;
   int ret = 0;

We can eliminate 'ret' with this change, since it is only being used
to catch the return value of parse_flow_match().

Ack

@@ -1442,13 +1444,13 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
   ret = -1;

Relates to the above comment.

   goto out;
   }
-ufid_to_rte_flow_associate(ufid, flow, actions_offloaded);
+flows_data = ufid_to_rte_flow_associate(ufid, flow, actions_offloaded);
   VLOG_DBG("%s: installed flow %p by ufid "UUID_FMT,
netdev_get_name(netdev), flow, UUID_ARGS((struct uuid *)ufid));

   out:
   free_flow_patterns(&patterns);
-return ret;
+return flows_data;
   }

   static int
@@ -1482,14 +1484,19 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, 
struct match *match,
struct dpif_flow_stats *stats)
   {
   struct ufid_to_rte_flow_data *rte_flow_data;
+struct dpif_flow_stats old_stats;
+bool modification = false;
   int ret;

   /*
* If an old rte_flow exists, it means it's a flow modification.
* Here destroy the old rte flow first before adding a new one.
+ * Keep the stats for the newly created rule.
*/
   rte_flow_data = ufid_to_rte_flow_data_find(ufid);
   if (rte_flow_data && rte_flow_data->rte_flow) {
+old_stats = rte_flow_data->stats;
+modification = true;
   ret = netdev_offload_dpdk_destroy_flow(netdev, ufid,
  rte_flow_data->rte_flow);
   if (ret < 0) {
@@ -1497,11 +1504,18 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, 
struct match *match,
   }
   }

+rte_flow_data = netdev_offload_dpdk_add_flow(netdev, match, actions,
+ actions_len, ufid, info);
+if (!rte_flow_data) {
+return -1;
+}
+if (modification) {
+rte_flow_data->stats = old_stats;
+}
   if (stats) {
-memset(stats, 0, sizeof *stats);

What if it is a new flow, don't we need to clear the stats ?


It is already cleared before. Allocation is xZalloc. See in 
ufid_to_rte_flow_associate:


    struct ufid_to_rte_flow_data *data = xzalloc(sizeof *data);


+*stats = rte_flow_data->stats;
   }
-return netdev_offload_dpdk_add_flow(netdev, match, actions,
-actions_len, ufid, info);
+return 0;
   }

   static int

___
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 1/1] netdev-offload-dpdk: Support vxlan encap offload with load actions

[Eli Britstein]

On 10/12/2020 10:20 AM, Sriharsha Basavapatna wrote:

On Sun, Sep 6, 2020 at 5:51 PM Eli Britstein  wrote:

ping

On 7/30/2020 1:58 PM, Eli Britstein wrote:

From: Lei Wang 

Struct match has the tunnel values/masks in
match->flow.tunnel/match->wc.masks.tunnel.
Load actions such as load:0xa566c10->NXM_NX_TUN_IPV4_DST[],
load:0xbba->NXM_NX_TUN_ID[] are utilizing the tunnel masks fields,
but those should not be used for matching.
Offloading fails if masks is not clear. Clear it if no tunnel used.

Signed-off-by: Lei Wang 
Reviewed-by: Eli Britstein 
Reviewed-by: Gaetan Rivet 
Signed-off-by: Eli Britstein 
---
   lib/netdev-offload-dpdk.c | 4 
   1 file changed, 4 insertions(+)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index de6101e4d..0d23e4879 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -682,6 +682,10 @@ parse_flow_match(struct flow_patterns *patterns,

   consumed_masks = &match->wc.masks;

+if (!flow_tnl_dst_is_set(&match->flow.tunnel)) {
+memset(&match->wc.masks.tunnel, 0, sizeof match->wc.masks.tunnel);
+}
+

This fix looks good to me.  Did you take a look to see if this can be
fixed in the code that generates these invalid masks in the first
place ?
I wouldn't say it's "invalid masks". OVS takes those masks into 
consideration with such usage of flow_tnl_dst_is_set, that is done in 
several places in the code. For example odp-util.c, lib/netdev-offload-tc.c.


Thanks,
-Harsha

   memset(&consumed_masks->in_port, 0, sizeof consumed_masks->in_port);
   /* recirc id must be zero. */
   if (match->wc.masks.recirc_id & match->flow.recirc_id) {

___
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 v3] conntrack: add generic IP protocol support

[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 
---
v3: Added NEWS item for this feature.
Small style fixes suggested by Ilya.
v2: Small style fix suggested by Aaron Conole.

 NEWS|4 
 lib/conntrack-private.h |3 +++
 lib/conntrack.c |   29 +++--
 tests/system-traffic.at |   29 +
 4 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/NEWS b/NEWS
index 4619e73..f63ed1a 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,10 @@ Post-v2.14.0
  * Removed support for vhost-user dequeue zero-copy.
- The environment variable OVS_UNBOUND_CONF, if set, is now used
  as the DNS resolver's (unbound) configuration file.
+   - Userspace datapath:
+ * Add generic IP protocol support to conntrack. With this change, all
+   none UDP, TCP, and ICMP traffic will be treated as general L3
+   traffic, i.e. using 3 tupples.
 
 
 v2.14.0 - 17 Aug 2020
diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index 3434753..4b1da03 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(MEMBER_SIZEOF(struct conn_key, 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 f42ba4b..66e43b3 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -146,12 +146,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] = &ct_proto_tcp,
-[IPPROTO_UDP] = &ct_proto_other,
-[IPPROTO_ICMP] = &ct_proto_icmp4,
-[IPPROTO_ICMPV6] = &ct_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,
@@ -293,6 +288,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(&ct->resources_lock);
@@ -320,6 +316,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(&setup_l4_once)) {
+for (int i = 0; i < ARRAY_SIZE(l4_protos); i++) {
+l4_protos[i] = &ct_proto_other;
+}
+/* IPPROTO_UDP uses ct_proto_other, so no need to initialize it. */
+l4_protos[IPPROTO_TCP] = &ct_proto_tcp;
+l4_protos[IPPROTO_ICMP] = &ct_proto_icmp4;
+l4_protos[IPPROTO_ICMPV6] = &ct_proto_icmp6;
+
+ovsthread_once_done(&setup_l4_once);
+}
 return ct;
 }
 
@@ -1982,9 +1990,10 @@ extract_l4(struct conn_key *key, const void *data, 
size_t size, bool *related,
 return (!related || check_l4_icmp6(key, data, size, l3,
 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;
 }
 
 static bool
@@ -2267,8 +2276,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 02f0e27..db081d4 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -2333,6 +2333,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,ar

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

[Eelco Chaudron]

On 8 Oct 2020, at 16:33, Ilya Maximets wrote:


On 9/17/20 10:41 AM, Eelco Chaudron wrote:
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 
---
v2: Small style fix suggested by Aaron Conole.

 lib/conntrack-private.h |3 +++
 lib/conntrack.c |   29 +++--
 tests/system-traffic.at |   29 +
 3 files changed, 51 insertions(+), 10 deletions(-)


Hi, Eelco.  Thanks for the patch!

Could you, please, add a NEWS entry since this is kind of user-visible 
change.

It should be something like:

   - Userspace datapath:
 * ...

Some small nitpicks inline. :)


Thanks for the review. Just sent out a V3, including the two nitpicks 
below, and adding the NEWS section as suggested here.


Cheers,

Eelco



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 should be 'MEMBER_SIZEOF(struct conn_key, 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..3597112 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] = &ct_proto_tcp,
-[IPPROTO_UDP] = &ct_proto_other,
-[IPPROTO_ICMP] = &ct_proto_icmp4,
-[IPPROTO_ICMPV6] = &ct_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(&ct->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(&setup_l4_once)) {
+for (int i = 0; i < ARRAY_SIZE(l4_protos); i++) {
+l4_protos[i] = &ct_proto_other;
+}
+/* IPPROTO_UDP uses ct_proto_other, so no need to initialize 
it. */

+l4_protos[IPPROTO_TCP]= &ct_proto_tcp;
+l4_protos[IPPROTO_ICMP]   = &ct_proto_icmp4;
+l4_protos[IPPROTO_ICMPV6] = &ct_proto_icmp6;
+
+ovsthread_once_done(&setup_l4_once);
+}
 return ct;
 }

@@ -1970,9 +1978,10 @@ extract_l4(struct conn_key *key, const void 
*data, size_t size, bool *related,

 return (!related || check_l4_icmp6(key, data, size, l3,
 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 */


Period in the end of a comment.


+return true;
 }

 static bool
@@ -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(

[ovs-dev] [PATCH ovs-dev, dpdk-latest 1/2] ovs-atomic: Rename memory_order -> ovs_memory_order

[Eli Britstein]

DPDK commit [1] uses function variables named "memory_order".
Compilation fails with:
error: declaration of 'memory_order' shadows a global declaration
[-Werror=shadow]
 rte_atomic_thread_fence(int memory_order)
Rename enum memory_order to ovs_memory_order to avoid that conflict.

[1] 672a15056380 ("eal: add wrapper for C11 atomic thread fence")

Signed-off-by: Eli Britstein 
---
 lib/ovs-atomic-clang.h|  2 +-
 lib/ovs-atomic-flag-gcc4.7+.h |  4 ++--
 lib/ovs-atomic-gcc4+.h| 12 ++--
 lib/ovs-atomic-gcc4.7+.h  |  2 +-
 lib/ovs-atomic-i586.h |  8 
 lib/ovs-atomic-msvc.h |  8 
 lib/ovs-atomic-pthreads.h | 10 +-
 lib/ovs-atomic-x86_64.h   |  8 
 lib/ovs-atomic.h  | 34 +-
 lib/ovs-rcu.h | 10 +-
 10 files changed, 49 insertions(+), 49 deletions(-)

diff --git a/lib/ovs-atomic-clang.h b/lib/ovs-atomic-clang.h
index 34cc2faa7..4ff44f69b 100644
--- a/lib/ovs-atomic-clang.h
+++ b/lib/ovs-atomic-clang.h
@@ -36,7 +36,7 @@ typedef enum {
 memory_order_release = 3,
 memory_order_acq_rel = 4,
 memory_order_seq_cst = 5
-} memory_order;
+} ovs_memory_order;
 
 #define atomic_thread_fence(ORDER) __c11_atomic_thread_fence(ORDER)
 #define atomic_signal_fence(ORDER) __c11_atomic_signal_fence(ORDER)
diff --git a/lib/ovs-atomic-flag-gcc4.7+.h b/lib/ovs-atomic-flag-gcc4.7+.h
index 49cd5d20f..a959ca496 100644
--- a/lib/ovs-atomic-flag-gcc4.7+.h
+++ b/lib/ovs-atomic-flag-gcc4.7+.h
@@ -28,7 +28,7 @@ typedef struct {
 
 static inline bool
 atomic_flag_test_and_set_explicit(volatile atomic_flag *object,
-  memory_order order)
+  ovs_memory_order order)
 {
 return __atomic_test_and_set(&object->b, order);
 }
@@ -40,7 +40,7 @@ atomic_flag_test_and_set(volatile atomic_flag *object)
 }
 
 static inline void
-atomic_flag_clear_explicit(volatile atomic_flag *object, memory_order order)
+atomic_flag_clear_explicit(volatile atomic_flag *object, ovs_memory_order 
order)
 {
 __atomic_clear(object, order);
 }
diff --git a/lib/ovs-atomic-gcc4+.h b/lib/ovs-atomic-gcc4+.h
index 25bcf20a0..b3e73d004 100644
--- a/lib/ovs-atomic-gcc4+.h
+++ b/lib/ovs-atomic-gcc4+.h
@@ -39,7 +39,7 @@ typedef enum {
 memory_order_release,
 memory_order_acq_rel,
 memory_order_seq_cst
-} memory_order;
+} ovs_memory_order;
 
 #define IS_LOCKLESS_ATOMIC(OBJECT) (sizeof(OBJECT) <= sizeof(void *))
 
@@ -47,7 +47,7 @@ typedef enum {
 #define atomic_init(OBJECT, VALUE) (*(OBJECT) = (VALUE), (void) 0)
 
 static inline void
-atomic_thread_fence(memory_order order)
+atomic_thread_fence(ovs_memory_order order)
 {
 if (order != memory_order_relaxed) {
 __sync_synchronize();
@@ -55,7 +55,7 @@ atomic_thread_fence(memory_order order)
 }
 
 static inline void
-atomic_thread_fence_if_seq_cst(memory_order order)
+atomic_thread_fence_if_seq_cst(ovs_memory_order order)
 {
 if (order == memory_order_seq_cst) {
 __sync_synchronize();
@@ -63,7 +63,7 @@ atomic_thread_fence_if_seq_cst(memory_order order)
 }
 
 static inline void
-atomic_signal_fence(memory_order order)
+atomic_signal_fence(ovs_memory_order order)
 {
 if (order != memory_order_relaxed) {
 asm volatile("" : : : "memory");
@@ -168,7 +168,7 @@ typedef struct {
 
 static inline bool
 atomic_flag_test_and_set_explicit(volatile atomic_flag *object,
-  memory_order order)
+  ovs_memory_order order)
 {
 bool old;
 
@@ -188,7 +188,7 @@ atomic_flag_test_and_set_explicit(volatile atomic_flag 
*object,
 
 static inline void
 atomic_flag_clear_explicit(volatile atomic_flag *object,
-   memory_order order)
+   ovs_memory_order order)
 {
 /* __sync_lock_release() by itself is a release barrier.  For
  * anything else additional barrier may be needed. */
diff --git a/lib/ovs-atomic-gcc4.7+.h b/lib/ovs-atomic-gcc4.7+.h
index 4c197ebe0..e0a821bbe 100644
--- a/lib/ovs-atomic-gcc4.7+.h
+++ b/lib/ovs-atomic-gcc4.7+.h
@@ -28,7 +28,7 @@ typedef enum {
 memory_order_release = __ATOMIC_RELEASE,
 memory_order_acq_rel = __ATOMIC_ACQ_REL,
 memory_order_seq_cst = __ATOMIC_SEQ_CST
-} memory_order;
+} ovs_memory_order;
 
 #define ATOMIC_VAR_INIT(VALUE) (VALUE)
 #define atomic_init(OBJECT, VALUE) (*(OBJECT) = (VALUE), (void) 0)
diff --git a/lib/ovs-atomic-i586.h b/lib/ovs-atomic-i586.h
index 9a385ce84..238409d8b 100644
--- a/lib/ovs-atomic-i586.h
+++ b/lib/ovs-atomic-i586.h
@@ -106,7 +106,7 @@ typedef enum {
 memory_order_release,
 memory_order_acq_rel,
 memory_order_seq_cst
-} memory_order;
+} ovs_memory_order;
 
 #define ATOMIC_BOOL_LOCK_FREE 2
 #define ATOMIC_CHAR_LOCK_FREE 2
@@ -143,7 +143,7 @@ typedef enum {
  * without a compiler memory barrier.
  */
 static inline void
-atomic_compiler_barrier(memory_order order)
+atomic_compil

[ovs-dev] [PATCH ovs-dev, dpdk-latest 2/2] netdev-dpdk: Remove usage of RTE_ETH_DEV_CLOSE_REMOVE flag

[Eli Britstein]

Following DPDK commit [1], RTE_ETH_DEV_CLOSE_REMOVE flag is obsolete.
Behavior is equivalent to "always on". Remove the usage of that flag.

[1] 638d40ba9622 ("ethdev: remove old close behaviour")

Signed-off-by: Eli Britstein 
---
 lib/netdev-dpdk.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index c048aaa75..710bc86c9 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1447,7 +1447,6 @@ netdev_dpdk_destruct(struct netdev *netdev)
 struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
 struct rte_device *rte_dev;
 struct rte_eth_dev *eth_dev;
-bool remove_on_close;
 
 ovs_mutex_lock(&dpdk_mutex);
 
@@ -1459,9 +1458,6 @@ netdev_dpdk_destruct(struct netdev *netdev)
  * FIXME: avoid direct access to DPDK internal array rte_eth_devices.
  */
 eth_dev = &rte_eth_devices[dev->port_id];
-remove_on_close =
-eth_dev->data &&
-(eth_dev->data->dev_flags & RTE_ETH_DEV_CLOSE_REMOVE);
 rte_dev = eth_dev->device;
 
 /* Remove the eth device. */
@@ -1472,7 +1468,7 @@ netdev_dpdk_destruct(struct netdev *netdev)
  * are not supported), or if all the eth devices belonging to the rte
  * device are closed.
  */
-if (!remove_on_close || !netdev_dpdk_get_num_ports(rte_dev)) {
+if (!netdev_dpdk_get_num_ports(rte_dev)) {
 int ret = rte_dev_remove(rte_dev);
 
 if (ret < 0) {
-- 
2.28.0.546.g385c171

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

[ovs-dev] [PATCH ovs-dev, dpdk-latest 0/2] DPDK: adaptations to 20.11

[Eli Britstein]

Adaptations to compile against DPDK 20.11.

Eli Britstein (2):
  ovs-atomic: Rename memory_order -> ovs_memory_order
  netdev-dpdk: Remove usage of RTE_ETH_DEV_CLOSE_REMOVE flag

 lib/netdev-dpdk.c |  6 +-
 lib/ovs-atomic-clang.h|  2 +-
 lib/ovs-atomic-flag-gcc4.7+.h |  4 ++--
 lib/ovs-atomic-gcc4+.h| 12 ++--
 lib/ovs-atomic-gcc4.7+.h  |  2 +-
 lib/ovs-atomic-i586.h |  8 
 lib/ovs-atomic-msvc.h |  8 
 lib/ovs-atomic-pthreads.h | 10 +-
 lib/ovs-atomic-x86_64.h   |  8 
 lib/ovs-atomic.h  | 34 +-
 lib/ovs-rcu.h | 10 +-
 11 files changed, 50 insertions(+), 54 deletions(-)

-- 
2.28.0.546.g385c171

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

Re: [ovs-dev] [PATCH ovn] vtep-controller: extract mac address in building umr

[Numan Siddique]

On Mon, Oct 12, 2020 at 2:46 PM  wrote:
>
> From: Vladislav Odintsov 

Thanks. I applied this patch to master, branch-20.09 and branch-20.06


Numan

>
> port_binding_rec->mac array contains items with addresses (MAC, IP, ...)
> delimited by whitespace.
> In Unicast_Macs_Remote record should be only one mac address.
> Now mac address extracts from port_binding_rec->mac[i].
>
> Submitted-at: https://github.com/ovn-org/ovn/pull/48
> Signed-off-by: Vladislav Odintsov 
> Signed-off-by: Numan Siddique 
> ---
>  controller-vtep/vtep.c   | 12 ++--
>  tests/ovn-controller-vtep.at | 16 
>  2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/controller-vtep/vtep.c b/controller-vtep/vtep.c
> index e2baca5a7..5538c7d6a 100644
> --- a/controller-vtep/vtep.c
> +++ b/controller-vtep/vtep.c
> @@ -20,6 +20,7 @@
>  #include "lib/hash.h"
>  #include "openvswitch/hmap.h"
>  #include "openvswitch/shash.h"
> +#include "lib/ovn-util.h"
>  #include "lib/smap.h"
>  #include "lib/sset.h"
>  #include "lib/util.h"
> @@ -366,7 +367,13 @@ vtep_macs_run(struct ovsdb_idl_txn *vtep_idl_txn, struct 
> shash *ucast_macs_rmts,
>  for (i = 0; i < port_binding_rec->n_mac; i++) {
>  const struct vteprec_ucast_macs_remote *umr;
>  const struct sbrec_port_binding *conflict;
> -char *mac = port_binding_rec->mac[i];
> +struct lport_addresses laddrs;
> +
> +if (!extract_lsp_addresses(port_binding_rec->mac[i], &laddrs)) {
> +continue;
> +};
> +
> +char *mac = laddrs.ea_s;
>
>  /* Checks for duplicate MAC in the same vtep logical switch. */
>  conflict = shash_find_data(&ls_node->added_macs, mac);
> @@ -384,7 +391,7 @@ vtep_macs_run(struct ovsdb_idl_txn *vtep_idl_txn, struct 
> shash *ucast_macs_rmts,
>  tnl_key);
>  umr = shash_find_data(ucast_macs_rmts, mac_ip_tnlkey);
>  /* If finds the 'umr' entry for the mac, ip, and tnl_key, deletes
> - * the entry from shash so that it is not gargage collected.
> + * the entry from shash so that it is not garbage collected.
>   *
>   * If not found, creates a new 'umr' entry. */
>  if (umr && umr->logical_switch == ls_node->vtep_ls) {
> @@ -395,6 +402,7 @@ vtep_macs_run(struct ovsdb_idl_txn *vtep_idl_txn, struct 
> shash *ucast_macs_rmts,
>  vteprec_ucast_macs_remote_set_locator(new_umr, pl);
>  }
>  free(mac_ip_tnlkey);
> +destroy_lport_addresses(&laddrs);
>  }
>  }
>
> diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at
> index 3092dd5ca..f0598fa4d 100644
> --- a/tests/ovn-controller-vtep.at
> +++ b/tests/ovn-controller-vtep.at
> @@ -383,6 +383,22 @@ AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote 
> | cut -d ':' -f2- | tr -
>  "f0:ab:cd:ef:01:03"
>  ])
>
> +# adds MAC-IP pair to logical switch port.
> +AT_CHECK([ovn-nbctl lsp-set-addresses vif0 "f0:ab:cd:ef:01:04 192.168.0.1"])
> +OVS_WAIT_UNTIL([test -n "`vtep-ctl list Ucast_Macs_Remote | grep 
> 'f0:ab:cd:ef:01:04'`"])
> +AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':' -f2- | 
> tr -d ' ' | sort], [0], [dnl
> +"f0:ab:cd:ef:01:04"
> +])
> +
> +# adds another MAC-IP pair to logical switch port.
> +AT_CHECK([ovn-nbctl lsp-set-addresses vif0 "f0:ab:cd:ef:01:04 192.168.0.1" 
> "f0:ab:cd:ef:01:05 192.168.0.2"])
> +OVS_WAIT_UNTIL([test -n "`vtep-ctl list Ucast_Macs_Remote | grep 
> 'f0:ab:cd:ef:01:05'`"])
> +AT_CHECK([vtep-ctl --columns=MAC list Ucast_Macs_Remote | cut -d ':' -f2- | 
> tr -d ' ' | sort], [0], [dnl
> +
> +"f0:ab:cd:ef:01:04"
> +"f0:ab:cd:ef:01:05"
> +])
> +
>  # removes one mac to logical switch port.
>  AT_CHECK([ovn-nbctl lsp-set-addresses vif0 f0:ab:cd:ef:01:03])
>  OVS_WAIT_UNTIL([test -z "`vtep-ctl --columns=MAC list Ucast_Macs_Remote | 
> grep 02`"])
> --
> 2.26.2
>
> ___
> 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 V2 1/1] netdev-offload-dpdk: Preserve HW statistics for modified flows

[Eli Britstein]

In case of a flow modification, preserve the HW statistics of the old HW
flow to the new one.

Fixes: 3c7330ebf036 ("netdev-offload-dpdk: Support offload of output action.")
Signed-off-by: Eli Britstein 
Reviewed-by: Gaetan Rivet 
---
 lib/netdev-offload-dpdk.c | 33 ++---
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 5b632bac4..dadd8f253 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -78,7 +78,7 @@ ufid_to_rte_flow_data_find(const ovs_u128 *ufid)
 return NULL;
 }
 
-static inline void
+static inline struct ufid_to_rte_flow_data *
 ufid_to_rte_flow_associate(const ovs_u128 *ufid,
struct rte_flow *rte_flow, bool actions_offloaded)
 {
@@ -103,6 +103,7 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid,
 
 cmap_insert(&ufid_to_rte_flow,
 CONST_CAST(struct cmap_node *, &data->node), hash);
+return data;
 }
 
 static inline void
@@ -1420,7 +1421,7 @@ out:
 return flow;
 }
 
-static int
+static struct ufid_to_rte_flow_data *
 netdev_offload_dpdk_add_flow(struct netdev *netdev,
  struct match *match,
  struct nlattr *nl_actions,
@@ -1429,12 +1430,11 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
  struct offload_info *info)
 {
 struct flow_patterns patterns = { .items = NULL, .cnt = 0 };
+struct ufid_to_rte_flow_data *flows_data = NULL;
 bool actions_offloaded = true;
 struct rte_flow *flow;
-int ret = 0;
 
-ret = parse_flow_match(&patterns, match);
-if (ret) {
+if (parse_flow_match(&patterns, match)) {
 VLOG_DBG_RL(&rl, "%s: matches of ufid "UUID_FMT" are not supported",
 netdev_get_name(netdev), UUID_ARGS((struct uuid *) ufid));
 goto out;
@@ -1452,16 +1452,15 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
 }
 
 if (!flow) {
-ret = -1;
 goto out;
 }
-ufid_to_rte_flow_associate(ufid, flow, actions_offloaded);
+flows_data = ufid_to_rte_flow_associate(ufid, flow, actions_offloaded);
 VLOG_DBG("%s: installed flow %p by ufid "UUID_FMT,
  netdev_get_name(netdev), flow, UUID_ARGS((struct uuid *)ufid));
 
 out:
 free_flow_patterns(&patterns);
-return ret;
+return flows_data;
 }
 
 static int
@@ -1495,14 +1494,19 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, 
struct match *match,
  struct dpif_flow_stats *stats)
 {
 struct ufid_to_rte_flow_data *rte_flow_data;
+struct dpif_flow_stats old_stats;
+bool modification = false;
 int ret;
 
 /*
  * If an old rte_flow exists, it means it's a flow modification.
  * Here destroy the old rte flow first before adding a new one.
+ * Keep the stats for the newly created rule.
  */
 rte_flow_data = ufid_to_rte_flow_data_find(ufid);
 if (rte_flow_data && rte_flow_data->rte_flow) {
+old_stats = rte_flow_data->stats;
+modification = true;
 ret = netdev_offload_dpdk_destroy_flow(netdev, ufid,
rte_flow_data->rte_flow);
 if (ret < 0) {
@@ -1510,11 +1514,18 @@ netdev_offload_dpdk_flow_put(struct netdev *netdev, 
struct match *match,
 }
 }
 
+rte_flow_data = netdev_offload_dpdk_add_flow(netdev, match, actions,
+ actions_len, ufid, info);
+if (!rte_flow_data) {
+return -1;
+}
+if (modification) {
+rte_flow_data->stats = old_stats;
+}
 if (stats) {
-memset(stats, 0, sizeof *stats);
+*stats = rte_flow_data->stats;
 }
-return netdev_offload_dpdk_add_flow(netdev, match, actions,
-actions_len, ufid, info);
+return 0;
 }
 
 static int
-- 
2.26.2.1730.g385c171

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

Re: [ovs-dev] [PATCH ovs-dev, dpdk-latest 2/2] netdev-dpdk: Remove usage of RTE_ETH_DEV_CLOSE_REMOVE flag

[Andrew Rybchenko]

On 10/12/20 5:04 PM, Eli Britstein wrote:
> Following DPDK commit [1], RTE_ETH_DEV_CLOSE_REMOVE flag is obsolete.
> Behavior is equivalent to "always on". Remove the usage of that flag.
> 
> [1] 638d40ba9622 ("ethdev: remove old close behaviour")
> 
> Signed-off-by: Eli Britstein 
> ---
>  lib/netdev-dpdk.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index c048aaa75..710bc86c9 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1447,7 +1447,6 @@ netdev_dpdk_destruct(struct netdev *netdev)
>  struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>  struct rte_device *rte_dev;
>  struct rte_eth_dev *eth_dev;
> -bool remove_on_close;
>  
>  ovs_mutex_lock(&dpdk_mutex);
>  
> @@ -1459,9 +1458,6 @@ netdev_dpdk_destruct(struct netdev *netdev)
>   * FIXME: avoid direct access to DPDK internal array rte_eth_devices.
>   */
>  eth_dev = &rte_eth_devices[dev->port_id];
> -remove_on_close =
> -eth_dev->data &&
> -(eth_dev->data->dev_flags & RTE_ETH_DEV_CLOSE_REMOVE);
>  rte_dev = eth_dev->device;
>  
>  /* Remove the eth device. */
> @@ -1472,7 +1468,7 @@ netdev_dpdk_destruct(struct netdev *netdev)
>   * are not supported), or if all the eth devices belonging to the rte
>   * device are closed.
>   */

Description above should be fixed as well

> -if (!remove_on_close || !netdev_dpdk_get_num_ports(rte_dev)) {
> +if (!netdev_dpdk_get_num_ports(rte_dev)) {
>  int ret = rte_dev_remove(rte_dev);
>  
>  if (ret < 0) {
> 

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

[ovs-dev] [PATCH v2] ovsdb-idl: Fix *_is_new() IDL functions

[Mark Gray]

Currently all functions of the type *_is_new() always return
'false'. This patch resolves this issue by using the
'OVSDB_IDL_CHANGE_INSERT' 'change_seqno' instead of the
'OVSDB_IDL_CHANGE_MODIFY' 'change_seqno' to determine if a row
is new and by resetting the 'OVSDB_IDL_CHANGE_INSERT'
'change_seqno' on clear.

Further to this, the code is also updated to match the following
behaviour:

When a row is inserted, the 'OVSDB_IDL_CHANGE_INSERT'
'change_seqno' is updated to match the new database
change_seqno. The 'OVSDB_IDL_CHANGE_MODIFY' 'change_seqno'
is not set for inserted rows (only for updated rows).

At the end of a run, ovsdb_idl_db_track_clear() should be
called to clear all tracking information, this includes
resetting all row 'change_seqno' to zero. This will ensure
that subsequent runs will not see a previously 'new' row.

add_tracked_change_for_references() is updated to only
track rows that reference the current row.

Update ovsdb_idl_txn_insert() to also set this flag when
inserting new rows locally.

Also, update unit tests in order to test the *_is_new(),
*_is_delete() functions.

Suggested-by: Dumitru Ceara 
Reported-at: https://bugzilla.redhat.com/1883562
Fixes: ca545a787ac0 ("ovsdb-idl.c: Increase seqno for change-tracking of table 
references.")
Signed-off-by: Mark Gray 
---
 lib/ovsdb-idl.c | 44 
 ovsdb/ovsdb-idlc.in |  2 +-
 tests/ovsdb-idl.at  |  5 -
 tests/test-ovsdb.c  | 32 
 4 files changed, 61 insertions(+), 22 deletions(-)

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index d8f221ca6073..4750d7c9018c 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -1959,6 +1959,11 @@ ovsdb_idl_db_track_clear(struct ovsdb_idl_db *db)
 free(row->updated);
 row->updated = NULL;
 }
+
+row->change_seqno[OVSDB_IDL_CHANGE_INSERT] =
+row->change_seqno[OVSDB_IDL_CHANGE_MODIFY] =
+row->change_seqno[OVSDB_IDL_CHANGE_DELETE] = 0;
+
 ovs_list_remove(&row->track_node);
 ovs_list_init(&row->track_node);
 if (ovsdb_idl_row_is_orphan(row) && row->tracked_old_datum) {
@@ -2684,24 +2689,27 @@ ovsdb_idl_process_update2(struct ovsdb_idl_table *table,
 return OVSDB_IDL_UPDATE_DB_CHANGED;
 }
 
-/* Recursively add rows to tracked change lists for current row
- * and the rows that reference this row. */
+/* Recursively add rows to tracked change lists for all rows that reference
+   'row'. */
 static void
 add_tracked_change_for_references(struct ovsdb_idl_row *row)
 {
-if (ovs_list_is_empty(&row->track_node) &&
-ovsdb_idl_track_is_set(row->table)) {
-ovs_list_push_back(&row->table->track_list,
-   &row->track_node);
-row->change_seqno[OVSDB_IDL_CHANGE_MODIFY]
-= row->table->change_seqno[OVSDB_IDL_CHANGE_MODIFY]
-= row->table->db->change_seqno + 1;
-
 const struct ovsdb_idl_arc *arc;
 LIST_FOR_EACH (arc, dst_node, &row->dst_arcs) {
-add_tracked_change_for_references(arc->src);
+struct ovsdb_idl_row *ref = arc->src;
+
+if (ovs_list_is_empty(&ref->track_node) &&
+ovsdb_idl_track_is_set(ref->table)) {
+ovs_list_push_back(&ref->table->track_list,
+   &ref->track_node);
+
+ref->change_seqno[OVSDB_IDL_CHANGE_MODIFY]
+= ref->table->change_seqno[OVSDB_IDL_CHANGE_MODIFY]
+= ref->table->db->change_seqno + 1;
+
+add_tracked_change_for_references(ref);
+}
 }
-}
 }
 
 
@@ -2767,7 +2775,14 @@ ovsdb_idl_row_change__(struct ovsdb_idl_row *row, const 
struct json *row_json,
 row->change_seqno[change]
 = row->table->change_seqno[change]
 = row->table->db->change_seqno + 1;
+
 if (table->modes[column_idx] & OVSDB_IDL_TRACK) {
+if (ovs_list_is_empty(&row->track_node) &&
+ovsdb_idl_track_is_set(row->table)) {
+ovs_list_push_back(&row->table->track_list,
+   &row->track_node);
+}
+
 add_tracked_change_for_references(row);
 if (!row->updated) {
 row->updated = bitmap_allocate(class->n_columns);
@@ -4843,6 +4858,11 @@ ovsdb_idl_txn_insert(struct ovsdb_idl_txn *txn,
 hmap_insert(&row->table->rows, &row->hmap_node, uuid_hash(&row->uuid));
 hmap_insert(&txn->txn_rows, &row->txn_node, uuid_hash(&row->uuid));
 ovsdb_idl_add_to_indexes(row);
+
+row->change_seqno[OVSDB_IDL_CHANGE_INSERT]
+= row->table->change_seqno[OVSDB_IDL_CHANGE_INSERT]
+= row->table->db->change_seqno 

Re: [ovs-dev] [PATCH] ovsdb-idl: Fix *_is_new() IDL functions

[Mark Gray]

On 10/10/2020 16:42, Mark Gray wrote:
> On 08/10/2020 19:57, Dumitru Ceara wrote:
>> On 10/2/20 1:13 AM, Han Zhou wrote:
>>>
>>>
>>> On Thu, Oct 1, 2020 at 2:30 AM Mark Gray >> > wrote:

 On 30/09/2020 21:04, Han Zhou wrote:
> On Wed, Sep 30, 2020 at 2:21 AM Dumitru Ceara >> > wrote:
>>
>> On 9/29/20 8:34 PM, Mark Gray wrote:
>>> Currently all functions of the type *_is_new() always return
>>> 'false'. This patch resolves this issue by using the
>>> 'OVSDB_IDL_CHANGE_INSERT' 'change_seqno' instead of the
>>> 'OVSDB_IDL_CHANGE_MODIFY' 'change_seqno' to determine if a row
>>> is new and by resetting the 'OVSDB_IDL_CHANGE_INSERT'
>>> 'change_seqno' on clear.
>>>
>>> Further to this, the code is also updated to match this
>>> behaviour:
>>>
>>> When a row is inserted, the 'OVSDB_IDL_CHANGE_INSERT'
>>> 'change_seqno' is updated to match the new database
>>> change_seqno. The 'OVSDB_IDL_CHANGE_MODIFY' 'change_seqno'
>>> is not set for inserted rows (only for updated rows).
>>>
>>> At the end of a run, ovsdb_idl_db_track_clear() should be
>>> called to clear all tracking information, this includes
>>> resetting all row 'change_seqno' to zero. This will ensure
>>> that subsequent runs will not see a previously 'new' row.
>>>
>>> add_tracked_change_for_references() is updated to only
>>> track rows that reference the current row.
>>>
>>> Signed-off-by: Mark Gray >> >
>>> Suggested-by: Dumitru Ceara >> >
>>> Reported-at: https://bugzilla.redhat.com/1883562
>>
>> Hi Mark,
>>
>> Thanks for working on this, it definitely looks like a nasty bug to me!
>>
>> We were lucky to not hit it in ovn-controller before. That's just
>> because all the "update" operations were handled there as "delete" +
>> "insert" but that is not mandatory and might change.>>
>
> Agree. Thanks for this critical finding! It was my bad - commit
>>> ca545a787ac
> introduced this bug, which was fixing another bug related to is_new. I
> should have added test cases to avoid the miss. Apologize.

 No worries, I will reference this commit in the commit message.
>
> For this fix, I don't understand why we need to check
> OVSDB_IDL_CHANGE_INSERT. I think the root cause is that in
> add_tracked_change_for_references() it updated the
>>> OVSDB_IDL_CHANGE_MODIFY
> seqno for the initial inserted *new* row, while it shouldn't. So
>>> should the
> fix only include the changes in add_tracked_change_for_references() and
> ovsdb_idl_row_change__()? Why do we need to change the implementation of
> _is_new() itself? Could you explain?

 In the case of a newly inserted row, the change_seqno triplet will still
 be {X, 0, 0}. It will remain as such until the row is modified, at which
 point it will become {X, Y, 0}. This means that the _*is_new() function
 will continue to return true until the row is modified which is not
 correct behaviour.
>>>
>>> Hmm, this is not a problem because if a row inserted in a previous run
>>> is not modified/deleted in the current run, it won't be added to
>>> tracking so no one would call _is_new() for the row. However, there will
>>> be a problem when a row is deleted (see below).
>>>
 If we reset the change_seqno triplet on each run, it
 will still not work. In the end, it is probably better to use the
 OVSDB_IDL_CHANGE_INSERT element to track this.

>>> Indeed, we should use OVSDB_IDL_CHANGE_INSERT to check _is_new() because
>>> if row is deleted without any modification after insertion, the
>>> OVSDB_IDL_CHANGE_MODIFY seqno would still be zero when it is being
>>> deleted. In that case calling the _is_new() function would return true
>>> for a deleted row, which is wrong. So, your change LGTM.
>>>
 On a seperate point, I don't expect add_tracked_change_for_references()
 will ever get called on insert as nothing will be referenced at that
 stage? Am I correct in my understanding?

>>> Yes, I think so. More accurately, it will get called once on the
>>> inserted row but will not trigger any recursive calls.
>>>
>>> ---
>>>  lib/ovsdb-idl.c     | 39 +++
>>>  ovsdb/ovsdb-idlc.in  |  2 +-
>>>  2 files changed, 28 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
>>> index d8f221ca6..3cfd73871 100644
>>> --- a/lib/ovsdb-idl.c
>>> +++ b/lib/ovsdb-idl.c
>>> @@ -1959,6 +1959,11 @@ ovsdb_idl_db_track_clear(struct
>>> ovsdb_idl_db *db)
>>>                      free(row->updated);
>>>                      row->updated = NULL;
>>>                  }
>>> +
>>> +                row->change_seqno[OVSDB_IDL_CHANGE_INSERT] =

[ovs-dev] [PATCH ovn] Correct to OVN, not OVS in README.rst.

[numans]

From: Robin Brämer 

Submitted-at: https://github.com/ovn-org/ovn/pull/57
Signed-off-by: Robin Brämer 
Signed-off-by: Numan Siddique 
---
 README.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/README.rst b/README.rst
index 07174ba57..74ae22f41 100644
--- a/README.rst
+++ b/README.rst
@@ -53,7 +53,7 @@ What other documentation is available?
 .. TODO(stephenfin): Update with a link to the hosting site of the docs, once
we know where that is
 
-To install Open vSwitch on a regular Linux or FreeBSD host, please read the
+To install OVN on a regular Linux or FreeBSD host, please read the
 `installation guide `__. For specifics
 around installation on a specific platform, refer to one of the `other
 installation guides `__
-- 
2.26.2

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

[ovs-dev] [PATCH ovs-dev, dpdk-latest V2 1/2] ovs-atomic: Rename memory_order -> ovs_memory_order

[Eli Britstein]

DPDK commit [1] uses function variables named "memory_order".
Compilation fails with:
error: declaration of 'memory_order' shadows a global declaration
[-Werror=shadow]
 rte_atomic_thread_fence(int memory_order)
Rename enum memory_order to ovs_memory_order to avoid that conflict.

[1] 672a15056380 ("eal: add wrapper for C11 atomic thread fence")

Signed-off-by: Eli Britstein 
---
 lib/ovs-atomic-clang.h|  2 +-
 lib/ovs-atomic-flag-gcc4.7+.h |  4 ++--
 lib/ovs-atomic-gcc4+.h| 12 ++--
 lib/ovs-atomic-gcc4.7+.h  |  2 +-
 lib/ovs-atomic-i586.h |  8 
 lib/ovs-atomic-msvc.h |  8 
 lib/ovs-atomic-pthreads.h | 10 +-
 lib/ovs-atomic-x86_64.h   |  8 
 lib/ovs-atomic.h  | 34 +-
 lib/ovs-rcu.h | 10 +-
 10 files changed, 49 insertions(+), 49 deletions(-)

diff --git a/lib/ovs-atomic-clang.h b/lib/ovs-atomic-clang.h
index 34cc2faa7..4ff44f69b 100644
--- a/lib/ovs-atomic-clang.h
+++ b/lib/ovs-atomic-clang.h
@@ -36,7 +36,7 @@ typedef enum {
 memory_order_release = 3,
 memory_order_acq_rel = 4,
 memory_order_seq_cst = 5
-} memory_order;
+} ovs_memory_order;
 
 #define atomic_thread_fence(ORDER) __c11_atomic_thread_fence(ORDER)
 #define atomic_signal_fence(ORDER) __c11_atomic_signal_fence(ORDER)
diff --git a/lib/ovs-atomic-flag-gcc4.7+.h b/lib/ovs-atomic-flag-gcc4.7+.h
index 49cd5d20f..a959ca496 100644
--- a/lib/ovs-atomic-flag-gcc4.7+.h
+++ b/lib/ovs-atomic-flag-gcc4.7+.h
@@ -28,7 +28,7 @@ typedef struct {
 
 static inline bool
 atomic_flag_test_and_set_explicit(volatile atomic_flag *object,
-  memory_order order)
+  ovs_memory_order order)
 {
 return __atomic_test_and_set(&object->b, order);
 }
@@ -40,7 +40,7 @@ atomic_flag_test_and_set(volatile atomic_flag *object)
 }
 
 static inline void
-atomic_flag_clear_explicit(volatile atomic_flag *object, memory_order order)
+atomic_flag_clear_explicit(volatile atomic_flag *object, ovs_memory_order 
order)
 {
 __atomic_clear(object, order);
 }
diff --git a/lib/ovs-atomic-gcc4+.h b/lib/ovs-atomic-gcc4+.h
index 25bcf20a0..b3e73d004 100644
--- a/lib/ovs-atomic-gcc4+.h
+++ b/lib/ovs-atomic-gcc4+.h
@@ -39,7 +39,7 @@ typedef enum {
 memory_order_release,
 memory_order_acq_rel,
 memory_order_seq_cst
-} memory_order;
+} ovs_memory_order;
 
 #define IS_LOCKLESS_ATOMIC(OBJECT) (sizeof(OBJECT) <= sizeof(void *))
 
@@ -47,7 +47,7 @@ typedef enum {
 #define atomic_init(OBJECT, VALUE) (*(OBJECT) = (VALUE), (void) 0)
 
 static inline void
-atomic_thread_fence(memory_order order)
+atomic_thread_fence(ovs_memory_order order)
 {
 if (order != memory_order_relaxed) {
 __sync_synchronize();
@@ -55,7 +55,7 @@ atomic_thread_fence(memory_order order)
 }
 
 static inline void
-atomic_thread_fence_if_seq_cst(memory_order order)
+atomic_thread_fence_if_seq_cst(ovs_memory_order order)
 {
 if (order == memory_order_seq_cst) {
 __sync_synchronize();
@@ -63,7 +63,7 @@ atomic_thread_fence_if_seq_cst(memory_order order)
 }
 
 static inline void
-atomic_signal_fence(memory_order order)
+atomic_signal_fence(ovs_memory_order order)
 {
 if (order != memory_order_relaxed) {
 asm volatile("" : : : "memory");
@@ -168,7 +168,7 @@ typedef struct {
 
 static inline bool
 atomic_flag_test_and_set_explicit(volatile atomic_flag *object,
-  memory_order order)
+  ovs_memory_order order)
 {
 bool old;
 
@@ -188,7 +188,7 @@ atomic_flag_test_and_set_explicit(volatile atomic_flag 
*object,
 
 static inline void
 atomic_flag_clear_explicit(volatile atomic_flag *object,
-   memory_order order)
+   ovs_memory_order order)
 {
 /* __sync_lock_release() by itself is a release barrier.  For
  * anything else additional barrier may be needed. */
diff --git a/lib/ovs-atomic-gcc4.7+.h b/lib/ovs-atomic-gcc4.7+.h
index 4c197ebe0..e0a821bbe 100644
--- a/lib/ovs-atomic-gcc4.7+.h
+++ b/lib/ovs-atomic-gcc4.7+.h
@@ -28,7 +28,7 @@ typedef enum {
 memory_order_release = __ATOMIC_RELEASE,
 memory_order_acq_rel = __ATOMIC_ACQ_REL,
 memory_order_seq_cst = __ATOMIC_SEQ_CST
-} memory_order;
+} ovs_memory_order;
 
 #define ATOMIC_VAR_INIT(VALUE) (VALUE)
 #define atomic_init(OBJECT, VALUE) (*(OBJECT) = (VALUE), (void) 0)
diff --git a/lib/ovs-atomic-i586.h b/lib/ovs-atomic-i586.h
index 9a385ce84..238409d8b 100644
--- a/lib/ovs-atomic-i586.h
+++ b/lib/ovs-atomic-i586.h
@@ -106,7 +106,7 @@ typedef enum {
 memory_order_release,
 memory_order_acq_rel,
 memory_order_seq_cst
-} memory_order;
+} ovs_memory_order;
 
 #define ATOMIC_BOOL_LOCK_FREE 2
 #define ATOMIC_CHAR_LOCK_FREE 2
@@ -143,7 +143,7 @@ typedef enum {
  * without a compiler memory barrier.
  */
 static inline void
-atomic_compiler_barrier(memory_order order)
+atomic_compil

[ovs-dev] [PATCH ovs-dev, dpdk-latest V2 2/2] netdev-dpdk: Remove usage of RTE_ETH_DEV_CLOSE_REMOVE flag

[Eli Britstein]

Following DPDK commit [1], RTE_ETH_DEV_CLOSE_REMOVE flag is obsolete.
Behavior is equivalent to "always on". Remove the usage of that flag.

[1] 638d40ba9622 ("ethdev: remove old close behaviour")

Signed-off-by: Eli Britstein 
---
 lib/netdev-dpdk.c | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index c048aaa75..76eec449a 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1447,7 +1447,6 @@ netdev_dpdk_destruct(struct netdev *netdev)
 struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
 struct rte_device *rte_dev;
 struct rte_eth_dev *eth_dev;
-bool remove_on_close;
 
 ovs_mutex_lock(&dpdk_mutex);
 
@@ -1459,20 +1458,15 @@ netdev_dpdk_destruct(struct netdev *netdev)
  * FIXME: avoid direct access to DPDK internal array rte_eth_devices.
  */
 eth_dev = &rte_eth_devices[dev->port_id];
-remove_on_close =
-eth_dev->data &&
-(eth_dev->data->dev_flags & RTE_ETH_DEV_CLOSE_REMOVE);
 rte_dev = eth_dev->device;
 
 /* Remove the eth device. */
 rte_eth_dev_close(dev->port_id);
 
-/* Remove this rte device and all its eth devices if flag
- * RTE_ETH_DEV_CLOSE_REMOVE is not supported (which means representors
- * are not supported), or if all the eth devices belonging to the rte
- * device are closed.
+/* Remove this rte device and all its eth devices if all the eth
+ * devices belonging to the rte device are closed.
  */
-if (!remove_on_close || !netdev_dpdk_get_num_ports(rte_dev)) {
+if (!netdev_dpdk_get_num_ports(rte_dev)) {
 int ret = rte_dev_remove(rte_dev);
 
 if (ret < 0) {
-- 
2.28.0.546.g385c171

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

Re: [ovs-dev] [PATCH ovs-dev, dpdk-latest 1/2] ovs-atomic: Rename memory_order -> ovs_memory_order

[Gaëtan Rivet]

On 12/10/20 14:04 +, Eli Britstein wrote:
> DPDK commit [1] uses function variables named "memory_order".
> Compilation fails with:
> error: declaration of 'memory_order' shadows a global declaration
> [-Werror=shadow]
>  rte_atomic_thread_fence(int memory_order)
> Rename enum memory_order to ovs_memory_order to avoid that conflict.
> 

Hi Eli,

The C11 standard section 7.17.1.4 defines 'memory_order' as the
"enumerated type whose enumerators identify memory ordering constraints".
I think in this case this is a DPDK bug. Its API should be compatible
with the C standard.

> [1] 672a15056380 ("eal: add wrapper for C11 atomic thread fence")
> 
> Signed-off-by: Eli Britstein 
> ---

Regards,
-- 
Gaëtan
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH ovs-dev, dpdk-latest V2 0/2] DPDK: adaptations to 20.11

[Eli Britstein]

Adaptations to compile against DPDK 20.11.

v2-v1:
- Amend comment referring CLOSE flag.

Eli Britstein (2):
  ovs-atomic: Rename memory_order -> ovs_memory_order
  netdev-dpdk: Remove usage of RTE_ETH_DEV_CLOSE_REMOVE flag

 lib/netdev-dpdk.c | 12 +++-
 lib/ovs-atomic-clang.h|  2 +-
 lib/ovs-atomic-flag-gcc4.7+.h |  4 ++--
 lib/ovs-atomic-gcc4+.h| 12 ++--
 lib/ovs-atomic-gcc4.7+.h  |  2 +-
 lib/ovs-atomic-i586.h |  8 
 lib/ovs-atomic-msvc.h |  8 
 lib/ovs-atomic-pthreads.h | 10 +-
 lib/ovs-atomic-x86_64.h   |  8 
 lib/ovs-atomic.h  | 34 +-
 lib/ovs-rcu.h | 10 +-
 11 files changed, 52 insertions(+), 58 deletions(-)

-- 
2.28.0.546.g385c171

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

Re: [ovs-dev] [PATCH ovn] Correct to OVN, not OVS in README.rst.

[Numan Siddique]

On Mon, Oct 12, 2020 at 8:04 PM  wrote:
>
> From: Robin Brämer 
>

Thanks for the PR. I applied this patch to master.

Numan

> Submitted-at: https://github.com/ovn-org/ovn/pull/57
> Signed-off-by: Robin Brämer 
> Signed-off-by: Numan Siddique 
> ---
>  README.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/README.rst b/README.rst
> index 07174ba57..74ae22f41 100644
> --- a/README.rst
> +++ b/README.rst
> @@ -53,7 +53,7 @@ What other documentation is available?
>  .. TODO(stephenfin): Update with a link to the hosting site of the docs, once
> we know where that is
>
> -To install Open vSwitch on a regular Linux or FreeBSD host, please read the
> +To install OVN on a regular Linux or FreeBSD host, please read the
>  `installation guide `__. For 
> specifics
>  around installation on a specific platform, refer to one of the `other
>  installation guides `__
> --
> 2.26.2
>
> ___
> 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] [RFC PATCH ovn 0/3] Unit Testing in OVN

[Mark Gray]

On 09/10/2020 18:23, Ilya Maximets wrote:
> On 10/9/20 5:29 PM, Mark Michelson wrote:
>> OVN has had a test framework for as long as I've been working on the
>> project. The test framework is designed for performing functional tests
>> of OVN. That is, with the entirety of OVN up and running, we can provide
>> configuration and test data and ensure that OVN does with that data what
>> we expect. This is 100% a good thing and has helped us to detect lots of
>> bugs before they can actually be merged in.
>>
>> What's missing, though, are smaller-scale unit tests. As an example, if
>> I wanted to test ovn-northd's IPAM code, I would need to start up
>> ovn-northd, create a logical switch, configure that logical switch to
>> use IPAM, and then create logical switch ports to exercise the IPAM
>> code. This can be overkill if my only goal is to ensure that IPAM's
>> algorithm for selecting the next IP address is correct.
>>
>> This patch series proposes a unit test framework for OVN.
>>
>> If you want to run the unit tests, you can do so in a couple of ways.
>>
>> 1) Within the testsuite.
>>./configure --with-ovs-source=/path/to/ovs --enable-unit-tests
>>make check TESTSUITEFLAGS="-k unit"
>>
>> 2) One-off from the command line
>>./configure --with-ovs-source=/path/to/ovs --enable-unit-tests
>>make sandbox
>>ovn-appctl -t ovn-northd unit-test 
>>
>> Some notes on this patch series
>> 1) Patch 1 is the most important one in the series. This is an RFC
>> because I'm trying to find out if the unit test framework itself is
>> good. The refactoring in patch 2 and the unit tests added in patch 3 are
>> meant to illustrate examples of the framework. They do not necessarily
>> need to be merged as-is. Feel free to comment on them if you'd like,
>> though.
>> 2) Ideally, new unit tests could be added to the testsuite via a script.
>> They've been added manually in this patch series.
>> 3) This patch series only adds unit test capabilities to ovn-northd.
>> However, the patch series we actually merge should add unit test
>> capabilities to ovn-controller as well.
> 
> That is an interesting approach, thanks for working n this!

+1

> This is very similar to ovstest framework (tests/ovstest.{c,h}), however
> there are few differences:
> 
> - ovstest is a separate executable and tests are implemented as a separate
>   source files.  So, usage is 'ovstest mytest' instead of
>   'ovn-appctl -t ovn-northd mytest'.  Upside of ovstest is that it doesn't
>   require test code being integrated to the main executable.  Downside
>   is that it requires functions being in a separate library that could
>   be included.  Not sure if this is a downside though, as it forces
>   developers to avoid huge source files with static functions.
> 

I don't think this is a downside. If the code was broken down into
smaller modules, we could do exactly what you are suggesting Ilya. If we
enable the ability to test code like this, it may reduce the motivation
for modularization of the code :)

However, if we did go down this path, the other way that I have seen
this done (while minimizing the amount test code leaking into production
code) is to include .c files in your test code.

#include "northd/ovn-northd.c"

That way the test code can access private functions and structure in
the code under test.


> - current implementation of ovn unit test framework doesn't allow passing
>   datasets via cmdline, while each unit test with ovstest could be called
>   with different cmdline arguments. This makes it easier to test, as you
>   don't need to rebuild binaries under test.
> 
> - since ovn unit test framework implements tests inside main source files
>   these files will, ideally, grow significantly.  ovn-northd is already
>   13K lines of code, so this, probably, is not a very good thing.
> 
> You also did a good job decoupling init_ipam_info out of northd stuff,
> so it basically self-sufficient now.  What my suggestion here is to take
> one more step forward and move this function to its own library, e.g.
> move all the ipam related code that could be decoupled to lib/ipam.c
> and lib/ipam.h.  This might be a good thing to do not only for unit
> testing purposes, but just as a generic refactoring step that will
> reduce code complexity and increase readability and maintainability.
> Also, this will open a road to write separate testing module like
> tests/test-ipam.c and integrate it into ovstest-like testing framework.
> Same could be done for all other logically separable parts of northd.
> Even actual logic of a northd itself could be split in parts, e.g.
> northd/ovs-northd-something.{c,h}.  In this case all the northd-specific
> datastructures/functions needed in different modules could be moved to
> northd/ovs-northd-private.{c,h}.  OVS uses this approach in many places,
> e.g.  ovsdb/raft.{c,h}, ovsdb/raft-rpc.{c,h}, ovsdb/raft-private.{c,h}.
> In this example rpc moved outside of main raft logic code and all required
>

Re: [ovs-dev] [PATCH ovs-dev, dpdk-latest 1/2] ovs-atomic: Rename memory_order -> ovs_memory_order

[0-day Robot]

Bleep bloop.  Greetings Eli Britstein, 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)
#60 FILE: lib/ovs-atomic-flag-gcc4.7+.h:43:
atomic_flag_clear_explicit(volatile atomic_flag *object, ovs_memory_order order)

WARNING: Line is 82 characters long (recommended limit is 79)
#313 FILE: lib/ovs-atomic.h:109:
 * enum ovs_memory_order specifies the strictness of a memory barrier.  It has 
the

WARNING: Line is 83 characters long (recommended limit is 79)
#322 FILE: lib/ovs-atomic.h:186:
 * OVS atomics require the ovs_memory_order to be passed as a compile-time 
constant

WARNING: Line is 83 characters long (recommended limit is 79)
#446 FILE: lib/ovs-rcu.h:250:
static inline int ovsrcu_index_get__(const ovsrcu_index *i, ovs_memory_order 
order)

Lines checked: 461, Warnings: 4, 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 ovs-dev, dpdk-latest 1/2] ovs-atomic: Rename memory_order -> ovs_memory_order

[Ilya Maximets]

On 10/12/20 4:37 PM, Gaëtan Rivet wrote:
> On 12/10/20 14:04 +, Eli Britstein wrote:
>> DPDK commit [1] uses function variables named "memory_order".
>> Compilation fails with:
>> error: declaration of 'memory_order' shadows a global declaration
>> [-Werror=shadow]
>>  rte_atomic_thread_fence(int memory_order)
>> Rename enum memory_order to ovs_memory_order to avoid that conflict.
>>
> 
> Hi Eli,
> 
> The C11 standard section 7.17.1.4 defines 'memory_order' as the
> "enumerated type whose enumerators identify memory ordering constraints".
> I think in this case this is a DPDK bug. Its API should be compatible
> with the C standard.

I agree here.  And, also, OVS defines this enum only for cases where
it is not defined by environment (e.g. stdatomic.h).  So, this patch will
actually fail the build on systems where ovs-atomic-c11.h is chosen as
an implementation for atomics, because it will not find ovs_memory_order
type defined.  Basically, I can not build OVS locally with this patch
applied because of this.

> 
>> [1] 672a15056380 ("eal: add wrapper for C11 atomic thread fence")
>>
>> Signed-off-by: Eli Britstein 
>> ---
> 
> Regards,
> 

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

Re: [ovs-dev] [RFC PATCH ovn 0/3] Unit Testing in OVN

[Ilya Maximets]

On 10/12/20 4:57 PM, Mark Gray wrote:
> On 09/10/2020 18:23, Ilya Maximets wrote:
>> On 10/9/20 5:29 PM, Mark Michelson wrote:
>>> OVN has had a test framework for as long as I've been working on the
>>> project. The test framework is designed for performing functional tests
>>> of OVN. That is, with the entirety of OVN up and running, we can provide
>>> configuration and test data and ensure that OVN does with that data what
>>> we expect. This is 100% a good thing and has helped us to detect lots of
>>> bugs before they can actually be merged in.
>>>
>>> What's missing, though, are smaller-scale unit tests. As an example, if
>>> I wanted to test ovn-northd's IPAM code, I would need to start up
>>> ovn-northd, create a logical switch, configure that logical switch to
>>> use IPAM, and then create logical switch ports to exercise the IPAM
>>> code. This can be overkill if my only goal is to ensure that IPAM's
>>> algorithm for selecting the next IP address is correct.
>>>
>>> This patch series proposes a unit test framework for OVN.
>>>
>>> If you want to run the unit tests, you can do so in a couple of ways.
>>>
>>> 1) Within the testsuite.
>>>./configure --with-ovs-source=/path/to/ovs --enable-unit-tests
>>>make check TESTSUITEFLAGS="-k unit"
>>>
>>> 2) One-off from the command line
>>>./configure --with-ovs-source=/path/to/ovs --enable-unit-tests
>>>make sandbox
>>>ovn-appctl -t ovn-northd unit-test 
>>>
>>> Some notes on this patch series
>>> 1) Patch 1 is the most important one in the series. This is an RFC
>>> because I'm trying to find out if the unit test framework itself is
>>> good. The refactoring in patch 2 and the unit tests added in patch 3 are
>>> meant to illustrate examples of the framework. They do not necessarily
>>> need to be merged as-is. Feel free to comment on them if you'd like,
>>> though.
>>> 2) Ideally, new unit tests could be added to the testsuite via a script.
>>> They've been added manually in this patch series.
>>> 3) This patch series only adds unit test capabilities to ovn-northd.
>>> However, the patch series we actually merge should add unit test
>>> capabilities to ovn-controller as well.
>>
>> That is an interesting approach, thanks for working n this!
> 
> +1
> 
>> This is very similar to ovstest framework (tests/ovstest.{c,h}), however
>> there are few differences:
>>
>> - ovstest is a separate executable and tests are implemented as a separate
>>   source files.  So, usage is 'ovstest mytest' instead of
>>   'ovn-appctl -t ovn-northd mytest'.  Upside of ovstest is that it doesn't
>>   require test code being integrated to the main executable.  Downside
>>   is that it requires functions being in a separate library that could
>>   be included.  Not sure if this is a downside though, as it forces
>>   developers to avoid huge source files with static functions.
>>
> 
> I don't think this is a downside. If the code was broken down into
> smaller modules, we could do exactly what you are suggesting Ilya. If we
> enable the ability to test code like this, it may reduce the motivation
> for modularization of the code :)
> 
> However, if we did go down this path, the other way that I have seen
> this done (while minimizing the amount test code leaking into production
> code) is to include .c files in your test code.
> 
> #include "northd/ovn-northd.c"
> 
> That way the test code can access private functions and structure in
> the code under test.

That's nasty. :)  Anyway, you can not just include ovn-northd.c because
it contains main() function.  You need to get rid of it somehow at first.

> 
> 
>> - current implementation of ovn unit test framework doesn't allow passing
>>   datasets via cmdline, while each unit test with ovstest could be called
>>   with different cmdline arguments. This makes it easier to test, as you
>>   don't need to rebuild binaries under test.
>>
>> - since ovn unit test framework implements tests inside main source files
>>   these files will, ideally, grow significantly.  ovn-northd is already
>>   13K lines of code, so this, probably, is not a very good thing.
>>
>> You also did a good job decoupling init_ipam_info out of northd stuff,
>> so it basically self-sufficient now.  What my suggestion here is to take
>> one more step forward and move this function to its own library, e.g.
>> move all the ipam related code that could be decoupled to lib/ipam.c
>> and lib/ipam.h.  This might be a good thing to do not only for unit
>> testing purposes, but just as a generic refactoring step that will
>> reduce code complexity and increase readability and maintainability.
>> Also, this will open a road to write separate testing module like
>> tests/test-ipam.c and integrate it into ovstest-like testing framework.
>> Same could be done for all other logically separable parts of northd.
>> Even actual logic of a northd itself could be split in parts, e.g.
>> northd/ovs-northd-something.{c,h}.  In this case all the northd-specific
>> da

Re: [ovs-dev] [RFC PATCH ovn 0/3] Unit Testing in OVN

[Mark Michelson]

On 10/9/20 1:23 PM, Ilya Maximets wrote:

On 10/9/20 5:29 PM, Mark Michelson wrote:

OVN has had a test framework for as long as I've been working on the
project. The test framework is designed for performing functional tests
of OVN. That is, with the entirety of OVN up and running, we can provide
configuration and test data and ensure that OVN does with that data what
we expect. This is 100% a good thing and has helped us to detect lots of
bugs before they can actually be merged in.

What's missing, though, are smaller-scale unit tests. As an example, if
I wanted to test ovn-northd's IPAM code, I would need to start up
ovn-northd, create a logical switch, configure that logical switch to
use IPAM, and then create logical switch ports to exercise the IPAM
code. This can be overkill if my only goal is to ensure that IPAM's
algorithm for selecting the next IP address is correct.

This patch series proposes a unit test framework for OVN.

If you want to run the unit tests, you can do so in a couple of ways.

1) Within the testsuite.
./configure --with-ovs-source=/path/to/ovs --enable-unit-tests
make check TESTSUITEFLAGS="-k unit"

2) One-off from the command line
./configure --with-ovs-source=/path/to/ovs --enable-unit-tests
make sandbox
ovn-appctl -t ovn-northd unit-test 

Some notes on this patch series
1) Patch 1 is the most important one in the series. This is an RFC
because I'm trying to find out if the unit test framework itself is
good. The refactoring in patch 2 and the unit tests added in patch 3 are
meant to illustrate examples of the framework. They do not necessarily
need to be merged as-is. Feel free to comment on them if you'd like,
though.
2) Ideally, new unit tests could be added to the testsuite via a script.
They've been added manually in this patch series.
3) This patch series only adds unit test capabilities to ovn-northd.
However, the patch series we actually merge should add unit test
capabilities to ovn-controller as well.


That is an interesting approach, thanks for working n this!
This is very similar to ovstest framework (tests/ovstest.{c,h}), however
there are few differences:

- ovstest is a separate executable and tests are implemented as a separate
   source files.  So, usage is 'ovstest mytest' instead of
   'ovn-appctl -t ovn-northd mytest'.  Upside of ovstest is that it doesn't
   require test code being integrated to the main executable.  Downside
   is that it requires functions being in a separate library that could
   be included.  Not sure if this is a downside though, as it forces
   developers to avoid huge source files with static functions.

- current implementation of ovn unit test framework doesn't allow passing
   datasets via cmdline, while each unit test with ovstest could be called
   with different cmdline arguments. This makes it easier to test, as you
   don't need to rebuild binaries under test.

- since ovn unit test framework implements tests inside main source files
   these files will, ideally, grow significantly.  ovn-northd is already
   13K lines of code, so this, probably, is not a very good thing.

You also did a good job decoupling init_ipam_info out of northd stuff,
so it basically self-sufficient now.  What my suggestion here is to take
one more step forward and move this function to its own library, e.g.
move all the ipam related code that could be decoupled to lib/ipam.c
and lib/ipam.h.  This might be a good thing to do not only for unit
testing purposes, but just as a generic refactoring step that will
reduce code complexity and increase readability and maintainability.
Also, this will open a road to write separate testing module like
tests/test-ipam.c and integrate it into ovstest-like testing framework.
Same could be done for all other logically separable parts of northd.
Even actual logic of a northd itself could be split in parts, e.g.
northd/ovs-northd-something.{c,h}.  In this case all the northd-specific
datastructures/functions needed in different modules could be moved to
northd/ovs-northd-private.{c,h}.  OVS uses this approach in many places,
e.g.  ovsdb/raft.{c,h}, ovsdb/raft-rpc.{c,h}, ovsdb/raft-private.{c,h}.
In this example rpc moved outside of main raft logic code and all required
common code moved to raft-private module.

What do you think?


Thanks for the detailed feedback, Ilya.

Quite a few things you mentioned ran through my mind as well. For 
instance, when I was doing the IPAM refactor, I considered that it would 
fit better in its own file instead of being in ovn-northd.c. If I put 
IPAM code into lib/ then I could add some code to test-ovn.c to test the 
public portions of it.


I came to the conclusion that a likely endgame here is to move ipam code 
to northd/ipam.[hc]. I could then put a northd/test-ipam.c file in place 
and have it be the test entry point for IPAM testing. I could have this 
northd/test-ipam.c file use ovstest to run the IPAM testing. We would 
need to add something to only c

Re: [ovs-dev] [RFC PATCH ovn 0/3] Unit Testing in OVN

[Numan Siddique]

On Mon, Oct 12, 2020 at 8:45 PM Mark Michelson  wrote:
>
> On 10/9/20 1:23 PM, Ilya Maximets wrote:
> > On 10/9/20 5:29 PM, Mark Michelson wrote:
> >> OVN has had a test framework for as long as I've been working on the
> >> project. The test framework is designed for performing functional tests
> >> of OVN. That is, with the entirety of OVN up and running, we can provide
> >> configuration and test data and ensure that OVN does with that data what
> >> we expect. This is 100% a good thing and has helped us to detect lots of
> >> bugs before they can actually be merged in.
> >>
> >> What's missing, though, are smaller-scale unit tests. As an example, if
> >> I wanted to test ovn-northd's IPAM code, I would need to start up
> >> ovn-northd, create a logical switch, configure that logical switch to
> >> use IPAM, and then create logical switch ports to exercise the IPAM
> >> code. This can be overkill if my only goal is to ensure that IPAM's
> >> algorithm for selecting the next IP address is correct.
> >>
> >> This patch series proposes a unit test framework for OVN.
> >>
> >> If you want to run the unit tests, you can do so in a couple of ways.
> >>
> >> 1) Within the testsuite.
> >> ./configure --with-ovs-source=/path/to/ovs --enable-unit-tests
> >> make check TESTSUITEFLAGS="-k unit"
> >>
> >> 2) One-off from the command line
> >> ./configure --with-ovs-source=/path/to/ovs --enable-unit-tests
> >> make sandbox
> >> ovn-appctl -t ovn-northd unit-test 
> >>
> >> Some notes on this patch series
> >> 1) Patch 1 is the most important one in the series. This is an RFC
> >> because I'm trying to find out if the unit test framework itself is
> >> good. The refactoring in patch 2 and the unit tests added in patch 3 are
> >> meant to illustrate examples of the framework. They do not necessarily
> >> need to be merged as-is. Feel free to comment on them if you'd like,
> >> though.
> >> 2) Ideally, new unit tests could be added to the testsuite via a script.
> >> They've been added manually in this patch series.
> >> 3) This patch series only adds unit test capabilities to ovn-northd.
> >> However, the patch series we actually merge should add unit test
> >> capabilities to ovn-controller as well.
> >
> > That is an interesting approach, thanks for working n this!
> > This is very similar to ovstest framework (tests/ovstest.{c,h}), however
> > there are few differences:
> >
> > - ovstest is a separate executable and tests are implemented as a separate
> >source files.  So, usage is 'ovstest mytest' instead of
> >'ovn-appctl -t ovn-northd mytest'.  Upside of ovstest is that it doesn't
> >require test code being integrated to the main executable.  Downside
> >is that it requires functions being in a separate library that could
> >be included.  Not sure if this is a downside though, as it forces
> >developers to avoid huge source files with static functions.
> >
> > - current implementation of ovn unit test framework doesn't allow passing
> >datasets via cmdline, while each unit test with ovstest could be called
> >with different cmdline arguments. This makes it easier to test, as you
> >don't need to rebuild binaries under test.
> >
> > - since ovn unit test framework implements tests inside main source files
> >these files will, ideally, grow significantly.  ovn-northd is already
> >13K lines of code, so this, probably, is not a very good thing.
> >
> > You also did a good job decoupling init_ipam_info out of northd stuff,
> > so it basically self-sufficient now.  What my suggestion here is to take
> > one more step forward and move this function to its own library, e.g.
> > move all the ipam related code that could be decoupled to lib/ipam.c
> > and lib/ipam.h.  This might be a good thing to do not only for unit
> > testing purposes, but just as a generic refactoring step that will
> > reduce code complexity and increase readability and maintainability.
> > Also, this will open a road to write separate testing module like
> > tests/test-ipam.c and integrate it into ovstest-like testing framework.
> > Same could be done for all other logically separable parts of northd.
> > Even actual logic of a northd itself could be split in parts, e.g.
> > northd/ovs-northd-something.{c,h}.  In this case all the northd-specific
> > datastructures/functions needed in different modules could be moved to
> > northd/ovs-northd-private.{c,h}.  OVS uses this approach in many places,
> > e.g.  ovsdb/raft.{c,h}, ovsdb/raft-rpc.{c,h}, ovsdb/raft-private.{c,h}.
> > In this example rpc moved outside of main raft logic code and all required
> > common code moved to raft-private module.
> >
> > What do you think?
>
> Thanks for the detailed feedback, Ilya.
>
> Quite a few things you mentioned ran through my mind as well. For
> instance, when I was doing the IPAM refactor, I considered that it would
> fit better in its own file instead of being in ovn-northd.c. If I put
> IPAM cod

Re: [ovs-dev] [PATCH ovs-dev, dpdk-latest 1/2] ovs-atomic: Rename memory_order -> ovs_memory_order

[Ilya Maximets]

On 10/12/20 5:04 PM, Ilya Maximets wrote:
> On 10/12/20 4:37 PM, Gaëtan Rivet wrote:
>> On 12/10/20 14:04 +, Eli Britstein wrote:
>>> DPDK commit [1] uses function variables named "memory_order".
>>> Compilation fails with:
>>> error: declaration of 'memory_order' shadows a global declaration
>>> [-Werror=shadow]
>>>  rte_atomic_thread_fence(int memory_order)
>>> Rename enum memory_order to ovs_memory_order to avoid that conflict.
>>>
>>
>> Hi Eli,
>>
>> The C11 standard section 7.17.1.4 defines 'memory_order' as the
>> "enumerated type whose enumerators identify memory ordering constraints".
>> I think in this case this is a DPDK bug. Its API should be compatible
>> with the C standard.
> 
> I agree here.  And, also, OVS defines this enum only for cases where
> it is not defined by environment (e.g. stdatomic.h).  So, this patch will
> actually fail the build on systems where ovs-atomic-c11.h is chosen as
> an implementation for atomics, because it will not find ovs_memory_order
> type defined.  Basically, I can not build OVS locally with this patch
> applied because of this.

And yes, there still be warning since DPDK will shadow the definition that
comes from the stdatomic.h.

> 
>>
>>> [1] 672a15056380 ("eal: add wrapper for C11 atomic thread fence")
>>>
>>> Signed-off-by: Eli Britstein 
>>> ---
>>
>> Regards,
>>
> 

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

Re: [ovs-dev] [PATCH 1/1] netdev-offload-dpdk: Support vxlan encap offload with load actions

[Sriharsha Basavapatna via dev]

On Mon, Oct 12, 2020 at 4:48 PM Eli Britstein  wrote:
>
>
> On 10/12/2020 10:20 AM, Sriharsha Basavapatna wrote:
> > On Sun, Sep 6, 2020 at 5:51 PM Eli Britstein  wrote:
> >> ping
> >>
> >> On 7/30/2020 1:58 PM, Eli Britstein wrote:
> >>> From: Lei Wang 
> >>>
> >>> Struct match has the tunnel values/masks in
> >>> match->flow.tunnel/match->wc.masks.tunnel.
> >>> Load actions such as load:0xa566c10->NXM_NX_TUN_IPV4_DST[],
> >>> load:0xbba->NXM_NX_TUN_ID[] are utilizing the tunnel masks fields,
> >>> but those should not be used for matching.
> >>> Offloading fails if masks is not clear. Clear it if no tunnel used.
> >>>
> >>> Signed-off-by: Lei Wang 
> >>> Reviewed-by: Eli Britstein 
> >>> Reviewed-by: Gaetan Rivet 
> >>> Signed-off-by: Eli Britstein 

Acked-by: Sriharsha Basavapatna 

See my comment below.

> >>> ---
> >>>lib/netdev-offload-dpdk.c | 4 
> >>>1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> >>> index de6101e4d..0d23e4879 100644
> >>> --- a/lib/netdev-offload-dpdk.c
> >>> +++ b/lib/netdev-offload-dpdk.c
> >>> @@ -682,6 +682,10 @@ parse_flow_match(struct flow_patterns *patterns,
> >>>
> >>>consumed_masks = &match->wc.masks;
> >>>
> >>> +if (!flow_tnl_dst_is_set(&match->flow.tunnel)) {
> >>> +memset(&match->wc.masks.tunnel, 0, sizeof 
> >>> match->wc.masks.tunnel);
> >>> +}
> >>> +
> > This fix looks good to me.  Did you take a look to see if this can be
> > fixed in the code that generates these invalid masks in the first
> > place ?
> I wouldn't say it's "invalid masks". OVS takes those masks into
> consideration with such usage of flow_tnl_dst_is_set, that is done in
> several places in the code. For example odp-util.c, lib/netdev-offload-tc.c.

It is invalid because the datapath rule is specifying tunnel masks
when we are not really matching on them. In the context of encap
actions (which is what this patch is fixing), tunnel masks shouldn't
be set.

> >
> > Thanks,
> > -Harsha
> >>>memset(&consumed_masks->in_port, 0, sizeof 
> >>> consumed_masks->in_port);
> >>>/* recirc id must be zero. */
> >>>if (match->wc.masks.recirc_id & match->flow.recirc_id) {
> >> ___
> >> 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 1/1] netdev-offload-dpdk: Preserve HW statistics for modified flows

[Sriharsha Basavapatna via dev]

On Mon, Oct 12, 2020 at 4:43 PM Eli Britstein  wrote:
>
>
> On 10/12/2020 11:45 AM, Sriharsha Basavapatna wrote:
> > On Sun, Sep 6, 2020 at 5:52 PM Eli Britstein  wrote:
> >> ping
> >>
> >> On 7/30/2020 4:52 PM, Eli Britstein wrote:
> >>> In case of a flow modification, preserve the HW statistics of the old HW
> >>> flow to the new one.
> >>>
> >>> Signed-off-by: Eli Britstein 
> >>> Reviewed-by: Gaetan Rivet 
> > Update fixes: tag
> I'll put 3c7330ebf036 netdev-offload-dpdk: Support offload of output action.
> As it's the first commit that does full offload. Before that there are
> no HW rules that count. What do you think?

That's fine.
> >>> ---
> >>>lib/netdev-offload-dpdk.c | 28 +---
> >>>1 file changed, 21 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> >>> index de6101e4d..960acb2da 100644
> >>> --- a/lib/netdev-offload-dpdk.c
> >>> +++ b/lib/netdev-offload-dpdk.c
> >>> @@ -78,7 +78,7 @@ ufid_to_rte_flow_data_find(const ovs_u128 *ufid)
> >>>return NULL;
> >>>}
> >>>
> >>> -static inline void
> >>> +static inline struct ufid_to_rte_flow_data *
> >>>ufid_to_rte_flow_associate(const ovs_u128 *ufid,
> >>>   struct rte_flow *rte_flow, bool 
> >>> actions_offloaded)
> >>>{
> >>> @@ -103,6 +103,7 @@ ufid_to_rte_flow_associate(const ovs_u128 *ufid,
> >>>
> >>>cmap_insert(&ufid_to_rte_flow,
> >>>CONST_CAST(struct cmap_node *, &data->node), hash);
> >>> +return data;
> >>>}
> >>>
> >>>static inline void
> >>> @@ -1407,7 +1408,7 @@ out:
> >>>return flow;
> >>>}
> >>>
> >>> -static int
> >>> +static struct ufid_to_rte_flow_data *
> >>>netdev_offload_dpdk_add_flow(struct netdev *netdev,
> >>> struct match *match,
> >>> struct nlattr *nl_actions,
> >>> @@ -1416,6 +1417,7 @@ netdev_offload_dpdk_add_flow(struct netdev *netdev,
> >>> struct offload_info *info)
> >>>{
> >>>struct flow_patterns patterns = { .items = NULL, .cnt = 0 };
> >>> +struct ufid_to_rte_flow_data *flows_data = NULL;
> >>>bool actions_offloaded = true;
> >>>struct rte_flow *flow;
> >>>int ret = 0;
> > We can eliminate 'ret' with this change, since it is only being used
> > to catch the return value of parse_flow_match().
> Ack
> >>> @@ -1442,13 +1444,13 @@ netdev_offload_dpdk_add_flow(struct netdev 
> >>> *netdev,
> >>>ret = -1;
> > Relates to the above comment.
> >>>goto out;
> >>>}
> >>> -ufid_to_rte_flow_associate(ufid, flow, actions_offloaded);
> >>> +flows_data = ufid_to_rte_flow_associate(ufid, flow, 
> >>> actions_offloaded);
> >>>VLOG_DBG("%s: installed flow %p by ufid "UUID_FMT,
> >>> netdev_get_name(netdev), flow, UUID_ARGS((struct uuid 
> >>> *)ufid));
> >>>
> >>>out:
> >>>free_flow_patterns(&patterns);
> >>> -return ret;
> >>> +return flows_data;
> >>>}
> >>>
> >>>static int
> >>> @@ -1482,14 +1484,19 @@ netdev_offload_dpdk_flow_put(struct netdev 
> >>> *netdev, struct match *match,
> >>> struct dpif_flow_stats *stats)
> >>>{
> >>>struct ufid_to_rte_flow_data *rte_flow_data;
> >>> +struct dpif_flow_stats old_stats;
> >>> +bool modification = false;
> >>>int ret;
> >>>
> >>>/*
> >>> * If an old rte_flow exists, it means it's a flow modification.
> >>> * Here destroy the old rte flow first before adding a new one.
> >>> + * Keep the stats for the newly created rule.
> >>> */
> >>>rte_flow_data = ufid_to_rte_flow_data_find(ufid);
> >>>if (rte_flow_data && rte_flow_data->rte_flow) {
> >>> +old_stats = rte_flow_data->stats;
> >>> +modification = true;
> >>>ret = netdev_offload_dpdk_destroy_flow(netdev, ufid,
> >>>   
> >>> rte_flow_data->rte_flow);
> >>>if (ret < 0) {
> >>> @@ -1497,11 +1504,18 @@ netdev_offload_dpdk_flow_put(struct netdev 
> >>> *netdev, struct match *match,
> >>>}
> >>>}
> >>>
> >>> +rte_flow_data = netdev_offload_dpdk_add_flow(netdev, match, actions,
> >>> + actions_len, ufid, 
> >>> info);
> >>> +if (!rte_flow_data) {
> >>> +return -1;
> >>> +}
> >>> +if (modification) {
> >>> +rte_flow_data->stats = old_stats;
> >>> +}
> >>>if (stats) {
> >>> -memset(stats, 0, sizeof *stats);
> > What if it is a new flow, don't we need to clear the stats ?
>
> It is already cleared before. Allocation is xZalloc. See in
> ufid_to_rte_flow_associate:
>
>  struct ufid_to_rte_flow_data *data = xzalloc(sizeof *data);

Ok, thanks.
>
> >>> +*stats = rte_flow_data->stats;
> >>>}
> >>> -

Re: [ovs-dev] [PATCH ovs-dev, dpdk-latest V2 1/2] ovs-atomic: Rename memory_order -> ovs_memory_order

[0-day Robot]

Bleep bloop.  Greetings Eli Britstein, 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)
#60 FILE: lib/ovs-atomic-flag-gcc4.7+.h:43:
atomic_flag_clear_explicit(volatile atomic_flag *object, ovs_memory_order order)

WARNING: Line is 82 characters long (recommended limit is 79)
#313 FILE: lib/ovs-atomic.h:109:
 * enum ovs_memory_order specifies the strictness of a memory barrier.  It has 
the

WARNING: Line is 83 characters long (recommended limit is 79)
#322 FILE: lib/ovs-atomic.h:186:
 * OVS atomics require the ovs_memory_order to be passed as a compile-time 
constant

WARNING: Line is 83 characters long (recommended limit is 79)
#446 FILE: lib/ovs-rcu.h:250:
static inline int ovsrcu_index_get__(const ovsrcu_index *i, ovs_memory_order 
order)

Lines checked: 461, Warnings: 4, 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] [RFC PATCH ovn 0/3] Unit Testing in OVN

[Mark Gray]

On 12/10/2020 16:13, Ilya Maximets wrote:
> On 10/12/20 4:57 PM, Mark Gray wrote:
>> On 09/10/2020 18:23, Ilya Maximets wrote:
>>> On 10/9/20 5:29 PM, Mark Michelson wrote:
 OVN has had a test framework for as long as I've been working on the
 project. The test framework is designed for performing functional tests
 of OVN. That is, with the entirety of OVN up and running, we can provide
 configuration and test data and ensure that OVN does with that data what
 we expect. This is 100% a good thing and has helped us to detect lots of
 bugs before they can actually be merged in.

 What's missing, though, are smaller-scale unit tests. As an example, if
 I wanted to test ovn-northd's IPAM code, I would need to start up
 ovn-northd, create a logical switch, configure that logical switch to
 use IPAM, and then create logical switch ports to exercise the IPAM
 code. This can be overkill if my only goal is to ensure that IPAM's
 algorithm for selecting the next IP address is correct.

 This patch series proposes a unit test framework for OVN.

 If you want to run the unit tests, you can do so in a couple of ways.

 1) Within the testsuite.
./configure --with-ovs-source=/path/to/ovs --enable-unit-tests
make check TESTSUITEFLAGS="-k unit"

 2) One-off from the command line
./configure --with-ovs-source=/path/to/ovs --enable-unit-tests
make sandbox
ovn-appctl -t ovn-northd unit-test 

 Some notes on this patch series
 1) Patch 1 is the most important one in the series. This is an RFC
 because I'm trying to find out if the unit test framework itself is
 good. The refactoring in patch 2 and the unit tests added in patch 3 are
 meant to illustrate examples of the framework. They do not necessarily
 need to be merged as-is. Feel free to comment on them if you'd like,
 though.
 2) Ideally, new unit tests could be added to the testsuite via a script.
 They've been added manually in this patch series.
 3) This patch series only adds unit test capabilities to ovn-northd.
 However, the patch series we actually merge should add unit test
 capabilities to ovn-controller as well.
>>>
>>> That is an interesting approach, thanks for working n this!
>>
>> +1
>>
>>> This is very similar to ovstest framework (tests/ovstest.{c,h}), however
>>> there are few differences:
>>>
>>> - ovstest is a separate executable and tests are implemented as a separate
>>>   source files.  So, usage is 'ovstest mytest' instead of
>>>   'ovn-appctl -t ovn-northd mytest'.  Upside of ovstest is that it doesn't
>>>   require test code being integrated to the main executable.  Downside
>>>   is that it requires functions being in a separate library that could
>>>   be included.  Not sure if this is a downside though, as it forces
>>>   developers to avoid huge source files with static functions.
>>>
>>
>> I don't think this is a downside. If the code was broken down into
>> smaller modules, we could do exactly what you are suggesting Ilya. If we
>> enable the ability to test code like this, it may reduce the motivation
>> for modularization of the code :)
>>
>> However, if we did go down this path, the other way that I have seen
>> this done (while minimizing the amount test code leaking into production
>> code) is to include .c files in your test code.
>>
>> #include "northd/ovn-northd.c"
>>
>> That way the test code can access private functions and structure in
>> the code under test.
> 
> That's nasty. :)  Anyway, you can not just include ovn-northd.c because
> it contains main() function.  You need to get rid of it somehow at first.

I did say "minimizing" :D - you would have to do something with main()
(#ifdef, or maybe there is a smart way to change the entry point of the
executable through the compiler). Everything is nasty to enable this in
C if the code is not modular but I think the preference should be to not
modify the production code. Maybe Numan's suggestion is worth looking at?

The other issue we will have is mocking out components so we could
inject defined responses to certain function calls to, for example,
force error conditions.

> 
>>
>>
>>> - current implementation of ovn unit test framework doesn't allow passing
>>>   datasets via cmdline, while each unit test with ovstest could be called
>>>   with different cmdline arguments. This makes it easier to test, as you
>>>   don't need to rebuild binaries under test.
>>>
>>> - since ovn unit test framework implements tests inside main source files
>>>   these files will, ideally, grow significantly.  ovn-northd is already
>>>   13K lines of code, so this, probably, is not a very good thing.
>>>
>>> You also did a good job decoupling init_ipam_info out of northd stuff,
>>> so it basically self-sufficient now.  What my suggestion here is to take
>>> one more step forward and move this function to its own library, e.g.

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

[David Marchand]

Hello Ian,

On Mon, Oct 5, 2020 at 7:11 PM Stokes, Ian  wrote:
> Thanks all for the reviews and input, I've made change above and applied to 
> master.

When you get the chance, could you rebase the dpdk-latest branch on
the master branch so that this patch is in it?
Thanks!


-- 
David Marchand

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

Re: [ovs-dev] [PATCH] smap: Add smap_get_uint() helper function.

[Numan Siddique]

On Fri, Oct 9, 2020 at 8:40 PM Ilya Maximets  wrote:
>
> On 9/18/20 5:27 PM, Dumitru Ceara wrote:
> > On 9/18/20 5:15 PM, num...@ovn.org wrote:
> >> From: Numan Siddique 
> >>
> >> This helper function is required by OVN.
> >>
> >> Suggested-by: Dumitru Ceara 
> >> Signed-off-by: Numan Siddique 
> >> ---
> >>  lib/smap.c | 16 
> >>  lib/smap.h |  2 ++
> >>  2 files changed, 18 insertions(+)
> >>
> >> diff --git a/lib/smap.c b/lib/smap.c
> >> index 149b8b243..e82261497 100644
> >> --- a/lib/smap.c
> >> +++ b/lib/smap.c
> >> @@ -247,6 +247,22 @@ smap_get_int(const struct smap *smap, const char 
> >> *key, int def)
> >>  return i_value;
> >>  }
> >>
> >> +/* Gets the value associated with 'key' in 'smap' and converts it to an
> >> + * unsigned int. If 'key' is not in 'smap' or a valid unsigned integer
> >> + * can't be parsed from it's value, returns 'def'. */
> >> +unsigned int
> >> +smap_get_uint(const struct smap *smap, const char *key, unsigned int def)
> >> +{
> >> +const char *value = smap_get(smap, key);
> >> +unsigned int u_value;
> >> +
> >> +if (!value || !str_to_uint(value, 10, &u_value)) {
> >> +return def;
> >> +}
> >> +
> >> +return u_value;
> >> +}
> >> +
> >>  /* Gets the value associated with 'key' in 'smap' and converts it to an
> >>   * unsigned long long.  If 'key' is not in 'smap' or a valid number can't 
> >> be
> >>   * parsed from it's value, returns 'def'. */
> >> diff --git a/lib/smap.h b/lib/smap.h
> >> index 766c65f7f..a92115966 100644
> >> --- a/lib/smap.h
> >> +++ b/lib/smap.h
> >> @@ -104,6 +104,8 @@ const char *smap_get_def(const struct smap *, const 
> >> char *key,
> >>  struct smap_node *smap_get_node(const struct smap *, const char *);
> >>  bool smap_get_bool(const struct smap *smap, const char *key, bool def);
> >>  int smap_get_int(const struct smap *smap, const char *key, int def);
> >> +unsigned int smap_get_uint(const struct smap *smap, const char *key,
> >> +   unsigned int def);
> >>  unsigned long long int smap_get_ullong(const struct smap *, const char 
> >> *key,
> >> unsigned long long def);
> >>  bool smap_get_uuid(const struct smap *, const char *key, struct uuid *);
> >>
> >
> > Looks good to me.
> >
> > Acked-by: Dumitru Ceara 
>
> Thanks, Numan and Dumitru!

Thanks Ilya and Dumitru.

Numan

>
> Applied to master.
>
> Best regards, Ilya Maximets.
> ___
> 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] Revert "travis: Disable check for array of flexible structures in sparse."

[Ilya Maximets]

This reverts commit 3c6b3a519ae6eae3da4cf7c59894b02b95cdade7.

The fix landed to Sparse main repository [1]:
  b5d46df743be ("flex-array: allow arrays of unions with flexible members.")

[1] https://git.kernel.org/pub/scm/devel/sparse/sparse.git

Signed-off-by: Ilya Maximets 
---
 .travis/linux-build.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
index 6b6935794..6981d1d47 100755
--- a/.travis/linux-build.sh
+++ b/.travis/linux-build.sh
@@ -4,7 +4,7 @@ set -o errexit
 set -x
 
 CFLAGS_FOR_OVS="-g -O2"
-SPARSE_FLAGS="-Wno-flexible-array-array"
+SPARSE_FLAGS=""
 EXTRA_OPTS="--enable-Werror"
 
 function install_kernel()
-- 
2.25.4

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

Re: [ovs-dev] [PATCH ovn 1/5] northd: Use 'enum ovn_stage' for the table value in the 'next' OVN action.

[Dumitru Ceara]

On 10/5/20 7:49 PM, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> Multiple places in ovn-northd.c hard codes the table value in the next() OVN 
> action.
> This patch changes those occurrences to use ovn_stage_get_table('enum 
> ovn_stage' value).
> 
> Hard coding of the table number can result in errors if new stages are added 
> (like
> the patch [1] which added new stages - ls_in_acl_hint and ls_out_acl_hint). 
> After the patch [1],
> the table number was wrong for reject ACLs associated in ingress logical 
> switch pipeline stage.
> Although this didn't result in any packet drops. This patch avoids such cases 
> in the future.
> 
> This patch also adds a new test case in ovn-northd.at for reject ACL flows.
> 
> [1] - 209ea46bbf9d("ovn-northd: Reduce number of flows generated for stateful 
> ACLs.")
> 
> Signed-off-by: Numan Siddique 

Hi Numan,

This change looks OK to me, just a few minor comments inline.

> ---
>  northd/ovn-northd.c |  36 ---
>  tests/ovn-northd.at | 247 
>  2 files changed, 266 insertions(+), 17 deletions(-)
> 
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 91da319415..d5fd7da03a 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -5411,6 +5411,12 @@ build_reject_acl_rules(struct ovn_datapath *od, struct 
> hmap *lflows,
>  struct ds actions = DS_EMPTY_INITIALIZER;
>  bool ingress = (stage == S_SWITCH_IN_ACL);
>  
> +char *next_action =
> +xasprintf("next(pipeline=%s,table=%d);",
> +  ingress ? "egress": "ingress",
> +  ingress ? ovn_stage_get_table(S_SWITCH_OUT_QOS_MARK)
> +  : ovn_stage_get_table(S_SWITCH_IN_L2_LKUP));
> +
>  /* TCP */
>  build_acl_log(&actions, acl);
>  if (extra_match->length > 0) {
> @@ -5419,9 +5425,7 @@ build_reject_acl_rules(struct ovn_datapath *od, struct 
> hmap *lflows,
>  ds_put_format(&match, "ip4 && tcp && (%s)", acl->match);
>  ds_put_format(&actions, "reg0 = 0; "
>"eth.dst <-> eth.src; ip4.dst <-> ip4.src; "
> -  "tcp_reset { outport <-> inport; %s };",
> -  ingress ? "next(pipeline=egress,table=5);"
> -  : "next(pipeline=ingress,table=20);");
> +  "tcp_reset { outport <-> inport; %s };", next_action);
>  ovn_lflow_add_with_hint(lflows, od, stage,
>  acl->priority + OVN_ACL_PRI_OFFSET + 10,
>  ds_cstr(&match), ds_cstr(&actions), stage_hint);
> @@ -5434,9 +5438,7 @@ build_reject_acl_rules(struct ovn_datapath *od, struct 
> hmap *lflows,
>  ds_put_format(&match, "ip6 && tcp && (%s)", acl->match);
>  ds_put_format(&actions, "reg0 = 0; "
>"eth.dst <-> eth.src; ip6.dst <-> ip6.src; "
> -  "tcp_reset { outport <-> inport; %s };",
> -  ingress ? "next(pipeline=egress,table=5);"
> -  : "next(pipeline=ingress,table=20);");
> +  "tcp_reset { outport <-> inport; %s };", next_action);
>  ovn_lflow_add_with_hint(lflows, od, stage,
>  acl->priority + OVN_ACL_PRI_OFFSET + 10,
>  ds_cstr(&match), ds_cstr(&actions), stage_hint);
> @@ -5454,9 +5456,7 @@ build_reject_acl_rules(struct ovn_datapath *od, struct 
> hmap *lflows,
>  }
>  ds_put_format(&actions, "reg0 = 0; "
>"icmp4 { eth.dst <-> eth.src; ip4.dst <-> ip4.src; "
> -  "outport <-> inport; %s };",
> -  ingress ? "next(pipeline=egress,table=5);"
> -  : "next(pipeline=ingress,table=20);");
> +  "outport <-> inport; %s };", next_action);
>  ovn_lflow_add_with_hint(lflows, od, stage,
>  acl->priority + OVN_ACL_PRI_OFFSET,
>  ds_cstr(&match), ds_cstr(&actions), stage_hint);
> @@ -5472,13 +5472,12 @@ build_reject_acl_rules(struct ovn_datapath *od, 
> struct hmap *lflows,
>  }
>  ds_put_format(&actions, "reg0 = 0; icmp6 { "
>"eth.dst <-> eth.src; ip6.dst <-> ip6.src; "
> -  "outport <-> inport; %s };",
> -  ingress ? "next(pipeline=egress,table=5);"
> -  : "next(pipeline=ingress,table=20);");
> +  "outport <-> inport; %s };", next_action);
>  ovn_lflow_add_with_hint(lflows, od, stage,
>  acl->priority + OVN_ACL_PRI_OFFSET,
>  ds_cstr(&match), ds_cstr(&actions), stage_hint);
>  
> +free(next_action);
>  ds_destroy(&match);
>  ds_destroy(&actions);
>  }
> @@ -9820,7 +9819,8 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap 
> *ports,
>  ds_put_format(&actions, "reg%d = 0; ", j);
>  }
>  ds_put_format(&actions, REGBIT_EGRESS_LOOPBACK" = 1; "
> -   

Re: [ovs-dev] [PATCH ovn 2/5] ovn-trace: Don't assert for next(stage=ingress, ..).

[Dumitru Ceara]

On 10/5/20 7:49 PM, num...@ovn.org wrote:
> From: Numan Siddique 
> 
> The commit [1] allowed next action to advance from ingress to egress 
> pipeline, but
> ovn-trace was not modified to handle this condition. Also corrected the 
> ovntrace node
> format message as per the next stage.
> 
> [1] - b4b68177eb2f("Fix conntrack entry leaks because of TCP RST packets not 
> sent to conntrack.")
> 
> Fixes: b4b68177eb2f("Fix conntrack entry leaks because of TCP RST packets not 
> sent to conntrack.")
> Signed-off-by: Numan Siddique 

Looks good to me, thanks!

Acked-by: Dumitru Ceara 

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

Re: [ovs-dev] [PATCH v2] Don't raise an Exception on failure to connect via SSL

[Thomas Neuman]

With other socket types, trying to connect and failing will return
an error code, but if an SSL Stream is used, then when
check_connection_completion(sock) is called, SSL will raise an
exception that doesn't derive from socket.error which is handled.

This adds handling for SSL.SysCallError which has the same
arguments as socket.error (errno, string). A future enhancement
could be to go through SSLStream class and implement error
checking for all of the possible exceptions similar to how
lib/stream-ssl.c's interpret_ssl_error() works across the various
methods that are implemented.

Signed-off-by: Terry Wilson https://mail.openvswitch.org/mailman/listinfo/ovs-dev>>
---
 python/ovs/stream.py | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/python/ovs/stream.py b/python/ovs/stream.py
index e9bb0c854..f5a520862 100644
--- a/python/ovs/stream.py
+++ b/python/ovs/stream.py
@@ -132,6 +132,10 @@ class Stream(object):
 IPTOS_PREC_INTERNETCONTROL = 0xc0
 DSCP_DEFAULT = IPTOS_PREC_INTERNETCONTROL >> 2
 
+@staticmethod

+def check_connection_completion(sock):
+return ovs.socket_util.check_connection_completion(sock)
+


Only thing I might question is making this a static method instead of, say, a 
class
method. Especially considering its usage in "open()" below, and the fact that 
we're
overriding it in the SSLStream subclass, it seems like that would be clearer to 
me.
But not a huge deal to me either way, so I'll defer to others' judgement.


 @staticmethod
 def open(name, dscp=DSCP_DEFAULT):
 """Attempts to connect a stream to a remote peer.  'name' is a
@@ -189,7 +193,7 @@ class Stream(object):
 if error:
 return error, None
 else:
-err = ovs.socket_util.check_connection_completion(sock)
+err = cls.check_connection_completion(sock)
 if err == errno.EAGAIN or err == errno.EINPROGRESS:
 status = errno.EAGAIN
 err = 0
@@ -261,7 +265,7 @@ class Stream(object):
 
 def __scs_connecting(self):

 if self.socket is not None:
-retval = ovs.socket_util.check_connection_completion(self.socket)
+retval = self.check_connection_completion(self.socket)
 assert retval != errno.EINPROGRESS
 elif sys.platform == 'win32':
 if self.retry_connect:
@@ -761,6 +765,13 @@ Stream.register_method("tcp", TCPStream)
 
 
 class SSLStream(Stream):

+@staticmethod
+def check_connection_completion(sock):
+try:
+return Stream.check_connection_completion(sock)
+except SSL.SysCallError as e:
+return ovs.socket_util.get_exception_errno(e)
+
 @staticmethod
 def needs_probes():
 return True



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

[ovs-dev] Want a brain like the best and brightest? Once a day...

[BrainPlus Precise]

Having trouble viewing this email? Please follow this link to see the messaged 
emailed to you. 

Product Image
Product Image






(Unsubscribe Instructions Here)
(Unsubscribe Instructions Here)

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

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

[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)
&flow->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 V4 09/24] datapath: simplify the flow_hash

[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 V4 04/24] datapath: Print error when ovs_execute_actions() fails

[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)
 &n_mask_hit);
if (unlikely(!flow)) {
struct dp_upcall_info upcall;
-   int error;
 
memset(&upcall, 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 = &stats->n_hit;
 
-- 
2.17.1

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

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

[Greg Rose]

This patch set will add support for Linux kernels up to 5.8. 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.

V4 passes Travis here:
https://travis-ci.org/github/gvrose8192/ovs-experimental/builds/735114390

---
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.
V3 - Add tag from Eelco Chaldron
   - Mix mistaken warning message in acinclude.m4
   - Remember to add link to passing Travis build in cover letter
V4 - Add portion change in ovs_dp_cmd_set missed in previous patch set
   - Simplify the WRITE_ONCE macro implemention for compat.

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   | 224 --
 datapath/datapath.h   |   2 +
 datapath/flow.c   |  13 +
 datapath/flow_netlink.c   |  18 +-
 datapath/flow_table.c | 214 +
 .../linux/compat/include/linux/compiler.h |   8 +
 datapath/linux/compat/include/linux/skbuff.h  |   7 +
 .../linux/compat/include/linux/static_key.h   |   7 +
 datapath/vport.c  |   5 +-
 14 files changed, 330 insertions(+), 193 deletions(-)

-- 
2.17.1

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

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

[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, &dp->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(&dp_vport_genl_family, &ovs_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 V4 07/24] datapath: don't unlock mutex when changing the user_features fails

[Greg Rose]

From: Tonghao Zhang 

Upstream commit:
commit 4c76bf696a608ea5cc555fe97ec59a9033236604
Author: Tonghao Zhang 
Date:   Fri Nov 1 22:23:53 2019 +0800

net: openvswitch: don't unlock mutex when changing the user_features fails

Unlocking of a not locked mutex is not allowed.
Other kernel thread may be in critical section while
we unlock it because of setting user_feature fail.

Fixes: 95a7233c4 ("net: openvswitch: Set OvS recirc_id from tc chain index")
Cc: Paul Blakey 
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/datapath.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/datapath/datapath.c b/datapath/datapath.c
index aceb655bd..cf216c8ec 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1746,6 +1746,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct 
genl_info *info)
ovs_dp_reset_user_features(skb, info);
}
 
+   ovs_unlock();
goto err_destroy_meters;
}
 
@@ -1762,7 +1763,6 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct 
genl_info *info)
return 0;
 
 err_destroy_meters:
-   ovs_unlock();
ovs_meters_exit(dp);
 err_destroy_ports_array:
kfree(dp->ports);
-- 
2.17.1

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

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

[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 
Acked-by: 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 V4 11/24] datapath: fix possible memleak on destroy flow-table

[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 

---
V4 - Simplify the WRITE_ONCE macro implemention as suggested by
 Yihung.
---
 datapath/flow_table.c | 186 +-
 .../linux/compat/include/linux/compiler.h |   8 +
 2 files changed, 106 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->flow_table.node[ti->node_ver]);
+   if (count)
+   table->count--;
+
+   if (ovs_identifier_is_ufid(&flow->id)) {
+   hlist_del_rcu(&flow->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 = &ti->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->flow_table.node[ver]);
-   if (ovs_identifier_is_ufid(&flow->id))
- 

[ovs-dev] [PATCH V4 08/24] datapath: optimize flow-mask looking up

[Greg Rose]

From: Tonghao Zhang 

Upstream commit:
commit 57f7d7b9164426c496300d254fd5167fbbf205ea
Author: Tonghao Zhang 
Date:   Fri Nov 1 22:23:49 2019 +0800

net: openvswitch: optimize flow-mask looking up

The full looking up on flow table traverses all mask array.
If mask-array is too large, the number of invalid flow-mask
increase, performance will be drop.

One bad case, for example: M means flow-mask is valid and NULL
of flow-mask means deleted.

+---+
| M | NULL | ...  | NULL | M|
+---+

In that case, without this patch, openvswitch will traverses all
mask array, because there will be one flow-mask in the tail. This
patch changes the way of flow-mask inserting and deleting, and the
mask array will be keep as below: there is not a NULL hole. In the
fast path, we can "break" "for" (not "continue") in flow_lookup
when we get a NULL flow-mask.

 "break"
v
+---+
| M | M |  NULL |...   | NULL | NULL|
+---+

This patch don't optimize slow or control path, still using ma->max
to traverse. Slow path:
* tbl_mask_array_realloc
* ovs_flow_tbl_lookup_exact
* flow_mask_find

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/flow_table.c | 103 ++
 1 file changed, 53 insertions(+), 50 deletions(-)

diff --git a/datapath/flow_table.c b/datapath/flow_table.c
index 76b390e9c..62d726ddd 100644
--- a/datapath/flow_table.c
+++ b/datapath/flow_table.c
@@ -540,8 +540,8 @@ static struct sw_flow *flow_lookup(struct flow_table *tbl,
   u32 *n_mask_hit,
   u32 *index)
 {
-   struct sw_flow_mask *mask;
struct sw_flow *flow;
+   struct sw_flow_mask *mask;
int i;
 
if (*index < ma->max) {
@@ -560,7 +560,7 @@ static struct sw_flow *flow_lookup(struct flow_table *tbl,
 
mask = rcu_dereference_ovsl(ma->masks[i]);
if (!mask)
-   continue;
+   break;
 
flow = masked_flow_lookup(ti, key, mask, n_mask_hit);
if (flow) { /* Found */
@@ -716,7 +716,7 @@ int ovs_flow_tbl_num_masks(const struct flow_table *table)
struct mask_array *ma;
 
ma = rcu_dereference_ovsl(table->mask_array);
-   return ma->count;
+   return READ_ONCE(ma->count);
 }
 
 static struct table_instance *table_instance_expand(struct table_instance *ti,
@@ -725,21 +725,33 @@ static struct table_instance 
*table_instance_expand(struct table_instance *ti,
return table_instance_rehash(ti, ti->n_buckets * 2, ufid);
 }
 
-static void tbl_mask_array_delete_mask(struct mask_array *ma,
-  struct sw_flow_mask *mask)
+static void tbl_mask_array_del_mask(struct flow_table *tbl,
+   struct sw_flow_mask *mask)
 {
-   int i;
+   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->max; i++) {
-   if (mask == ovsl_dereference(ma->masks[i])) {
-   RCU_INIT_POINTER(ma->masks[i], NULL);
-   ma->count--;
-   kfree_rcu(mask, rcu);
-   return;
-   }
+   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. */
@@ -753,18 +765,8 @@ static void flow_mask_remove(struct flow_table *tbl, 
struct sw_flow_mask *mask)
BUG_ON(!mask->ref_count);
mask->ref_count--;
 
-   if (!mask->ref_count) {
-   struct mask_array *ma;
-
-   ma = ovsl_dereference(tbl->mask_array);
-   tbl_mask_array_delete_mask(ma, mask);
-
-   /* Shrink the mask array if necessary. */
-   if (ma->max >= (MASK_ARRAY_SIZE_MIN * 2) &&
- 

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

[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 1bc8e1439..52a59f135 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 V4 12/24] datapath: simplify the ovs_dp_cmd_new

[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 cf216c8ec..22a08baa3 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(&dp->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(&dp->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(&dp->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(&dp->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 V4 10/24] datapath: add likely in flow_lookup

[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 V4 13/24] datapath: select vport upcall portid directly

[Greg Rose]

From: Tonghao Zhang 

Upstream commit:
commit 90ce9f23a886bdef7a4b7a9bd52c7a50a6a81635
Author: Tonghao Zhang 
Date:   Thu Nov 7 00:34:28 2019 +0800

net: openvswitch: select vport upcall portid directly

The commit 69c51582ff786 ("dpif-netlink: don't allocate per
thread netlink sockets"), in Open vSwitch ovs-vswitchd, has
changed the number of allocated sockets to just one per port
by moving the socket array from a per handler structure to
a per datapath one. In the kernel datapath, a vport will have
only one socket in most case, if so select it directly in
fast-path.

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/vport.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/datapath/vport.c b/datapath/vport.c
index f929282dc..bd62c5612 100644
--- a/datapath/vport.c
+++ b/datapath/vport.c
@@ -507,8 +507,9 @@ u32 ovs_vport_find_upcall_portid(const struct vport *vport, 
struct sk_buff *skb)
 
ids = rcu_dereference(vport->upcall_portids);
 
-   if (ids->n_ids == 1 && ids->ids[0] == 0)
-   return 0;
+   /* If there is only one portid, select it in the fast-path. */
+   if (ids->n_ids == 1)
+   return ids->ids[0];
 
hash = skb_get_hash(skb);
ids_index = hash - ids->n_ids * reciprocal_divide(hash, ids->rn_ids);
-- 
2.17.1

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

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

[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 22a08baa3..ddc0b4491 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 V4 05/24] datapath: Set OvS recirc_id from tc chain index

[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 

---
V4 - Add in portion of commit for ovs_dp_cmd_set which was missed
 in first patch.
---
 acinclude.m4  |  3 ++
 datapath/datapath.c   | 38 ---
 datapath/datapath.h   |  2 +
 datapath/flow.c   | 13 +++
 .../linux/compat/include/linux/static_key.h   |  7 
 5 files changed, 58 insertions(+), 5 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..009887691 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(&tc_recirc_sharing_support);
+   else
+   static_branch_disable(&tc_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();
@@ -1825,7 +1851,9 @@ static int ovs_dp_cmd_set(struct sk_buff *skb, struct 
genl_info *info)
if (IS_ERR(dp))
goto err_unlock_free;
 
-   ovs_dp_change(dp, info->attrs);
+   err = ovs_dp_change(dp, info->attrs);
+   if (err)
+   goto err_unlock_free;
 
  

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

[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 9448a4c1a..1bc8e1439 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(&dp_flow_genl_family, 
&ovs_dp_flow_multicast_group, reply, info);
} else {
genl_set_err(&dp_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 V4 06/24] datapath: fix GFP flags in rtnl_net_notifyid()

[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 009887691..aceb655bd 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -1992,7 +1992,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;
@@ -2014,7 +2014,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;
@@ -2056,11 +2056,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;
@@ -2202,7 +2203,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);
@@ -2262,7 +2263,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();
 
@@ -2302,7 +2303,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 */
@@ -2349,7 +2350,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 V4 15/24] datapath: fix flow command message size

[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 ddc0b4491..1020fee41 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 V4 21/24] datapath: use hlist_for_each_entry_rcu instead of hlist_for_each_entry

[Greg Rose]

From: Tonghao Zhang 

Upstream commit:
commit 64948427a63f49dd0ce403388d232f22cc1971a8
Author: Tonghao Zhang 
Date:   Thu Mar 26 04:27:24 2020 +0800

net: openvswitch: use hlist_for_each_entry_rcu instead of 
hlist_for_each_entry

The struct sw_flow is protected by RCU, when traversing them,
use hlist_for_each_entry_rcu.

Signed-off-by: Tonghao Zhang 
Tested-by: Greg Rose 
Reviewed-by: Greg Rose 
Signed-off-by: David S. Miller 

Compat fixup - OVS doesn't support lockdep_ovsl_is_held() yet

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

diff --git a/datapath/flow_table.c b/datapath/flow_table.c
index bd05dd394..650338fb0 100644
--- a/datapath/flow_table.c
+++ b/datapath/flow_table.c
@@ -485,12 +485,12 @@ static void flow_table_copy_flows(struct table_instance 
*old,
struct hlist_head *head = &old->buckets[i];
 
if (ufid)
-   hlist_for_each_entry(flow, head,
-ufid_table.node[old_ver])
+   hlist_for_each_entry_rcu(flow, head,
+ufid_table.node[old_ver])
ufid_table_instance_insert(new, flow);
else
-   hlist_for_each_entry(flow, head,
-flow_table.node[old_ver])
+   hlist_for_each_entry_rcu(flow, head,
+flow_table.node[old_ver])
table_instance_insert(new, flow);
}
 
-- 
2.17.1

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

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

[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 4619e73bf..b0bce4195 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,8 @@ Post-v2.14.0
  * Removed support for vhost-user dequeue zero-copy.
- The environment variable OVS_UNBOUND_CONF, if set, is now used
  as the DNS resolver's (unbound) configuration file.
+   - 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 V4 16/24] datapath: drop unneeded BUG_ON() in ovs_flow_cmd_build_info()

[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 1020fee41..9448a4c1a 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

Re: [ovs-dev] [PATCH] Revert "travis: Disable check for array of flexible structures in sparse."

[Gregory Rose]

On 10/12/2020 11:15 AM, Ilya Maximets wrote:

This reverts commit 3c6b3a519ae6eae3da4cf7c59894b02b95cdade7.

The fix landed to Sparse main repository [1]:
   b5d46df743be ("flex-array: allow arrays of unions with flexible members.")

[1] https://git.kernel.org/pub/scm/devel/sparse/sparse.git

Signed-off-by: Ilya Maximets 
---
  .travis/linux-build.sh | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
index 6b6935794..6981d1d47 100755
--- a/.travis/linux-build.sh
+++ b/.travis/linux-build.sh
@@ -4,7 +4,7 @@ set -o errexit
  set -x
  
  CFLAGS_FOR_OVS="-g -O2"

-SPARSE_FLAGS="-Wno-flexible-array-array"
+SPARSE_FLAGS=""
  EXTRA_OPTS="--enable-Werror"
  
  function install_kernel()




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

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

[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 V4 22/24] acinclude: Enable builds up to Linux 5.8

[Greg Rose]

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..1460289ca 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.8.x is not supported (please refer to the FAQ for advice)])
fi
 elif test "$version" = 4; then
: # Linux 4.x
-- 
2.17.1

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

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

[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

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

[Greg Rose]

From: aaron conole 

Upstream commit:
commit 5d50aa83e2c8e91ced2cca77c198b468ca9210f4
author: aaron conole 
date:   tue dec 3 16:34:13 2019 -0500

openvswitch: support asymmetric conntrack

the openvswitch module shares a common conntrack and nat infrastructure
exposed via netfilter.  it's possible that a packet needs both snat and
dnat manipulation, due to e.g. tuple collision.  netfilter can support
this because it runs through the nat table twice - once on ingress and
again after egress.  the openvswitch module doesn't have such capability.

like netfilter hook infrastructure, we should run through nat twice to
keep the symmetry.

fixes: 05752523e565 ("openvswitch: interface with nat.")
signed-off-by: aaron conole 
signed-off-by: david s. miller 

Fixes: c5f6c06b58d6 ("datapath: Interface with NAT.")
Cc: aaron conole 
Acked-by: Aaron Conole 
Signed-off-by: Greg Rose 
---
 datapath/conntrack.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/datapath/conntrack.c b/datapath/conntrack.c
index 5b4d6cce0..c7a318baf 100644
--- a/datapath/conntrack.c
+++ b/datapath/conntrack.c
@@ -978,6 +978,17 @@ static int ovs_ct_nat(struct net *net, struct sw_flow_key 
*key,
}
err = ovs_ct_nat_execute(skb, ct, ctinfo, &info->range, maniptype);
 
+   if (err == NF_ACCEPT &&
+   ct->status & IPS_SRC_NAT && ct->status & IPS_DST_NAT) {
+   if (maniptype == NF_NAT_MANIP_SRC)
+   maniptype = NF_NAT_MANIP_DST;
+   else
+   maniptype = NF_NAT_MANIP_SRC;
+
+   err = ovs_ct_nat_execute(skb, ct, ctinfo, &info->range,
+maniptype);
+   }
+
/* Mark NAT done if successful and update the flow key. */
if (err == NF_ACCEPT)
ovs_nat_update_key(key, skb, maniptype);
-- 
2.17.1

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

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

[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: 55, 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 V4 02/24] datapath: drop unneeded likely() call around IS_ERR()

[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 V4 03/24] datapath: do not update max_headroom if new headroom is equal to old headroom

[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 V4 04/24] datapath: Print error when ovs_execute_actions() fails

[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 V4 05/24] datapath: Set OvS recirc_id from tc chain index

[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: 200, 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 V4 06/24] datapath: fix GFP flags in rtnl_net_notifyid()

[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 V4 07/24] datapath: don't unlock mutex when changing the user_features fails

[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 V4 08/24] datapath: optimize flow-mask looking up

[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: 226, 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 V4 09/24] datapath: simplify the flow_hash

[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 V4 10/24] datapath: add likely in flow_lookup

[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: 53, 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 V4 11/24] datapath: fix possible memleak on destroy flow-table

[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: 297, 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 V4 12/24] datapath: simplify the ovs_dp_cmd_new

[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 V4 13/24] datapath: select vport upcall portid directly

[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 V4 14/24] datapath: don't call pad_packet if not necessary

[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 V4 15/24] datapath: fix flow command message size

[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 V4 16/24] datapath: drop unneeded BUG_ON() in ovs_flow_cmd_build_info()

[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 V4 17/24] datapath: remove another BUG_ON()

[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 V4 18/24] datapath: support asymmetric conntrack

[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 V4 19/24] datapath: use skb_list_walk_safe helper for gso segments

[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 V4 20/24] datapath: Distribute switch variables for initialization

[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 V4 21/24] datapath: use hlist_for_each_entry_rcu instead of hlist_for_each_entry

[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: 53, 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] windows: Bump OpenSSL version version

[Alin Gabriel Serdean]

Switch from OpenSSL 1.0.2 to 1.1.1.

Signed-off-by: Alin Gabriel Serdean 
---
 appveyor.yml   | 6 +++---
 m4/ax_check_openssl.m4 | 2 +-
 utilities/ovs-pki.in   | 8 
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/appveyor.yml b/appveyor.yml
index 25c3f69fb..9debf1465 100644
--- a/appveyor.yml
+++ b/appveyor.yml
@@ -15,15 +15,15 @@ init:
 
 mkdir C:\openvswitch\driver
 
-$source = "https://slproweb.com/download/Win64OpenSSL-1_0_2u.exe";
+$source = "https://slproweb.com/download/Win64OpenSSL-1_1_1h.exe";
 
-$destination = "C:\ovs-build-downloads\Win64OpenSSL-1_0_2u.exe"
+$destination = "C:\ovs-build-downloads\Win64OpenSSL-1_1_1h.exe"
 
 Invoke-WebRequest $source -OutFile $destination
 
 cd C:\ovs-build-downloads
 
-.\Win64OpenSSL-1_0_2u.exe /silent /verysilent /sp- /suppressmsgboxes
+.\Win64OpenSSL-1_1_1h.exe /silent /verysilent /sp- /suppressmsgboxes
 
 Start-Sleep -s 30
 
diff --git a/m4/ax_check_openssl.m4 b/m4/ax_check_openssl.m4
index 281d4dc65..37f983284 100644
--- a/m4/ax_check_openssl.m4
+++ b/m4/ax_check_openssl.m4
@@ -81,7 +81,7 @@ AC_DEFUN([AX_CHECK_OPENSSL], [
 SSL_INCLUDES="-I$ssldir/include"
 SSL_LDFLAGS="-L$ssldir/lib"
 if test "$WIN32" = "yes"; then
-SSL_LIBS="-lssleay32 -llibeay32"
+SSL_LIBS="-llibssl -llibcrypto"
 SSL_DIR=/$(echo ${ssldir} | ${SED} -e 's/://')
 else
 SSL_LIBS="-lssl -lcrypto"
diff --git a/utilities/ovs-pki.in b/utilities/ovs-pki.in
index e0ba910f9..c846b69a1 100755
--- a/utilities/ovs-pki.in
+++ b/utilities/ovs-pki.in
@@ -57,6 +57,14 @@ FreeBSD|NetBSD|Darwin)
 ;;
 esac
 
+case $(uname -s) in
+MINGW*|MSYS*)
+mkdir() {
+command mkdir -p "${@: -1}"
+}
+;;
+esac
+
 for option; do
 # This option-parsing mechanism borrowed from a Autoconf-generated
 # configure script under the following license:
-- 
2.27.0.windows.1

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

[ovs-dev] Science In 2020 Is Booming. Restored Health Is Waiting...

[Restore Health Pro]

Having trouble viewing this email? Please follow this link to see the messaged 
emailed to you. 








(Unsubscribe Instructions Here)

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

Re: [ovs-dev] My business is drowning in fees. Inquiry about $24.99/mo unlimited CC processing

[Anne Marie]

I wanted to ask about your business - 
Are you currently paying more than 0. 79% in  VISA/MC Credit Card Processing 
Fees?
Yes that includes interchange fees. 
Give me  3 minutes to explain your alternative.  

1) The only question that needs an answer:

How much is your Credit Card Processor charging you every month? 
  - Are you spending:  $500,  $1000,  $3000, or even more?

Your processor isn' t telling you  about all the available options.   Why are 
they hiding  the lower fee option?
  - Use the new laws - demand the alternative processing model. Were you aware?


2) Here' s your other option (according to US Law: 15 U. S.  Code § 1693o– 2)

We get our merchants running on the  new processing model:
Unlimited Flat-Fee Processing for only $24. 99  per month.   
  - Our businesses get unlimited credit card processing.  
  - Same price each month - thats for  unlimited transactions. 
  - We cover equipment fees - we start you off with a terminal no cost. 
  - We' ll be honest - no surprises, no hidden fees. 

Please do this now:  Click this link to reply - we only need a name and phone 
number to call. 



  September  2020 LIMITED Promotion:


3) Promo: First-Come-First-Serve (Waiting List Available):

Do you want an even better deal?  Email us. 
You still need more incentive?  Email us. 

Email us today - and here' s what we' re offering:

Free Equipment (Max 2x Terminals). 
30-Day Commitment Needed. 
No Cancellation Fees. 
Try Without Obligation. 

Click to send  a quick email:  I'm interested. 

Best,
Anne Lopez




(Image of our most popular equipment & processing terminals. )


4)  How does this payment process work?

Recent  US Law (15 U. S.  Code § 1693o– 2 | U. S.  Code)  places the 
responsibility for processing fees on the card holders.  Those paying with cash 
get a ' cash discount. ' Consumers  understand Covid19 is hurting businesses.   
They want to support you.   Consumer reports studies have shown that 98. 2% of 
customers had no issues at purchase time.   There is a reason  why businesses 
love this!

5)  What steps do I need to take?

1) Don' t cancel your current processor. 
2) Place our new terminal right next to your old one. 
3) Try us out, risk free, no obligation, no time limit. 
4) If you' re not happy, we' ll come get our equipment. 



 



To  Unsubscribe  you  must  click the Unsubscribe link here. 
This is an automated system -  emails or replies  with  " stop" or " remove" do 
not work. 
Your request will be  handled  promptly but please allow 3-5 days  for complete 
removal. 

You may also  write to the address below:
8 The Green STE 8 - Dover, DE 19901





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

Re: [ovs-dev] Established businesses - $24.99/mo credit card processing - unlimited flat-fee

[Anne Marie]

I wanted to ask about your business - 
Are you currently paying more than 0. 79% in  VISA/MC Credit Card Processing 
Fees?
Yes that includes interchange fees. 
Give me  3 minutes to explain your alternative.  

1) The only question that needs an answer:

How much is your Credit Card Processor charging you every month? 
  - Are you spending:  $500,  $1000,  $3000, or even more?

Your processor isn' t telling you  about all the available options.   Why are 
they hiding  the lower fee option?
  - Use the new laws - demand the alternative processing model. Were you aware?


2) Here' s your other option (according to US Law: 15 U. S.  Code § 1693o– 2)

We get our merchants running on the  new processing model:
Unlimited Flat-Fee Processing for only $24. 99  per month.   
  - Our businesses get unlimited credit card processing.  
  - Same price each month - thats for  unlimited transactions. 
  - We cover equipment fees - we start you off with a terminal no cost. 
  - We' ll be honest - no surprises, no hidden fees. 

Please do this now:  Click this link to reply - we only need a name and phone 
number to call. 



  September  2020 LIMITED Promotion:


3) Promo: First-Come-First-Serve (Waiting List Available):

Do you want an even better deal?  Email us. 
You still need more incentive?  Email us. 

Email us today - and here' s what we' re offering:

Free Equipment (Max 2x Terminals). 
30-Day Commitment Needed. 
No Cancellation Fees. 
Try Without Obligation. 

Click to send  a quick email:  I'm interested. 

Best,
Anne Lopez




(Image of our most popular equipment & processing terminals. )


4)  How does this payment process work?

Recent  US Law (15 U. S.  Code § 1693o– 2 | U. S.  Code)  places the 
responsibility for processing fees on the card holders.  Those paying with cash 
get a ' cash discount. ' Consumers  understand Covid19 is hurting businesses.   
They want to support you.   Consumer reports studies have shown that 98. 2% of 
customers had no issues at purchase time.   There is a reason  why businesses 
love this!

5)  What steps do I need to take?

1) Don' t cancel your current processor. 
2) Place our new terminal right next to your old one. 
3) Try us out, risk free, no obligation, no time limit. 
4) If you' re not happy, we' ll come get our equipment. 



 



To  Unsubscribe  you  must  click the Unsubscribe link here. 
This is an automated system -  emails or replies  with  " stop" or " remove" do 
not work. 
Your request will be  handled  promptly but please allow 3-5 days  for complete 
removal. 

You may also  write to the address below:
8 The Green STE 8 - Dover, DE 19901





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

[ovs-dev] [PATCH ovn] tests: Use ovn-nbctl --wait=hv for DHCP option tests.

[Gregory Smith]

This patch makes the "dhcpv4 : 1 HV, 2 LS, 2 LSPs/LS" testcase more
deterministic, by waiting for updates to DHCP options to take effect on
the hypervisor, before sending a DHCP request.

Fixes: b06319993deb ("Fix the data type for DHCP option tftp_server (66)")
Fixes: d79bb92c4b49 ("Add support for DHCP domain search option (119)")
Signed-off-by: Gregory Smith 
---
 tests/ovn.at | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/ovn.at b/tests/ovn.at
index 6f1ab5926..488fd119b 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -5815,7 +5815,7 @@ rm -f 2.expected
 
 # Set tftp server option (IPv4 address) for ls1
 echo "-- Set tftp server (IPv4 address) "
-ovn-nbctl dhcp-options-set-options $d1 server_id=10.0.0.1 \
+ovn-nbctl --wait=hv dhcp-options-set-options $d1 server_id=10.0.0.1 \
 server_mac=ff:10:00:00:00:01 lease_time=3600 router=10.0.0.1 \
 tftp_server=10.10.10.10
 echo "--"
@@ -5846,7 +5846,7 @@ rm -f 2.expected
 
 # Set tftp server option (Hostname) for ls1
 echo "-- Set tftp server (hostname) "
-ovn-nbctl dhcp-options-set-options $d1 server_id=10.0.0.1 \
+ovn-nbctl --wait=hv dhcp-options-set-options $d1 server_id=10.0.0.1 \
 server_mac=ff:10:00:00:00:01 lease_time=3600 router=10.0.0.1 \
 tftp_server=\"test_tftp_server\"
 echo "--"
@@ -5877,7 +5877,7 @@ rm -f 2.expected
 
 # Set domain search list option for ls1
 echo "-- Set domain search list "
-ovn-nbctl dhcp-options-set-options $d1 server_id=10.0.0.1 \
+ovn-nbctl --wait=hv dhcp-options-set-options $d1 server_id=10.0.0.1 \
 server_mac=ff:10:00:00:00:01 lease_time=3600 router=10.0.0.1 \
 domain_search_list=\"test1.com,test2.com\"
 echo "--"
-- 
2.28.0

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

Re: [ovs-dev] Referred by friend - Still offering $24.99/mo unlimited flat-fee credit card processing?

[Anne Marie]

I wanted to ask about your business - 
Are you currently paying more than 0. 79% in  VISA/MC Credit Card Processing 
Fees?
Yes that includes interchange fees. 
Give me  3 minutes to explain your alternative.  

1) The only question that needs an answer:

How much is your Credit Card Processor charging you every month? 
  - Are you spending:  $500,  $1000,  $3000, or even more?

Your processor isn' t telling you  about all the available options.   Why are 
they hiding  the lower fee option?
  - Use the new laws - demand the alternative processing model. Were you aware?


2) Here' s your other option (according to US Law: 15 U. S.  Code § 1693o– 2)

We get our merchants running on the  new processing model:
Unlimited Flat-Fee Processing for only $24. 99  per month.   
  - Our businesses get unlimited credit card processing.  
  - Same price each month - thats for  unlimited transactions. 
  - We cover equipment fees - we start you off with a terminal no cost. 
  - We' ll be honest - no surprises, no hidden fees. 

Please do this now:  Click this link to reply - we only need a name and phone 
number to call. 



  September  2020 LIMITED Promotion:


3) Promo: First-Come-First-Serve (Waiting List Available):

Do you want an even better deal?  Email us. 
You still need more incentive?  Email us. 

Email us today - and here' s what we' re offering:

Free Equipment (Max 2x Terminals). 
30-Day Commitment Needed. 
No Cancellation Fees. 
Try Without Obligation. 

Click to send  a quick email:  I'm interested. 

Best,
Anne Lopez




(Image of our most popular equipment & processing terminals. )


4)  How does this payment process work?

Recent  US Law (15 U. S.  Code § 1693o– 2 | U. S.  Code)  places the 
responsibility for processing fees on the card holders.  Those paying with cash 
get a ' cash discount. ' Consumers  understand Covid19 is hurting businesses.   
They want to support you.   Consumer reports studies have shown that 98. 2% of 
customers had no issues at purchase time.   There is a reason  why businesses 
love this!

5)  What steps do I need to take?

1) Don' t cancel your current processor. 
2) Place our new terminal right next to your old one. 
3) Try us out, risk free, no obligation, no time limit. 
4) If you' re not happy, we' ll come get our equipment. 



 



To  Unsubscribe  you  must  click the Unsubscribe link here. 
This is an automated system -  emails or replies  with  " stop" or " remove" do 
not work. 
Your request will be  handled  promptly but please allow 3-5 days  for complete 
removal. 

You may also  write to the address below:
8 The Green STE 8 - Dover, DE 19901





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

Re: [ovs-dev] [PATCH ovn v4] ofctrl: Add a predictable resolution for conflicting flow actions.

[Han Zhou]

On Mon, Oct 12, 2020 at 3:16 AM Dumitru Ceara  wrote:
>
>
> On 10/11/20 9:40 PM, Han Zhou wrote:
> >
> >
> > On Sun, Oct 11, 2020 at 5:14 AM Dumitru Ceara  > > wrote:
> >>
> >> On 10/6/20 10:30 AM, Dumitru Ceara wrote:
> >>> On 10/2/20 10:28 PM, Han Zhou wrote:
> 
>  Hi Dumitru,
> 
> >>>
> >>> Hi Han,
> >>>
>  Thanks for the revision. It looks good overall. The major
>  problems left are: 1. it missed updating the active
>  desired_flow in the installed_flow. 2. when a tracked flow
>  deletion is handled, if there are other desired flows linked to
>  the same installed flow, it should use the same criteria to
>  decide which flow should become active from the candidate
>  flows.
> 
> >>>
> >>> I see, you're right, thanks. I'll fix it in the next revision.
> >>>
>  I also noticed 2 problems of the existing code while reviewing
>  this patch. I submitted 2 patches: 1.
> 
https://patchwork.ozlabs.org/project/ovn/patch/1601663136-19111-1-git-send-email-hz...@ovn.org/
> >>
> 
> >> 2.
> 
https://patchwork.ozlabs.org/project/ovn/patch/20201002200504.2954064-1-hz...@ovn.org/
> >>
> 
> >>
>  1) is required for your solution to work properly. 2) is not
>  directly related but will cause a merge conflict. Please help
>  review both since they are closely related to your patch.
> 
> >>>
> >>> I'll try to review your patches as soon as possible and I'll
> >>> respin mine afterwards.
> >>>
>  Please see more detailed comments below.
> 
>  On Wed, Sep 30, 2020 at 12:39 AM Dumitru Ceara
>  mailto:dce...@redhat.com>
>  >> wrote:
> >
> > Until now, in case the ACL configuration generates openflows
> > that have the same match but different actions,
> > ovn-controller was using the following approach: 1. If the
> > flow being added contains conjunctive actions, merge its
> > actions with the already existing flow. 2. Otherwise, if the
> > flow is being added incrementally
> > (update_installed_flows_by_track), don't install the new flow
> > but instead keep the old one. 3. Otherwise,
> > (update_installed_flows_by_compare), don't install the new
> > flow but instead keep the old one.
> >
> > Even though one can argue that having an ACL with a match
> > that includes the match of another ACL is a misconfiguration,
> > it can happen that the users provision OVN like this.
> > Depending on the order of reading and installing the logical
> > flows, the above operations can yield unpredictable results,
> > e.g., allow specific traffic but then after ovn-controller is
> > restarted (or a recompute happens) that specific traffic
> > starts being dropped.
> >
> > A simple example of ACL configuration is: ovn-nbctl acl-add
> > ls to-lport 3 '(ip4.src==10.0.0.1 || ip4.src==10.0.0.2) &&
> > (ip4.dst == 10.0.0.3 || ip4.dst == 10.0.0.4)' allow ovn-nbctl
> > acl-add ls to-lport 3 'ip4.src==10.0.0.1' allow ovn-nbctl
> > acl-add ls to-lport 2 'arp' allow ovn-nbctl acl-add ls
> > to-lport 1 'ip4' drop
> >
> > This is a pattern used by most CMSs: - define a default deny
> > policy. - punch holes in the default deny policy based on
> > user specific configurations.
> >
> > Without this commit the behavior for traffic from 10.0.0.1 to
> > 10.0.0.5 is unpredictable. Depending on the order of
> > operations traffic might be dropped or allowed.
> >
> > It's also quite hard to force the CMS to ensure that such
> > match overlaps never occur.
> >
> > To address this issue we now resolve conflicts between flows
> > with the same match and different actions by giving
> > precedence to less restrictive flows. This means that if the
> > installed flow has action "conjunction" and the desired flow
> > doesn't then we prefer the desired flow. Similarly, if the
> > desired flow has action "conjunction" and the installed flow
> > doesn't then we prefer the already installed flow.
> >
> > CC: Daniel Alvarez  >   > >> CC: Han Zhou  >   > >> Reported-at:
> > https://bugzilla.redhat.com/1871931 Acked-by: Mark Michelson
> > mailto:mmich...@redhat.com>
>  >>
> > Signed-off-by: Dumitru Ceara  > 
>  >>
> > --- v4: - Address Han's comments: - make sure only flows with
> > action conjunction are combined. v3: - Add Mark's ack. - Add
> > last missing pcap check in the test. v2: - Address Han's
> > comments: - Do not delete desired flow that cannot be merged,
> >