Re: [ovs-dev] [PATCH] datapath: meter: Fix setting meter id for new entries

[Justin Pettit]

> On Aug 7, 2018, at 10:19 PM, Gregory Rose  wrote:
> 
> 
> On 8/7/2018 8:31 PM, Justin Pettit wrote:
>> Thanks, Greg. I actually have this queued up with another patch that will 
>> disable meters entirely on broken kernels. I plan to send that out tomorrow.
>> 
>> --Justin
> 
> Oops - perhaps I was a bit hasty.  I was just trying to get caught up with 
> upstream.
> 
> Apologies if I interrupted your work flow.

No problem at all.  I appreciate you staying on top of this!  We'd already had 
a few patches slip through the cracks recently, so it's great to have you 
keeping them in sync.

--Justin


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

Re: [ovs-dev] [PATCH] datapath: meter: Fix setting meter id for new entries

[Justin Pettit]

That's fine.  I had just been holding off because I thought you'd wanted the 
probe to disable meters on kernels with a broken implementation.  Regardless, I 
did get that patch out earlier today:

https://mail.openvswitch.org/pipermail/ovs-dev/2018-August/350885.html

Thanks,

--Justin


> On Aug 8, 2018, at 8:12 AM, Ben Pfaff  wrote:
> 
> Justin, I already applied this as a straightforward backport.  Hope it
> doesn't disrupt your work.
> 
> On Tue, Aug 07, 2018 at 08:31:35PM -0700, Justin Pettit wrote:
>> Thanks, Greg. I actually have this queued up with another patch that will 
>> disable meters entirely on broken kernels. I plan to send that out tomorrow. 
>> 
>> --Justin
>> 
>> 
>>> On Aug 7, 2018, at 4:45 PM, Greg Rose  wrote:
>>> 
>>> From: Justin Pettit 
>>> 
>>> Upstream commit:
>>>   From: Justin Pettit 
>>>   Date: Sat, 28 Jul 2018 15:26:01 -0700
>>>   Subject: [PATCH] openvswitch: meter: Fix setting meter id for new entries
>>> 
>>>   The meter code would create an entry for each new meter.  However, it
>>>   would not set the meter id in the new entry, so every meter would appear
>>>   to have a meter id of zero.  This commit properly sets the meter id when
>>>   adding the entry.
>>> 
>>>   Fixes: 96fbc13d7e77 ("openvswitch: Add meter infrastructure")
>>>   Signed-off-by: Justin Pettit 
>>>   Cc: Andy Zhou 
>>>   Signed-off-by: David S. Miller 
>>> 
>>> Cc: Justin Pettit 
>>> Signed-off-by: Greg Rose 
>>> ---
>>> datapath/meter.c | 10 +-
>>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>> 
>>> diff --git a/datapath/meter.c b/datapath/meter.c
>>> index 1c2ed46..281d869 100644
>>> --- a/datapath/meter.c
>>> +++ b/datapath/meter.c
>>> @@ -221,6 +221,7 @@ static struct dp_meter *dp_meter_create(struct nlattr 
>>> **a)
>>>   if (!meter)
>>>   return ERR_PTR(-ENOMEM);
>>> 
>>> +meter->id = nla_get_u32(a[OVS_METER_ATTR_ID]);
>>>   meter->used = div_u64(ktime_get_ns(), 1000 * 1000);
>>>   meter->kbps = a[OVS_METER_ATTR_KBPS] ? 1 : 0;
>>>   meter->keep_stats = !a[OVS_METER_ATTR_CLEAR];
>>> @@ -290,6 +291,10 @@ static int ovs_meter_cmd_set(struct sk_buff *skb, 
>>> struct genl_info *info)
>>>   u32 meter_id;
>>>   bool failed;
>>> 
>>> +if (!a[OVS_METER_ATTR_ID]) {
>>> +return -ENODEV;
>>> +}
>>> +
>>>   meter = dp_meter_create(a);
>>>   if (IS_ERR_OR_NULL(meter))
>>>   return PTR_ERR(meter);
>>> @@ -308,11 +313,6 @@ static int ovs_meter_cmd_set(struct sk_buff *skb, 
>>> struct genl_info *info)
>>>   goto exit_unlock;
>>>   }
>>> 
>>> -if (!a[OVS_METER_ATTR_ID]) {
>>> -err = -ENODEV;
>>> -goto exit_unlock;
>>> -}
>>> -
>>>   meter_id = nla_get_u32(a[OVS_METER_ATTR_ID]);
>>> 
>>>   /* Cannot fail after this. */
>>> -- 
>>> 1.8.3.1
>>> 
>> 
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

Re: [ovs-dev] [PATCH] releases: Add 2.10 entry for supported DPDK versions.

[Flavio Leitner]

On Wed, Aug 08, 2018 at 12:00:30PM +0100, Ian Stokes wrote:
> This commit adds an entry for OVS 2.10 and the supported DPDK version in
> releases.rst.
> 
> Signed-off-by: Ian Stokes 
> ---

Acked-by: Flavio Leitner 


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

Re: [ovs-dev] [PATCH] ofproto-dpif-xlate: Improve log message.

[Flavio Leitner]

On Tue, Aug 07, 2018 at 11:18:56AM -0700, Ben Pfaff wrote:
> Until now, the bridge name was at the end of the log message, after the
> flow, which made it easy to miss.  This commit moves it before the flow
> where it is easier to spot.
> 
> Signed-off-by: Ben Pfaff 
> ---

Acked-by: Flavio Leitner 


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

Re: [ovs-dev] [PATCH 1/8] ip6_gre: Fix a bug that clears address bits

[William Tu]

thanks for the fix.

On Wed, Aug 8, 2018 at 11:32 AM, Yifeng Sun  wrote:

> In compatable gre module, skb->cb is used as ovs_gso_cb.
> This bug clears the 16-23 bit in the address of ovs_gso_cb.tun_dst.
>

can you explain more about ovs_gso_cb?

>
> Signed-off-by: Yifeng Sun 
> ---
>  datapath/linux/compat/ip6_gre.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/datapath/linux/compat/ip6_gre.c b/datapath/linux/compat/ip6_
> gre.c
> index 54a76ab..16c1f72 100644
> --- a/datapath/linux/compat/ip6_gre.c
> +++ b/datapath/linux/compat/ip6_gre.c
> @@ -1146,7 +1146,6 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct
> sk_buff *skb,
> goto tx_err;
>
> t->parms.o_flags &= ~TUNNEL_KEY;
> -   IPCB(skb)->flags = 0;
>

The upstream linux kernel has the above code.
Do we need to fix the upstream kernel then backport?

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

[ovs-dev] [PATCH 1/2] dpif: Don't pass in '*meter_id' to meter_set commands.

[Justin Pettit]

The original intent of the API appears to be that the underlying DPIF
implementaion would choose a local meter id.  However, neither of the
existing datapath meter implementations (userspace or Linux) implemented
that; they expected a valid meter id to be passed in, otherwise they
returned an error.  This commit follows the existing implementations and
makes the API somewhat cleaner.

Signed-off-by: Justin Pettit 
---
 lib/dpif-netdev.c  |  4 ++--
 lib/dpif-netlink.c | 13 -
 lib/dpif-provider.h|  9 -
 lib/dpif.c | 14 ++
 lib/dpif.h |  2 +-
 ofproto/ofproto-dpif.c |  2 +-
 6 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 26d07b39c9af..8c9062485bf7 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -5160,11 +5160,11 @@ dp_netdev_run_meter(struct dp_netdev *dp, struct 
dp_packet_batch *packets_,
 
 /* Meter set/get/del processing is still single-threaded. */
 static int
-dpif_netdev_meter_set(struct dpif *dpif, ofproto_meter_id *meter_id,
+dpif_netdev_meter_set(struct dpif *dpif, ofproto_meter_id meter_id,
   struct ofputil_meter_config *config)
 {
 struct dp_netdev *dp = get_dp_netdev(dpif);
-uint32_t mid = meter_id->uint32;
+uint32_t mid = meter_id.uint32;
 struct dp_meter *meter;
 int i;
 
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index f669b1108d61..bf94e5413e71 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -3030,7 +3030,7 @@ dpif_netlink_meter_get_features(const struct dpif *dpif_,
 }
 
 static int
-dpif_netlink_meter_set(struct dpif *dpif_, ofproto_meter_id *meter_id,
+dpif_netlink_meter_set(struct dpif *dpif_, ofproto_meter_id meter_id,
struct ofputil_meter_config *config)
 {
 struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
@@ -3057,9 +3057,8 @@ dpif_netlink_meter_set(struct dpif *dpif_, 
ofproto_meter_id *meter_id,
 
 dpif_netlink_meter_init(dpif, , stub, sizeof stub, OVS_METER_CMD_SET);
 
-if (meter_id->uint32 != UINT32_MAX) {
-nl_msg_put_u32(, OVS_METER_ATTR_ID, meter_id->uint32);
-}
+nl_msg_put_u32(, OVS_METER_ATTR_ID, meter_id.uint32);
+
 if (config->flags & OFPMF13_KBPS) {
 nl_msg_put_flag(, OVS_METER_ATTR_KBPS);
 }
@@ -3098,7 +3097,11 @@ dpif_netlink_meter_set(struct dpif *dpif_, 
ofproto_meter_id *meter_id,
 return error;
 }
 
-meter_id->uint32 = nl_attr_get_u32(a[OVS_METER_ATTR_ID]);
+if (nl_attr_get_u32(a[OVS_METER_ATTR_ID]) != meter_id.uint32) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+VLOG_INFO_RL(,
+ "Kernel returned a different meter id than requested");
+}
 ofpbuf_delete(msg);
 return 0;
 }
diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 62b3598acfc5..8906d4e0a1e6 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -451,12 +451,11 @@ struct dpif_class {
 void (*meter_get_features)(const struct dpif *,
struct ofputil_meter_features *);
 
-/* Adds or modifies 'meter' in 'dpif'.   If '*meter_id' is UINT32_MAX,
- * adds a new meter, otherwise modifies an existing meter.
+/* Adds or modifies the meter in 'dpif' with the given 'meter_id'
+ * and the configuration in 'config'.
  *
- * If meter is successfully added, sets '*meter_id' to the new meter's
- * meter id selected by 'dpif'. */
-int (*meter_set)(struct dpif *, ofproto_meter_id *meter_id,
+ * The meter id specified through 'config->meter_id' is ignored. */
+int (*meter_set)(struct dpif *, ofproto_meter_id meter_id,
  struct ofputil_meter_config *);
 
 /* Queries 'dpif' for meter stats with the given 'meter_id'.  Stores
diff --git a/lib/dpif.c b/lib/dpif.c
index c267bcfb0c55..d799f972cbf6 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1886,13 +1886,12 @@ dpif_meter_get_features(const struct dpif *dpif,
 }
 }
 
-/* Adds or modifies meter identified by 'meter_id' in 'dpif'.  If '*meter_id'
- * is UINT32_MAX, adds a new meter, otherwise modifies an existing meter.
+/* Adds or modifies the meter in 'dpif' with the given 'meter_id' and
+ * the configuration in 'config'.
  *
- * If meter is successfully added, sets '*meter_id' to the new meter's
- * meter number. */
+ * The meter id specified through 'config->meter_id' is ignored. */
 int
-dpif_meter_set(struct dpif *dpif, ofproto_meter_id *meter_id,
+dpif_meter_set(struct dpif *dpif, ofproto_meter_id meter_id,
struct ofputil_meter_config *config)
 {
 COVERAGE_INC(dpif_meter_set);
@@ -1918,11 +1917,10 @@ dpif_meter_set(struct dpif *dpif, ofproto_meter_id 
*meter_id,
 int error = dpif->dpif_class->meter_set(dpif, meter_id, config);
 if (!error) {
 VLOG_DBG_RL(_rl, "%s: DPIF meter %"PRIu32" set",
-dpif_name(dpif), meter_id->uint32);
+

[ovs-dev] [PATCH 2/2] dpif-netlink: Probe for broken Linux meter implementations.

[Justin Pettit]

Meter support was introduced in Linux 4.15.  In some versions of Linux
4.15, 4.16, and 4.17, there was a bug that never set the id when the
meter was created, so all meters essentially had an id of zero.  This
commit adds a probe to check for that condition and disable meters on
those kernels.

Signed-off-by: Justin Pettit 
---
 lib/dpif-netlink.c | 54 ++
 1 file changed, 54 insertions(+)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index bf94e5413e71..60ce1a6d22a3 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -2926,6 +2926,12 @@ dpif_netlink_ct_flush(struct dpif *dpif OVS_UNUSED, 
const uint16_t *zone,
 #define DP_SUPPORTED_METER_FLAGS_MASK \
 (OFPMF13_STATS | OFPMF13_PKTPS | OFPMF13_KBPS | OFPMF13_BURST)
 
+/* Meter support was introduced in Linux 4.15.  In some versions of
+ * Linux 4.15, 4.16, and 4.17, there was a bug that never set the id
+ * when the meter was created, so all meters essentially had an id of
+ * zero.  Check for that condition and disable meters on those kernels. */
+static bool probe_broken_meters(struct dpif *);
+
 static void
 dpif_netlink_meter_init(struct dpif_netlink *dpif, struct ofpbuf *buf,
 void *stub, size_t size, uint32_t command)
@@ -2977,6 +2983,11 @@ static void
 dpif_netlink_meter_get_features(const struct dpif *dpif_,
 struct ofputil_meter_features *features)
 {
+if (probe_broken_meters((struct dpif *)dpif_)) {
+features = NULL;
+return;
+}
+
 struct ofpbuf buf, *msg;
 uint64_t stub[1024 / 8];
 
@@ -3033,6 +3044,10 @@ static int
 dpif_netlink_meter_set(struct dpif *dpif_, ofproto_meter_id meter_id,
struct ofputil_meter_config *config)
 {
+if (probe_broken_meters(dpif_)) {
+return ENOMEM;
+}
+
 struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
 struct ofpbuf buf, *msg;
 uint64_t stub[1024 / 8];
@@ -3206,6 +3221,45 @@ dpif_netlink_meter_del(struct dpif *dpif, 
ofproto_meter_id meter_id,
 OVS_METER_CMD_DEL);
 }
 
+static bool
+probe_broken_meters(struct dpif *dpif)
+{
+static bool checked = false;
+static bool broken_meters = false;
+
+if (checked) {
+return broken_meters;
+}
+checked = true;
+
+/* This test is destructive if a probe occurs while ovs-vswitchd is
+ * running (e.g., an ovs-dpctl meter command is called), so choose a
+ * random high meter id to make this less likely to occur. */
+ofproto_meter_id id1 = { 54545401 };
+ofproto_meter_id id2 = { 54545402 };
+struct ofputil_meter_band band = {OFPMBT13_DROP, 0, 1, 0};
+struct ofputil_meter_config config1 = { 1, OFPMF13_KBPS, 1, };
+struct ofputil_meter_config config2 = { 2, OFPMF13_KBPS, 1, };
+
+/* Try adding two meters and make sure that they both come back with
+ * the proper meter id. */
+dpif_netlink_meter_set(dpif, id1, );
+dpif_netlink_meter_set(dpif, id2, );
+
+if (dpif_netlink_meter_get(dpif, id1, NULL, 0)
+|| dpif_netlink_meter_get(dpif, id2, NULL, 0)) {
+VLOG_INFO("The kernel module has a broken meter implementation.");
+broken_meters = true;
+goto done;
+}
+
+dpif_netlink_meter_del(dpif, id1, NULL, 0);
+dpif_netlink_meter_del(dpif, id2, NULL, 0);
+
+done:
+return broken_meters;
+}
+
 
 const struct dpif_class dpif_netlink_class = {
 "system",
-- 
2.17.1

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

Re: [ovs-dev] OVS DPDK: dpdk_merge pull request for branch-2.10

[0-day Robot]

Bleep bloop.  Greetings Ian Stokes, 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: Too many signoffs; are you missing Co-authored-by lines?
Lines checked: 101, Warnings: 0, Errors: 1


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

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

[ovs-dev] [PATCH 8/8] system-traffic.at: Add ip6erspan v2 tunnel test that doesn't depend on upstream gre module

[Yifeng Sun]

Introduce a new test that doesn't setup native ip6erspan v2 tunnels but sends
simulated raw packets.
This test is supposed to only run for kernel version from 4.4.x to 4.15.x.

Signed-off-by: Yifeng Sun 
---
 tests/system-traffic.at | 47 +++
 1 file changed, 47 insertions(+)

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 60fd032..1eeaff0 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -765,6 +765,53 @@ AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP6 
fc00:100::100 > fc00:100::1:
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([datapath - ping over ip6erspan v2 tunnel by simulated packets])
+OVS_CHECK_KERNEL(4, 4, 15)
+
+OVS_TRAFFIC_VSWITCHD_START()
+AT_CHECK([ovs-vsctl -- set bridge br0 
other-config:hwaddr=\"f2:ff:00:00:00:01\"])
+ADD_BR([br-underlay], [set bridge br-underlay 
other-config:hwaddr=\"f2:ff:00:00:00:02\"])
+
+AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
+AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
+
+ADD_NAMESPACES(at_ns0)
+
+dnl Set up underlay link from host into the namespace using veth pair.
+ADD_VETH(p0, at_ns0, br-underlay, "fc00:100::1/96", f2:ff:00:00:00:03, [], 
nodad)
+AT_CHECK([ip addr add dev br-underlay "fc00:100::100/96" nodad])
+AT_CHECK([ip link set dev br-underlay up])
+
+dnl Set up tunnel endpoints on OVS outside the namespace and simulate a native
+dnl linux device inside the namespace.
+ADD_OVS_TUNNEL6([ip6erspan], [br0], [at_erspan0], [fc00:100::1], 
[10.1.1.100/24],
+[options:key=121 options:erspan_ver=2 options:erspan_dir=0 
options:erspan_hwid=0x7])
+
+OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 2 fc00:100::100])
+
+ip netns exec at_ns0 tcpdump -U -i p0 -w p0.pcap &
+sleep 1
+
+dnl First, check the underlay.
+NS_CHECK_EXEC([at_ns0], [ping6 -q -c 3 -i 0.3 -w 2 fc00:100::100 | 
FORMAT_PING], [0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+dnl Okay, now send raw arp request and icmp echo request.
+ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 
packet=f2ff0002f2ff000386dd60008531003e2f40fc000101fc0001000100100088be00062079af514f998070f2ff000408060001080006040001f2ff00040a0101010a010164
 actions=normal"
+
+sleep 1
+AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "0806 0001 0800 0604 0002" 2>&1 
1>/dev/null])
+AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "0a01 0164 f2ff  0004" 2>&1 
1>/dev/null])
+
+ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 
packet=f2ff0002f2ff000386dd60008531007e3c40fc000101fc00010001002f00040104010100100088be000720004079af514f9b8070f2ff0001f2ff000408004554ffcb4000400124770a0101010a0101640800419e23ac000112d7685b4caf0c00101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637
 actions=normal"
+
+sleep 1
+AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP6 fc00:100::100 > 
fc00:100::1: GREv0, .* length 118" 2>&1 1>/dev/null])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([datapath - clone action])
 OVS_TRAFFIC_VSWITCHD_START()
 
-- 
2.7.4

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

[ovs-dev] [PATCH 7/8] system-traffic.at: Add ip6erspan v1 tunnel test that doesn't depend on upstream gre module

[Yifeng Sun]

Introduce a new test that doesn't setup native ip6erspan v1 tunnels but sends
simulated raw packets.
This test is supposed to only run for kernel version from 4.4.x to 4.15.x.

Signed-off-by: Yifeng Sun 
---
 tests/system-traffic.at | 51 +
 1 file changed, 51 insertions(+)

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 64b37df..60fd032 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -714,6 +714,57 @@ AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP 
172.31.1.100 > 172.31.1.1: GRE
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([datapath - ping over ip6erspan v1 tunnel by simulated packets])
+OVS_CHECK_KERNEL(4, 4, 15)
+
+OVS_TRAFFIC_VSWITCHD_START()
+AT_CHECK([ovs-vsctl -- set bridge br0 
other-config:hwaddr=\"f2:ff:00:00:00:01\"])
+ADD_BR([br-underlay], [set bridge br-underlay 
other-config:hwaddr=\"f2:ff:00:00:00:02\"])
+
+AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
+AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
+
+ADD_NAMESPACES(at_ns0)
+
+dnl Set up underlay link from host into the namespace using veth pair.
+ADD_VETH(p0, at_ns0, br-underlay, "fc00:100::1/96", f2:ff:00:00:00:03, [], 
nodad)
+AT_CHECK([ip addr add dev br-underlay "fc00:100::100/96" nodad])
+AT_CHECK([ip link set dev br-underlay up])
+
+dnl Set up tunnel endpoints on OVS outside the namespace and simulate a native
+dnl linux device inside the namespace.
+ADD_OVS_TUNNEL6([ip6erspan], [br0], [at_erspan0], [fc00:100::1], 
[10.1.1.100/24],
+[options:key=123 options:erspan_ver=1 options:erspan_idx=0x7])
+
+OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 2 fc00:100::100])
+
+ip netns exec at_ns0 tcpdump -U -i p0 -w p0.pcap &
+sleep 1
+
+dnl First, check the underlay
+NS_CHECK_EXEC([at_ns0], [ping6 -q -c 3 -i 0.3 -w 2 fc00:100::100 | 
FORMAT_PING], [0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+dnl Okay, now send raw arp request and icmp echo request.
+ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 
packet=f2ff0002f2ff000386dd60008531003a2f40fc000101fc0001000100100088be0005107b0007f2ff000408060001080006040001f2ff00040a0101010a010164
 actions=normal"
+
+sleep 1
+dnl check arp reply
+AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "0806 0001 0800 0604 0002 f2ff 
" 2>&1 1>/dev/null])
+AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "0a01 0164 f2ff  0004 0a01 
0101" 2>&1 1>/dev/null])
+
+AT_FAIL_IF([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP6 fc00:100::100 > 
fc00:100::1: GREv0, .* length 114" 2>&1 1>/dev/null])
+
+ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 
packet=f2ff0002f2ff000386dd60008531007a3c40fc000101fc00010001002f00040104010100100088be00061000407b0007f2ff0001f2ff00040800455429b640004001fa8c0a0101010a01016408005c2c7526000118d3685be4aa0200101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637
 actions=normal"
+
+sleep 1
+dnl check icmp reply
+AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP6 fc00:100::100 > 
fc00:100::1: GREv0, .* length 114" 2>&1 1>/dev/null])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([datapath - clone action])
 OVS_TRAFFIC_VSWITCHD_START()
 
-- 
2.7.4

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

[ovs-dev] [PATCH 6/8] system-traffic.at: Add erspan v2 tunnel test that doesn't depend on upstream gre module

[Yifeng Sun]

Introduce a new test that doesn't setup native erspan v2 tunnels but sends
simulated raw packets.
This test is supposed to only run for kernel version from 4.4.x to 4.15.x.

Signed-off-by: Yifeng Sun 
---
 tests/system-traffic.at | 44 
 1 file changed, 44 insertions(+)

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 44669f8..64b37df 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -670,6 +670,50 @@ AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP 
172.31.1.100 > 172.31.1.1: GRE
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([datapath - ping over erspan v2 tunnel by simulated packets])
+OVS_CHECK_KERNEL(4, 4, 15)
+
+OVS_TRAFFIC_VSWITCHD_START()
+AT_CHECK([ovs-vsctl -- set bridge br0 
other-config:hwaddr=\"f2:ff:00:00:00:01\"])
+ADD_BR([br-underlay], [set bridge br-underlay 
other-config:hwaddr=\"f2:ff:00:00:00:02\"])
+
+AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
+AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
+
+ADD_NAMESPACES(at_ns0)
+
+dnl Set up underlay link from host into the namespace using veth pair.
+ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24", f2:ff:00:00:00:03)
+AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
+AT_CHECK([ip link set dev br-underlay up])
+
+dnl Set up tunnel endpoints on OVS outside the namespace and simulate a native
+dnl linux device inside the namespace.
+ADD_OVS_TUNNEL([erspan], [br0], [at_erspan0], [172.31.1.1], [10.1.1.100/24], 
[options:key=1 options:erspan_ver=2 options:erspan_dir=1 
options:erspan_hwid=0x7])
+
+ip netns exec at_ns0 tcpdump -U -i p0 -w p0.pcap &
+sleep 1
+
+dnl First, check the underlay
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 | FORMAT_PING], 
[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+dnl Okay, send raw arp request and icmp echo request.
+ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 
packet=f2ff0002f2ff000308004552373d4000402fa89cac1f0101ac1f0164100088be000620016f54b4178078f2ff000408060001080006040001f2ff00040a0101010a010164
 actions=normal"
+
+sleep 1
+AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "0806 0001 0800 0604 0002 f2ff 
" 2>&1 1>/dev/null])
+AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "0a01 0164 f2ff  0004 0a01 
0101" 2>&1 1>/dev/null])
+
+ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 
packet=f2ff0002f2ff00030800459287e14000402f57b8ac1f0101ac1f0164100088be00052001144cd5a48078f2ff0001f2ff00040800455c38d640004001eb640a0101010a01016408005e57585f0001df6c6b5b45bc0500101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f
 actions=normal"
+
+sleep 1
+AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP 172.31.1.100 > 172.31.1.1: 
GREv0, .* length 126" 2>&1 1>/dev/null])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([datapath - clone action])
 OVS_TRAFFIC_VSWITCHD_START()
 
-- 
2.7.4

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

[ovs-dev] [PATCH 5/8] system-traffic.at: Add erspan v1 tunnel test that doesn't depend on upstream gre module

[Yifeng Sun]

Introduce a new test that doesn't setup native erspan v1 tunnels but sends
simulated raw packets.
This test is supposed to only run for kernel version from 4.4.x to 4.15.x.

Signed-off-by: Yifeng Sun 
---
 tests/system-traffic.at | 48 
 1 file changed, 48 insertions(+)

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index dca2bc8..44669f8 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -622,6 +622,54 @@ AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP 
172.31.1.100 > 172.31.1.1: GRE
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([datapath - ping over erspan v1 tunnel by simulated packets])
+OVS_CHECK_KERNEL(4, 4, 15)
+
+OVS_TRAFFIC_VSWITCHD_START()
+AT_CHECK([ovs-vsctl -- set bridge br0 
other-config:hwaddr=\"f2:ff:00:00:00:01\"])
+ADD_BR([br-underlay], [set bridge br-underlay 
other-config:hwaddr=\"f2:ff:00:00:00:02\"])
+
+AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
+AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
+
+ADD_NAMESPACES(at_ns0)
+
+dnl Set up underlay link from host into the namespace using veth pair.
+ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24", f2:ff:00:00:00:03)
+AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
+AT_CHECK([ip link set dev br-underlay up])
+
+dnl Set up tunnel endpoints on OVS outside the namespace and emulate a native
+dnl linux device inside the namespace.
+ADD_OVS_TUNNEL([erspan], [br0], [at_erspan0], [172.31.1.1], [10.1.1.100/24], 
[options:key=1 options:erspan_ver=1 options:erspan_idx=7])
+
+ip netns exec at_ns0 tcpdump -U -i p0 -w p0.pcap &
+sleep 1
+
+dnl First, check the underlay
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 | FORMAT_PING], 
[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+dnl Okay, now send out an arp request from 10.1.1.1 for 10.1.1.100 in erspan.
+ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 
packet=f2ff0002f2ff00030800454e151d4000402fcac0ac1f0101ac1f0164100088be000610010007f2ff000408060001080006040001f2ff00040a0101010a010164
 actions=normal"
+
+sleep 1
+dnl 0002 is arp reply, followed by mac address of 10.1.1.100.
+AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "f2ff  0001 0806" 2>&1 
1>/dev/null])
+AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "0800 0604 0002 f2ff  0001 
0a01" 2>&1 1>/dev/null])
+
+dnl Okay, now check the overlay with raw icmp packets.
+AT_FAIL_IF([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP 172.31.1.100 > 172.31.1.1: 
GREv0, .* length 1258" 2>&1 1>/dev/null])
+
+ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 
packet=f2ff0002f2ff00030800458e70cb4000402f6ed2ac1f0101ac1f0164100088be000510010007f2ff0001f2ff00040800455c4a3340004001da070a0101010a010164080084f238fb0001f36a6b5b21870e00101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f
 actions=normal"
+
+sleep 1
+AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP 172.31.1.100 > 172.31.1.1: 
GREv0, .* length 122" 2>&1 1>/dev/null])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([datapath - clone action])
 OVS_TRAFFIC_VSWITCHD_START()
 
-- 
2.7.4

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

[ovs-dev] [PATCH 4/8] system-traffic.at: Add gre tunnel test that doesn't depend on upstream gre module

[Yifeng Sun]

Introduce a new test that doesn't setup native gre tunnels but sends
simulated raw packets.
This test is supposed to only run for kernel version from 4.4.x to 4.15.x.

Signed-off-by: Yifeng Sun 
---
 tests/system-traffic.at | 47 +++
 1 file changed, 47 insertions(+)

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index cf53c10..dca2bc8 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -575,6 +575,53 @@ NS_CHECK_EXEC([at_ns0], [ping -s 3200 -q -c 3 -i 0.3 -w 2 
10.1.1.100 | FORMAT_PI
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([datapath - ping over gre tunnel by simulated packets])
+OVS_CHECK_KERNEL(4, 4, 15)
+
+OVS_TRAFFIC_VSWITCHD_START()
+
+AT_CHECK([ovs-vsctl -- set bridge br0 
other-config:hwaddr=\"f2:ff:00:00:00:01\"])
+ADD_BR([br-underlay], [set bridge br-underlay 
other-config:hwaddr=\"f2:ff:00:00:00:02\"])
+
+AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
+AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
+
+ADD_NAMESPACES(at_ns0)
+
+dnl Set up underlay link from host into the namespace using veth pair.
+ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24", f2:ff:00:00:00:03)
+AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
+AT_CHECK([ip link set dev br-underlay up])
+
+dnl Set up tunnel endpoints on OVS outside the namespace.
+ADD_OVS_TUNNEL([gre], [br0], [at_gre0], [172.31.1.1], [10.1.1.100/24])
+
+ip netns exec at_ns0 tcpdump -U -i p0 -w p0.pcap &
+sleep 1
+
+dnl First, check the underlay.
+NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 | FORMAT_PING], 
[0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+dnl We don't actually add gretap port as below, instead, we will
+dnl emulate one that sends out packets. Suppose its mac address is 
f2:ff:00:00:00:04.
+dnl ADD_NATIVE_TUNNEL([gretap], [ns_gre0], [at_ns0], [172.31.1.100], 
[10.1.1.1/24])
+
+dnl Now, check the overlay by sending out raw arp and icmp packets.
+ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 
packet=f2ff0002f2ff000308004542ec2c4000402ff3bcac1f0101ac1f01646558f2ff000408060001080006040001f2ff00040a0101010a010164
 actions=NORMAL"
+
+sleep 1
+AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP 172.31.1.100 > 172.31.1.1: 
GREv0, length 46: ARP, Reply 10.1.1.100 is-at f2:ff:00:00:00:01 .* length 28" 
2>&1 1>/dev/null])
+
+ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 
packet=f2ff0002f2ff00030800457aec8e4000402ff322ac1f0101ac1f01646558f2ff0001f2ff000408004554548f40004001cfb30a0101010a0101640800e6e829270003e1a3435bff1a0500101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637
 actions=NORMAL"
+
+sleep 1
+AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP 172.31.1.100 > 172.31.1.1: 
GREv0, length 102: IP 10.1.1.100 > 10.1.1.1: ICMP echo reply, .* length 64$" 
2>&1 1>/dev/null])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([datapath - clone action])
 OVS_TRAFFIC_VSWITCHD_START()
 
-- 
2.7.4

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

[ovs-dev] [PATCH 2/8] system-common-macros.at: Skip tests for certain kernel versions

[Yifeng Sun]

Some tests depend on upstream gre modules to setup testing environments.
However, some kernel versions require compatable gre modules being used.
This patch helps to skip tests that fail due to this reason. The new m4
functions will be used by later patches.

Signed-off-by: Yifeng Sun 
---
 tests/system-common-macros.at | 20 
 1 file changed, 20 insertions(+)

diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index 64bf5ec..0bb4f8c 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -329,3 +329,23 @@ m4_define([OVS_CHECK_IPROUTE_ENCAP],
 # OVS_CHECK_CT_CLEAR()
 m4_define([OVS_CHECK_CT_CLEAR],
 [AT_SKIP_IF([! grep -q "Datapath supports ct_clear action" 
ovs-vswitchd.log])])
+
+# OVS_CHECK_KERNEL([version], [minsublevel], [maxsublevel])
+#
+# Check if kernel version falls between version.minsublevel and
+# version.maxsublevel, skip this test if it is not.
+m4_define([OVS_CHECK_KERNEL],
+[version=$(uname -r | sed -e 's/\./ /g' | awk '{print $ 1}')
+ sublevel=$(uname -r | sed -e 's/\./ /g' | awk '{print $ 2}')
+ AT_SKIP_IF([test $version -ne $1 || test $sublevel -lt $2 || test 
$sublevel -gt $3])
+])
+
+# OVS_CHECK_KERNEL_EXCL([version], [minsublevel], [maxsublevel])
+#
+# Check that kernel version doesn't fall between version.minsublevel and
+# version.maxsublevel, skip this test if it is.
+m4_define([OVS_CHECK_KERNEL_EXCL],
+[version=$(uname -r | sed -e 's/\./ /g' | awk '{print $ 1}')
+ sublevel=$(uname -r | sed -e 's/\./ /g' | awk '{print $ 2}')
+ AT_SKIP_IF([test $version -eq $1 && test $sublevel -ge $2 && test 
$sublevel -le $3])
+])
-- 
2.7.4

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

[ovs-dev] [PATCH 3/8] system-traffic.at: Skip gre-related failed tests on kernel version from 4.4 to 4.15

[Yifeng Sun]

Skip gre, erspan and ip6erspan related tests on kernel version from
4.4.x to 4.15.x because these tests will always fail.

Signed-off-by: Yifeng Sun 
---
 tests/system-traffic.at | 5 +
 1 file changed, 5 insertions(+)

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index cbd9542..cf53c10 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -300,6 +300,7 @@ OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over gre tunnel])
+OVS_CHECK_KERNEL_EXCL(4, 4, 15)
 OVS_CHECK_GRE()
 
 OVS_TRAFFIC_VSWITCHD_START()
@@ -340,6 +341,7 @@ OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over erspan v1 tunnel])
+OVS_CHECK_KERNEL_EXCL(4, 4, 15)
 OVS_CHECK_GRE()
 OVS_CHECK_ERSPAN()
 
@@ -375,6 +377,7 @@ OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over erspan v2 tunnel])
+OVS_CHECK_KERNEL_EXCL(4, 4, 15)
 OVS_CHECK_GRE()
 OVS_CHECK_ERSPAN()
 
@@ -410,6 +413,7 @@ OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over ip6erspan v1 tunnel])
+OVS_CHECK_KERNEL_EXCL(4, 4, 15)
 OVS_CHECK_GRE()
 OVS_CHECK_ERSPAN()
 
@@ -448,6 +452,7 @@ OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over ip6erspan v2 tunnel])
+OVS_CHECK_KERNEL_EXCL(4, 4, 15)
 OVS_CHECK_GRE()
 OVS_CHECK_ERSPAN()
 
-- 
2.7.4

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

[ovs-dev] [PATCH 1/8] ip6_gre: Fix a bug that clears address bits

[Yifeng Sun]

In compatable gre module, skb->cb is used as ovs_gso_cb.
This bug clears the 16-23 bit in the address of ovs_gso_cb.tun_dst.

Signed-off-by: Yifeng Sun 
---
 datapath/linux/compat/ip6_gre.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/datapath/linux/compat/ip6_gre.c b/datapath/linux/compat/ip6_gre.c
index 54a76ab..16c1f72 100644
--- a/datapath/linux/compat/ip6_gre.c
+++ b/datapath/linux/compat/ip6_gre.c
@@ -1146,7 +1146,6 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff 
*skb,
goto tx_err;
 
t->parms.o_flags &= ~TUNNEL_KEY;
-   IPCB(skb)->flags = 0;
 
tun_info = ovs_skb_tunnel_info(skb);
if (unlikely(!tun_info ||
-- 
2.7.4

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

Re: [ovs-dev] OVS DPDK: dpdk_merge pull request for branch-2.10

[Ben Pfaff]

On Wed, Aug 08, 2018 at 11:30:33PM +0100, Ian Stokes wrote:
> Hi Ben,
> 
> The following changes since commit faf64fb8861f312aca86a1d2b8fcb30d0504b09b:
> 
>   table: fix html buffer output (2018-08-08 11:19:15 -0700)
> 
> are available in the git repository at:
> 
>   https://github.com/istokes/ovs dpdk_merge_2_10
> 
> for you to fetch changes up to 553d3f1549385a009f703b86aa76d9896fa8da27:
> 
>   ofp-port: Fix buffer overread parsing Intel custom statistics. (2018-08-08
> 22:06:46 +0100)

Thanks.  I merged all the backport branches.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] OVS DPDK: dpdk_merge pull request for master

[Ben Pfaff]

On Wed, Aug 08, 2018 at 11:30:02PM +0100, Ian Stokes wrote:
> Hi Ben,
> 
> The following changes since commit c3cc694b93dd523176d2131a4b1b3b3170644638:
> 
>   table: fix html buffer output (2018-08-08 11:18:07 -0700)
> 
> are available in the git repository at:
> 
>   https://github.com/istokes/ovs dpdk_merge
> 
> for you to fetch changes up to b2d9a9ef5aae1b899e8458c5324e85ff70f27c7e:
> 
>   ofp-port: Drop of useless indirection in ofputil_pull_ofp14_port_stats().
> (2018-08-08 22:06:21 +0100)

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

Re: [ovs-dev] [PATCH v3 0/9] tests: Clean up syslog.

[Ben Pfaff]

On Wed, Aug 08, 2018 at 03:35:31PM +0300, Ilya Maximets wrote:
> Each run of the testsuite produces millions lines in a system
> log. This is completely unnecessary and makes it difficult to
> use system logs on test / build servers.
> 
> This series is aimed to disable most of the syslog messages.
> There are still few logs that requires significant changes in
> tests or code to disable. They will be removed separately if
> needed.

What do you think of the following approach?
https://patchwork.ozlabs.org/patch/955273/
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] tests: Don't log to syslog during tests.

[Ben Pfaff]

Until now, "make check" generated a huge amount of output to syslog.  This
commit suppresses it.

CC: Ilya Maximets 
Signed-off-by: Ben Pfaff 
---
 NEWS  |  2 ++
 lib/automake.mk   |  2 ++
 lib/syslog-null.c | 60 +++
 lib/syslog-null.h | 22 
 lib/vlog.c| 12 +--
 lib/vlog.man  |  7 ++-
 lib/vlog.xml  | 11 +-
 tests/atlocal.in  |  4 
 8 files changed, 116 insertions(+), 4 deletions(-)
 create mode 100644 lib/syslog-null.c
 create mode 100644 lib/syslog-null.h

diff --git a/NEWS b/NEWS
index 7875f6673e34..ae21340e9046 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,7 @@
 Post-v2.10.0
 -
+   - The environment variable OVS_SYSLOG_METHOD, if set, is now used
+ as the default syslog method.
 
 
 v2.10.0 - xx xxx 
diff --git a/lib/automake.mk b/lib/automake.mk
index fb43aa1413b2..63e9d72ac18a 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -280,6 +280,8 @@ lib_libopenvswitch_la_SOURCES = \
lib/syslog-direct.h \
lib/syslog-libc.c \
lib/syslog-libc.h \
+   lib/syslog-null.c \
+   lib/syslog-null.h \
lib/syslog-provider.h \
lib/table.c \
lib/table.h \
diff --git a/lib/syslog-null.c b/lib/syslog-null.c
new file mode 100644
index ..9dbd13911c21
--- /dev/null
+++ b/lib/syslog-null.c
@@ -0,0 +1,60 @@
+/*
+ * Copyright (c) 2015, 2016 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include "syslog-null.h"
+
+#include 
+
+#include "compiler.h"
+#include "syslog-provider.h"
+#include "util.h"
+
+static void syslog_null_open(struct syslogger *this, int facility);
+static void syslog_null_log(struct syslogger *this, int pri, const char *msg);
+
+static struct syslog_class syslog_null_class = {
+syslog_null_open,
+syslog_null_log,
+};
+
+struct syslog_null {
+struct syslogger parent;
+};
+
+/* This function  creates object that delegate all logging to null's
+ * syslog implementation. */
+struct syslogger *
+syslog_null_create(void)
+{
+struct syslog_null *this = xmalloc(sizeof *this);
+
+this->parent.class = _null_class;
+this->parent.prefix = "";
+
+return >parent;
+}
+
+static void
+syslog_null_open(struct syslogger *this OVS_UNUSED, int facility OVS_UNUSED)
+{
+/* Nothing to do. */
+}
+
+static void
+syslog_null_log(struct syslogger *this OVS_UNUSED, int pri OVS_UNUSED,
+const char *msg OVS_UNUSED)
+{
+/* Nothing to do. */
+}
diff --git a/lib/syslog-null.h b/lib/syslog-null.h
new file mode 100644
index ..0f7731dc4dcc
--- /dev/null
+++ b/lib/syslog-null.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2015 Nicira, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at:
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef SYSLOG_NULL_H
+#define SYSLOG_NULL_H 1
+
+struct syslogger *syslog_null_create(void);
+
+#endif /* syslog-null.h */
diff --git a/lib/vlog.c b/lib/vlog.c
index bf5fd88ba9e8..01cfdc5d3d2d 100644
--- a/lib/vlog.c
+++ b/lib/vlog.c
@@ -39,6 +39,7 @@
 #include "svec.h"
 #include "syslog-direct.h"
 #include "syslog-libc.h"
+#include "syslog-null.h"
 #include "syslog-provider.h"
 #include "timeval.h"
 #include "unixctl.h"
@@ -584,7 +585,9 @@ vlog_set_syslog_method(const char *method)
 return;
 }
 
-if (!strcmp(method, "libc")) {
+if (!strcmp(method, "null")) {
+syslogger = syslog_null_create();
+} else if (!strcmp(method, "libc")) {
 syslogger = syslog_libc_create();
 } else if (!strncmp(method, "udp:", 4) || !strncmp(method, "unix:", 5)) {
 syslogger = syslog_direct_create(method);
@@ -778,7 +781,12 @@ vlog_init(void)
  * log anything before calling ovsthread_once_done() will deadlock. */
 atomic_read_explicit(_facility, , memory_order_relaxed);
 if (!syslogger) {
-syslogger = syslog_libc_create();
+char *env = getenv("OVS_SYSLOG_METHOD");
+ 

[ovs-dev] OVS DPDK: dpdk_merge pull request for branch-2.7

[Ian Stokes]

Hi Ben,

The following changes since commit e1619a1e7a8dded3c7da9ab40bd62a3628478711:

  ofctl: Fixup compare_flows function (2018-08-08 21:11:14 +0300)

are available in the git repository at:

  https://github.com/istokes/ovs dpdk_merge_2_7

for you to fetch changes up to a90ff16cad22afd4cea6dfd9f0b93f93988842cb:

  netdev-dpdk: Use hex for PCI vendor ID. (2018-08-08 22:12:58 +0100)


Kevin Traynor (1):
  netdev-dpdk: Use hex for PCI vendor ID.

 lib/netdev-dpdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

[ovs-dev] OVS DPDK: dpdk_merge pull request for branch-2.6

[Ian Stokes]

Hi Ben,

The following changes since commit a9e21a1b9134090a3a12b1760daffba7f2777472:

  pcap-file: Fix formatting of log message. (2018-08-03 16:55:51 -0700)

are available in the git repository at:

  https://github.com/istokes/ovs dpdk_merge_2_6

for you to fetch changes up to 53e8175a09777c8c9d6995f8b60d1085fafc85a0:

  netdev-dpdk: Use hex for PCI vendor ID. (2018-08-08 22:13:40 +0100)


Kevin Traynor (1):
  netdev-dpdk: Use hex for PCI vendor ID.

 lib/netdev-dpdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

[ovs-dev] OVS DPDK: dpdk_merge pull request for branch-2.8

[Ian Stokes]

Hi Ben,

The following changes since commit 59c0ba2990b7149ba42bb34b76cbe27b9cbed0e9:

  ofctl: Fixup compare_flows function (2018-08-08 21:09:55 +0300)

are available in the git repository at:

  https://github.com/istokes/ovs dpdk_merge_2_8

for you to fetch changes up to 05ae838b2057fd2c55cdca87228f34741b046b79:

  netdev-dpdk: Use hex for PCI vendor ID. (2018-08-08 22:12:19 +0100)


Kevin Traynor (1):
  netdev-dpdk: Use hex for PCI vendor ID.

 lib/netdev-dpdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

[ovs-dev] OVS DPDK: dpdk_merge pull request for branch-2.9

[Ian Stokes]

Hi Ben,

The following changes since commit c3a91ed557b7765c907ee5597ae8e68b1181869a:

  ofctl: Fixup compare_flows function (2018-08-08 21:03:17 +0300)

are available in the git repository at:

  https://github.com/istokes/ovs dpdk_merge_2_9

for you to fetch changes up to 661b583e599e80f377c5f2fd147f330ef30514c0:

  netdev-dpdk: Use hex for PCI vendor ID. (2018-08-08 22:11:19 +0100)


Ben Pfaff (1):
  ofp-port: Fix buffer overread parsing Intel custom statistics.

Kevin Traynor (1):
  netdev-dpdk: Use hex for PCI vendor ID.

Sugesh Chandran (1):
  netdev-dpdk: Fix failure to configure flow control at netdev-init.

 lib/netdev-dpdk.c | 17 -
 lib/ofp-util.c| 50 --
 2 files changed, 28 insertions(+), 39 deletions(-)

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

[ovs-dev] OVS DPDK: dpdk_merge pull request for branch-2.10

[Ian Stokes]

Hi Ben,

The following changes since commit faf64fb8861f312aca86a1d2b8fcb30d0504b09b:

  table: fix html buffer output (2018-08-08 11:19:15 -0700)

are available in the git repository at:

  https://github.com/istokes/ovs dpdk_merge_2_10

for you to fetch changes up to 553d3f1549385a009f703b86aa76d9896fa8da27:

  ofp-port: Fix buffer overread parsing Intel custom statistics. 
(2018-08-08 22:06:46 +0100)



Ben Pfaff (1):
  ofp-port: Fix buffer overread parsing Intel custom statistics.

Ilya Maximets (1):
  dpif-netdev: Fix zero length keys insertion to EMC.

Kevin Traynor (1):
  netdev-dpdk: Use hex for PCI vendor ID.

Sugesh Chandran (1):
  netdev-dpdk: Fix failure to configure flow control at netdev-init.

 lib/dpif-netdev.c |  2 +-
 lib/netdev-dpdk.c | 16 +++-
 lib/ofp-port.c| 50 --
 3 files changed, 28 insertions(+), 40 deletions(-)

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

[ovs-dev] OVS DPDK: dpdk_merge pull request for master

[Ian Stokes]

Hi Ben,

The following changes since commit c3cc694b93dd523176d2131a4b1b3b3170644638:

  table: fix html buffer output (2018-08-08 11:18:07 -0700)

are available in the git repository at:

  https://github.com/istokes/ovs dpdk_merge

for you to fetch changes up to b2d9a9ef5aae1b899e8458c5324e85ff70f27c7e:

  ofp-port: Drop of useless indirection in 
ofputil_pull_ofp14_port_stats(). (2018-08-08 22:06:21 +0100)



Ben Pfaff (2):
  ofp-port: Fix buffer overread parsing Intel custom statistics.
  ofp-port: Drop of useless indirection in 
ofputil_pull_ofp14_port_stats().


Ilya Maximets (1):
  dpif-netdev: Fix zero length keys insertion to EMC.

Kevin Traynor (1):
  netdev-dpdk: Use hex for PCI vendor ID.

Sugesh Chandran (1):
  netdev-dpdk: Fix failure to configure flow control at netdev-init.

 lib/dpif-netdev.c |  2 +-
 lib/netdev-dpdk.c | 16 +++-
 lib/ofp-port.c| 78 
+-

 3 files changed, 29 insertions(+), 67 deletions(-)

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

Re: [ovs-dev] [PATCH] unixctl: Style fix.

[Ben Pfaff]

On Wed, Aug 08, 2018 at 05:19:40PM -0400, Aaron Conole wrote:
> Ben Pfaff  writes:
> 
> > Reported-by: Aaron Conole 
> > Signed-off-by: Ben Pfaff 
> > ---
> 
> Acked-by: Aaron Conole 

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

Re: [ovs-dev] s/rhel/rpm/?

[Ben Pfaff]

On Thu, Aug 09, 2018 at 12:29:20AM +0300, Markos Chandras wrote:
> On 08/08/2018 09:01 PM, Ben Pfaff wrote:
> > [asking some random SuSE and Red Hat people]
> > 
> > It had somehow slipped past my notice before that the spec files we have
> > are useful for SuSE as well as Red Hat.  Should we make the directory or
> > file names more generic?
> > 
> > Thanks,
> > 
> > Ben.
> > 
> Hello Ben,
> 
> The SUSE spec file[1] mostly matches the rhel/openvswitch-fedora.spec.in
> one from the OvS tree, but because of the different packaging policies
> between the two distributions, we need to adapt it a little bit. Our
> spec file is also adapted to build on RHEL and SUSE (note all the %if
> 0%{?suse_version} blocks there).
> 
> The rhel/ directory currently has quite a few spec files and most of
> them only make sense for RHEL. I can perhaps commit the spec file from
> [1] as openvswitch-suse.spec.in and then we can rename the directory to
> 'rpm' since it would then hold spec files for multiple distros. Would
> that work?

OK, I misunderstood.  I had the mistaken idea from one of your messages
that SuSE was using the same specs file as Fedora, verbatim.

Personally, I'd rather move all the distro packaging out of OVS, because
distro packagers are good at packaging and OVS developers generally
aren't.  So, unless it would actually make your job easier, let's keep
things as is.

Thanks,

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

Re: [ovs-dev] s/rhel/rpm/?

[Markos Chandras]

On 08/08/2018 09:01 PM, Ben Pfaff wrote:
> [asking some random SuSE and Red Hat people]
> 
> It had somehow slipped past my notice before that the spec files we have
> are useful for SuSE as well as Red Hat.  Should we make the directory or
> file names more generic?
> 
> Thanks,
> 
> Ben.
> 
Hello Ben,

The SUSE spec file[1] mostly matches the rhel/openvswitch-fedora.spec.in
one from the OvS tree, but because of the different packaging policies
between the two distributions, we need to adapt it a little bit. Our
spec file is also adapted to build on RHEL and SUSE (note all the %if
0%{?suse_version} blocks there).

The rhel/ directory currently has quite a few spec files and most of
them only make sense for RHEL. I can perhaps commit the spec file from
[1] as openvswitch-suse.spec.in and then we can rename the directory to
'rpm' since it would then hold spec files for multiple distros. Would
that work?

[1]
https://build.opensuse.org/package/view_file/network/openvswitch/openvswitch.spec?expand=1

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] unixctl: Style fix.

[Aaron Conole]

Ben Pfaff  writes:

> Reported-by: Aaron Conole 
> Signed-off-by: Ben Pfaff 
> ---

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

Re: [ovs-dev] [PATCH] unixctl: Style fix.

[Justin Pettit]

> On Aug 8, 2018, at 1:31 PM, Ben Pfaff  wrote:
> 
> Reported-by: Aaron Conole 
> Signed-off-by: Ben Pfaff 

Acked-by: Justin Pettit 

--Justin


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

Re: [ovs-dev] [patch v7] dpctl: Make opt_dpif_open() more general.

[Darrell Ball]

Ben

If you apply this, I have one line to delete inline.

Darrell

On Wed, Aug 8, 2018 at 2:09 PM, Darrell Ball  wrote:

> By making opt_dpif_open() more general, it can be used effectively
> by all potential callers and avoids trying to open potentially bogus
> datapaths provided by the user. Also, the error handling is improved by
> reducing bogus errors and having more specific real errors.
>
> Signed-off-by: Darrell Ball 
> ---
>  lib/dpctl.c | 56 ++
> ++-
>  tests/system-traffic.at | 10 -
>  2 files changed, 56 insertions(+), 10 deletions(-)
>
> diff --git a/lib/dpctl.c b/lib/dpctl.c
> index c600eeb..ef057aa 100644
> --- a/lib/dpctl.c
> +++ b/lib/dpctl.c
> @@ -187,18 +187,64 @@ parsed_dpif_open(const char *arg_, bool create,
> struct dpif **dpifp)
>  return result;
>  }
>
> +static bool
> +dp_exists(const char *queried_dp)
> +{
> +struct sset dpif_names = SSET_INITIALIZER(_names),
> +dpif_types = SSET_INITIALIZER(_types);
> +bool found = false;
> +char *queried_name, *queried_type;
> +
> +dp_parse_name(queried_dp, _name, _type);
> +dp_enumerate_types(_types);
> +
> +if (!sset_contains(_types, queried_type)) {
> +goto out;
> +}
> +
> +int error = dp_enumerate_names(queried_type, _names);
> +if (!error && sset_contains(_names, queried_name)) {
> +found = true;
> +goto out;
>

Pls remove the above "goto out;"



> +}
> +
> +out:
> +sset_destroy(_names);
> +sset_destroy(_types);
> +return found;
> +}
> +
> +static bool
> +dp_arg_exists(int argc, const char *argv[])
> +{
> +if (argc > 1 && dp_exists(argv[1])) {
> +return true;
> +}
> +return false;
> +}
> +
>  /* Open a dpif with an optional name argument.
>   *
> - * The datapath name is not a mandatory parameter for this command.  If
> - * it is not specified -- so 'argc' < 'max_args' -- we retrieve it from
> - * the current setup, assuming only one exists.  On success stores the
> - * opened dpif in '*dpifp'. */
> + * The datapath name is not a mandatory parameter for this command.  If
> it is
> + * not specified, we retrieve it from the current setup, assuming only one
> + * exists.  On success stores the opened dpif in '*dpifp'. */
>  static int
>  opt_dpif_open(int argc, const char *argv[], struct dpctl_params *dpctl_p,
>uint8_t max_args, struct dpif **dpifp)
>  {
> +char *dpname;
> +if (dp_arg_exists(argc, argv)) {
> +dpname = xstrdup(argv[1]);
> +} else if (argc != max_args) {
> +dpname = get_one_dp(dpctl_p);
> +} else {
> +/* If the arguments are the maximum possible number and there is
> no
> + * valid datapath argument, then we fall into the case of dpname
> is
> + * NULL, since this is an error. */
> +dpname = NULL;
> +}
> +
>  int error = 0;
> -char *dpname = argc >= max_args ? xstrdup(argv[1]) :
> get_one_dp(dpctl_p);
>  if (!dpname) {
>  error = EINVAL;
>  dpctl_error(dpctl_p, error, "datapath not found");
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index cbd9542..a7c8a24 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -1040,7 +1040,7 @@ AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=5
> $ICMP_TUPLE])
>  AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep
> "orig=.src=10\.1\.1\.2,"], [1], [dnl
>  ])
>
> -OVS_TRAFFIC_VSWITCHD_STOP(["/could not create datapath/d"])
> +OVS_TRAFFIC_VSWITCHD_STOP
>  AT_CLEANUP
>
>  AT_SETUP([conntrack - IPv4 ping])
> @@ -1124,17 +1124,17 @@ ovs-appctl: ovs-vswitchd: server returned an error
>  ])
>
>  AT_CHECK([ovs-appctl dpctl/ct-set-maxconns one-bad-dp 10], [2], [], [dnl
> -ovs-vswitchd: opening datapath (Address family not supported by protocol)
> +ovs-vswitchd: datapath not found (Invalid argument)
>  ovs-appctl: ovs-vswitchd: server returned an error
>  ])
>
>  AT_CHECK([ovs-appctl dpctl/ct-get-maxconns one-bad-dp], [2], [], [dnl
> -ovs-vswitchd: opening datapath (Address family not supported by protocol)
> +ovs-vswitchd: datapath not found (Invalid argument)
>  ovs-appctl: ovs-vswitchd: server returned an error
>  ])
>
>  AT_CHECK([ovs-appctl dpctl/ct-get-nconns one-bad-dp], [2], [], [dnl
> -ovs-vswitchd: opening datapath (Address family not supported by protocol)
> +ovs-vswitchd: datapath not found (Invalid argument)
>  ovs-appctl: ovs-vswitchd: server returned an error
>  ])
>
> @@ -1164,7 +1164,7 @@ AT_CHECK([ovs-appctl dpctl/ct-get-maxconns], [], [dnl
>  10
>  ])
>
> -OVS_TRAFFIC_VSWITCHD_STOP(["/could not create datapath one-bad-dp of
> unknown type system/d"])
> +OVS_TRAFFIC_VSWITCHD_STOP
>  AT_CLEANUP
>
>  AT_SETUP([conntrack - IPv6 ping])
> --
> 1.9.1
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [patch v7] dpctl: Make opt_dpif_open() more general.

[Darrell Ball]

By making opt_dpif_open() more general, it can be used effectively
by all potential callers and avoids trying to open potentially bogus
datapaths provided by the user. Also, the error handling is improved by
reducing bogus errors and having more specific real errors.

Signed-off-by: Darrell Ball 
---
 lib/dpctl.c | 56 -
 tests/system-traffic.at | 10 -
 2 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/lib/dpctl.c b/lib/dpctl.c
index c600eeb..ef057aa 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -187,18 +187,64 @@ parsed_dpif_open(const char *arg_, bool create, struct 
dpif **dpifp)
 return result;
 }
 
+static bool
+dp_exists(const char *queried_dp)
+{
+struct sset dpif_names = SSET_INITIALIZER(_names),
+dpif_types = SSET_INITIALIZER(_types);
+bool found = false;
+char *queried_name, *queried_type;
+
+dp_parse_name(queried_dp, _name, _type);
+dp_enumerate_types(_types);
+
+if (!sset_contains(_types, queried_type)) {
+goto out;
+}
+
+int error = dp_enumerate_names(queried_type, _names);
+if (!error && sset_contains(_names, queried_name)) {
+found = true;
+goto out;
+}
+
+out:
+sset_destroy(_names);
+sset_destroy(_types);
+return found;
+}
+
+static bool
+dp_arg_exists(int argc, const char *argv[])
+{
+if (argc > 1 && dp_exists(argv[1])) {
+return true;
+}
+return false;
+}
+
 /* Open a dpif with an optional name argument.
  *
- * The datapath name is not a mandatory parameter for this command.  If
- * it is not specified -- so 'argc' < 'max_args' -- we retrieve it from
- * the current setup, assuming only one exists.  On success stores the
- * opened dpif in '*dpifp'. */
+ * The datapath name is not a mandatory parameter for this command.  If it is
+ * not specified, we retrieve it from the current setup, assuming only one
+ * exists.  On success stores the opened dpif in '*dpifp'. */
 static int
 opt_dpif_open(int argc, const char *argv[], struct dpctl_params *dpctl_p,
   uint8_t max_args, struct dpif **dpifp)
 {
+char *dpname;
+if (dp_arg_exists(argc, argv)) {
+dpname = xstrdup(argv[1]);
+} else if (argc != max_args) {
+dpname = get_one_dp(dpctl_p);
+} else {
+/* If the arguments are the maximum possible number and there is no
+ * valid datapath argument, then we fall into the case of dpname is
+ * NULL, since this is an error. */
+dpname = NULL;
+}
+
 int error = 0;
-char *dpname = argc >= max_args ? xstrdup(argv[1]) : get_one_dp(dpctl_p);
 if (!dpname) {
 error = EINVAL;
 dpctl_error(dpctl_p, error, "datapath not found");
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index cbd9542..a7c8a24 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -1040,7 +1040,7 @@ AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=5 
$ICMP_TUPLE])
 AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.2,"], 
[1], [dnl
 ])
 
-OVS_TRAFFIC_VSWITCHD_STOP(["/could not create datapath/d"])
+OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([conntrack - IPv4 ping])
@@ -1124,17 +1124,17 @@ ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
 AT_CHECK([ovs-appctl dpctl/ct-set-maxconns one-bad-dp 10], [2], [], [dnl
-ovs-vswitchd: opening datapath (Address family not supported by protocol)
+ovs-vswitchd: datapath not found (Invalid argument)
 ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
 AT_CHECK([ovs-appctl dpctl/ct-get-maxconns one-bad-dp], [2], [], [dnl
-ovs-vswitchd: opening datapath (Address family not supported by protocol)
+ovs-vswitchd: datapath not found (Invalid argument)
 ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
 AT_CHECK([ovs-appctl dpctl/ct-get-nconns one-bad-dp], [2], [], [dnl
-ovs-vswitchd: opening datapath (Address family not supported by protocol)
+ovs-vswitchd: datapath not found (Invalid argument)
 ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
@@ -1164,7 +1164,7 @@ AT_CHECK([ovs-appctl dpctl/ct-get-maxconns], [], [dnl
 10
 ])
 
-OVS_TRAFFIC_VSWITCHD_STOP(["/could not create datapath one-bad-dp of unknown 
type system/d"])
+OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([conntrack - IPv6 ping])
-- 
1.9.1

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

Re: [ovs-dev] [patch v6] dpctl: Make opt_dpif_open() more general.

[Darrell Ball]

hold off on this version

I can save a couple lines of code, so I'll send a V7

Darrell


On Wed, Aug 8, 2018 at 1:05 PM, Darrell Ball  wrote:

> By making opt_dpif_open() more general, it can be used effectively
> by all potential callers and avoids trying to open potentially bogus
> datapaths provided by the user. Also, the error handling is improved by
> having more specific errors.
>
> Signed-off-by: Darrell Ball 
> ---
>  lib/dpctl.c | 61 ++
> +++
>  tests/system-traffic.at | 10 
>  2 files changed, 61 insertions(+), 10 deletions(-)
>
> diff --git a/lib/dpctl.c b/lib/dpctl.c
> index c600eeb..6f50d89 100644
> --- a/lib/dpctl.c
> +++ b/lib/dpctl.c
> @@ -187,18 +187,69 @@ parsed_dpif_open(const char *arg_, bool create,
> struct dpif **dpifp)
>  return result;
>  }
>
> +static bool
> +dp_exists(const char *queried_dp)
> +{
> +struct sset dpif_names = SSET_INITIALIZER(_names),
> +dpif_types = SSET_INITIALIZER(_types);
> +bool found = false;
> +char *queried_name, *queried_type;
> +
> +dp_parse_name(queried_dp, _name, _type);
> +dp_enumerate_types(_types);
> +
> +if (!sset_contains(_types, queried_type)) {
> +goto out;
> +}
> +
> +int error = dp_enumerate_names(queried_type, _names);
> +if (!error) {
> +const char *name;
> +SSET_FOR_EACH (name, _names) {
> +if (!strcmp(name, queried_name)) {
> +found = true;
> +goto out;
> +}
> +}
> +}
> +
> +out:
> +sset_destroy(_names);
> +sset_destroy(_types);
> +return found;
> +}
> +
> +static bool
> +dp_arg_exists(int argc, const char *argv[])
> +{
> +if (argc > 1 && dp_exists(argv[1])) {
> +return true;
> +}
> +return false;
> +}
> +
>  /* Open a dpif with an optional name argument.
>   *
> - * The datapath name is not a mandatory parameter for this command.  If
> - * it is not specified -- so 'argc' < 'max_args' -- we retrieve it from
> - * the current setup, assuming only one exists.  On success stores the
> - * opened dpif in '*dpifp'. */
> + * The datapath name is not a mandatory parameter for this command.  If
> it is
> + * not specified, we retrieve it from the current setup, assuming only one
> + * exists.  On success stores the opened dpif in '*dpifp'. */
>  static int
>  opt_dpif_open(int argc, const char *argv[], struct dpctl_params *dpctl_p,
>uint8_t max_args, struct dpif **dpifp)
>  {
> +char *dpname;
> +if (dp_arg_exists(argc, argv)) {
> +dpname = xstrdup(argv[1]);
> +} else if (argc != max_args) {
> +dpname = get_one_dp(dpctl_p);
> +} else {
> +/* If the arguments are the maximum possible number and there is
> no
> + * valid datapath argument, then we fall into the case of dpname
> is
> + * NULL, since this is an error. */
> +dpname = NULL;
> +}
> +
>  int error = 0;
> -char *dpname = argc >= max_args ? xstrdup(argv[1]) :
> get_one_dp(dpctl_p);
>  if (!dpname) {
>  error = EINVAL;
>  dpctl_error(dpctl_p, error, "datapath not found");
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index cbd9542..a7c8a24 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -1040,7 +1040,7 @@ AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=5
> $ICMP_TUPLE])
>  AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep
> "orig=.src=10\.1\.1\.2,"], [1], [dnl
>  ])
>
> -OVS_TRAFFIC_VSWITCHD_STOP(["/could not create datapath/d"])
> +OVS_TRAFFIC_VSWITCHD_STOP
>  AT_CLEANUP
>
>  AT_SETUP([conntrack - IPv4 ping])
> @@ -1124,17 +1124,17 @@ ovs-appctl: ovs-vswitchd: server returned an error
>  ])
>
>  AT_CHECK([ovs-appctl dpctl/ct-set-maxconns one-bad-dp 10], [2], [], [dnl
> -ovs-vswitchd: opening datapath (Address family not supported by protocol)
> +ovs-vswitchd: datapath not found (Invalid argument)
>  ovs-appctl: ovs-vswitchd: server returned an error
>  ])
>
>  AT_CHECK([ovs-appctl dpctl/ct-get-maxconns one-bad-dp], [2], [], [dnl
> -ovs-vswitchd: opening datapath (Address family not supported by protocol)
> +ovs-vswitchd: datapath not found (Invalid argument)
>  ovs-appctl: ovs-vswitchd: server returned an error
>  ])
>
>  AT_CHECK([ovs-appctl dpctl/ct-get-nconns one-bad-dp], [2], [], [dnl
> -ovs-vswitchd: opening datapath (Address family not supported by protocol)
> +ovs-vswitchd: datapath not found (Invalid argument)
>  ovs-appctl: ovs-vswitchd: server returned an error
>  ])
>
> @@ -1164,7 +1164,7 @@ AT_CHECK([ovs-appctl dpctl/ct-get-maxconns], [], [dnl
>  10
>  ])
>
> -OVS_TRAFFIC_VSWITCHD_STOP(["/could not create datapath one-bad-dp of
> unknown type system/d"])
> +OVS_TRAFFIC_VSWITCHD_STOP
>  AT_CLEANUP
>
>  AT_SETUP([conntrack - IPv6 ping])
> --
> 1.9.1
>
>
___
dev mailing list
d...@openvswitch.org

Re: [ovs-dev] [PATCH v2 1/3] unixctl: Make path to unixctl_server socket available to the client.

[Ben Pfaff]

On Wed, Aug 08, 2018 at 04:13:39PM -0400, Aaron Conole wrote:
> Ben Pfaff  writes:
> > @@ -429,10 +426,17 @@ unixctl_server_destroy(struct unixctl_server *server)
> >  kill_connection(conn);
> >  }
> >  
> > +free (server->path);
> 
> Just a small nit, this looks like gnu style.

Oops.  I've written a lot of GNU code, sometimes it comes naturally.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] unixctl: Style fix.

[Ben Pfaff]

Reported-by: Aaron Conole 
Signed-off-by: Ben Pfaff 
---
 lib/unixctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/unixctl.c b/lib/unixctl.c
index 9b3b0671f33c..730bd043a9ff 100644
--- a/lib/unixctl.c
+++ b/lib/unixctl.c
@@ -426,7 +426,7 @@ unixctl_server_destroy(struct unixctl_server *server)
 kill_connection(conn);
 }
 
-free (server->path);
+free(server->path);
 pstream_close(server->listener);
 free(server);
 }
-- 
2.16.1

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

Re: [ovs-dev] [PATCH 1/2] ofp-port: Fix buffer overread parsing Intel custom statistics.

[Ian Stokes]

On 8/8/2018 7:11 PM, Ben Pfaff wrote:

On Wed, Aug 08, 2018 at 09:24:30AM +0100, Ian Stokes wrote:

On 8/7/2018 1:00 AM, Ben Pfaff wrote:

On Fri, Jul 27, 2018 at 11:14:43AM -0700, Ben Pfaff wrote:

CC: Michal Weglicki 
Fixes: 971f4b394c6e ("netdev: Custom statistics.")
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9445
Signed-off-by: Ben Pfaff 


Still needs a review.

Thanks,

Ben.



Thanks for the patch Ben, unfortunately Michal is no longer active on the ML
and I only just spotted this series, feel free to  cc myself for Intel
custom stats work in the future.

Patch looks good to me overall, tested fine also. I can include this in this
weeks pull request.


Thanks.  I expect that as a bug fix this should be backported as far as
needed.

Yes, I've backported this to 2.10 and 2.9. I've kept the second patch in 
the series separate as recommended so that will only be applied to master.


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

Re: [ovs-dev] [PATCH v2 1/3] unixctl: Make path to unixctl_server socket available to the client.

[Aaron Conole]

Ben Pfaff  writes:

> Acked-by: Alin Gabriel Serdean 
> Signed-off-by: Ben Pfaff 
> ---
>  lib/unixctl.c   | 52 
>  lib/unixctl.h   |  2 ++
>  tests/daemon.at |  4 ++--
>  3 files changed, 32 insertions(+), 26 deletions(-)
>
> diff --git a/lib/unixctl.c b/lib/unixctl.c
> index bd9c1caeedef..9b3b0671f33c 100644
> --- a/lib/unixctl.c
> +++ b/lib/unixctl.c
> @@ -56,6 +56,7 @@ struct unixctl_conn {
>  struct unixctl_server {
>  struct pstream *listener;
>  struct ovs_list conns;
> +char *path;
>  };
>  
>  static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
> @@ -216,48 +217,44 @@ unixctl_command_reply_error(struct unixctl_conn *conn, 
> const char *error)
>  int
>  unixctl_server_create(const char *path, struct unixctl_server **serverp)
>  {
> -struct unixctl_server *server;
> -struct pstream *listener;
> -char *punix_path;
> -int error;
> -
>  *serverp = NULL;
>  if (path && !strcmp(path, "none")) {
>  return 0;
>  }
>  
> -if (path) {
> -char *abs_path;
> -abs_path = abs_file_name(ovs_rundir(), path);
> -punix_path = xasprintf("punix:%s", abs_path);
> -free(abs_path);
> -} else {
> -#ifndef _WIN32
> -punix_path = xasprintf("punix:%s/%s.%ld.ctl", ovs_rundir(),
> -   program_name, (long int) getpid());
> +#ifdef _WIN32
> +enum { WINDOWS = 1 };
>  #else
> -punix_path = xasprintf("punix:%s/%s.ctl", ovs_rundir(), 
> program_name);
> +enum { WINDOWS = 0 };
>  #endif
> -}
>  
> -error = pstream_open(punix_path, , 0);
> +long int pid = getpid();
> +char *abs_path
> += (path ? abs_file_name(ovs_rundir(), path)
> +   : WINDOWS ? xasprintf("%s/%s.ctl", ovs_rundir(), program_name)
> +   : xasprintf("%s/%s.%ld.ctl", ovs_rundir(), program_name, pid));
> +
> +struct pstream *listener;
> +char *punix_path = xasprintf("punix:%s", abs_path);
> +int error = pstream_open(punix_path, , 0);
> +free(punix_path);
> +
>  if (error) {
> -ovs_error(error, "could not initialize control socket %s", 
> punix_path);
> -goto exit;
> +ovs_error(error, "%s: could not initialize control socket", 
> abs_path);
> +free(abs_path);
> +return error;
>  }
>  
>  unixctl_command_register("list-commands", "", 0, 0, 
> unixctl_list_commands,
>   NULL);
>  unixctl_command_register("version", "", 0, 0, unixctl_version, NULL);
>  
> -server = xmalloc(sizeof *server);
> +struct unixctl_server *server = xmalloc(sizeof *server);
>  server->listener = listener;
> +server->path = abs_path;
>  ovs_list_init(>conns);
>  *serverp = server;
> -
> -exit:
> -free(punix_path);
> -return error;
> +return 0;
>  }
>  
>  static void
> @@ -429,10 +426,17 @@ unixctl_server_destroy(struct unixctl_server *server)
>  kill_connection(conn);
>  }
>  
> +free (server->path);

Just a small nit, this looks like gnu style.

>  pstream_close(server->listener);
>  free(server);
>  }
>  }
> +
> +const char *
> +unixctl_server_get_path(const struct unixctl_server *server)
> +{
> +return server ? server->path : NULL;
> +}
>  
>  /* On POSIX based systems, connects to a unixctl server socket.  'path' 
> should
>   * be the name of a unixctl server socket.  If it does not start with '/', it
> diff --git a/lib/unixctl.h b/lib/unixctl.h
> index ce43893c6a7d..4562dbc49113 100644
> --- a/lib/unixctl.h
> +++ b/lib/unixctl.h
> @@ -28,6 +28,8 @@ void unixctl_server_run(struct unixctl_server *);
>  void unixctl_server_wait(struct unixctl_server *);
>  void unixctl_server_destroy(struct unixctl_server *);
>  
> +const char *unixctl_server_get_path(const struct unixctl_server *);
> +
>  /* Client for Unix domain socket control connection. */
>  struct jsonrpc;
>  int unixctl_client_create(const char *path, struct jsonrpc **client);
> diff --git a/tests/daemon.at b/tests/daemon.at
> index 952d5a7c7bbe..b379fa83f9aa 100644
> --- a/tests/daemon.at
> +++ b/tests/daemon.at
> @@ -149,7 +149,7 @@ AT_SETUP([daemon --detach startup errors])
>  AT_CAPTURE_FILE([pid])
>  OVSDB_INIT([db])
>  AT_CHECK([ovsdb-server --detach --no-chdir --pidfile 
> --unixctl=nonexistent/unixctl db], [1], [], [stderr])
> -AT_CHECK([grep 'ovsdb-server: could not initialize control socket' stderr],
> +AT_CHECK([grep 'could not initialize control socket' stderr],
>[0], [ignore])
>  AT_CHECK([test ! -s pid])
>  AT_CLEANUP
> @@ -159,7 +159,7 @@ AT_SKIP_IF([test "$IS_WIN32" = "yes"])
>  AT_CAPTURE_FILE([pid])
>  OVSDB_INIT([db])
>  AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --monitor 
> --unixctl=nonexistent/unixctl db], [1], [], [stderr])
> -AT_CHECK([grep 'ovsdb-server: could not initialize control socket' stderr],
> +AT_CHECK([grep 'could not initialize control socket' stderr],
>[0], 

Re: [ovs-dev] [patch v5] dpctl: Make opt_dpif_open() more general.

[Darrell Ball]

On 8/8/18, 11:14 AM, "ovs-dev-boun...@openvswitch.org on behalf of Ben Pfaff" 
 wrote:

On Tue, Aug 07, 2018 at 09:49:38PM -0700, Darrell Ball wrote:
> On Tue, Aug 7, 2018 at 4:20 PM, Ben Pfaff  wrote:
> 
> > On Mon, Aug 06, 2018 at 07:24:39PM -0700, Darrell Ball wrote:
> > > By making opt_dpif_open() more general, it can be used effectively
> > > by all potential callers and avoids trying to open potentially bogus
> > > datapaths provided by the user. Also, the error handling is improved 
by
> > > having more specific errors.
> > >
> > > Signed-off-by: Darrell Ball 
> > > ---
> > >  lib/dpctl.c | 61 ++
> > +++
> > >  tests/system-traffic.at |  8 +++
> > >  2 files changed, 60 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/lib/dpctl.c b/lib/dpctl.c
> > > index c600eeb..b8a8dbf 100644
> > > --- a/lib/dpctl.c
> > > +++ b/lib/dpctl.c
> > > @@ -187,18 +187,69 @@ parsed_dpif_open(const char *arg_, bool create,
> > struct dpif **dpifp)
> > >  return result;
> > >  }
> > >
> > > +static bool
> > > +dp_exists(const char *queried_dp)
> > > +{
> > > +struct sset dpif_names = SSET_INITIALIZER(_names),
> > > +dpif_types = SSET_INITIALIZER(_types);
> > > +bool found = false;
> > > +char *queried_name, *queried_type;
> > > +
> > > +dp_parse_name(queried_dp, _name, _type);
> > > +dp_enumerate_types(_types);
> > > +
> > > +const char *type;
> > > +SSET_FOR_EACH (type, _types) {
> > > +int error = dp_enumerate_names(type, _names);
> > > +if (!error) {
> > > +const char *name;
> > > +SSET_FOR_EACH (name, _names) {
> > > +if (!strcmp(type, queried_type) &&
> > > +!strcmp(name, queried_name)) {
> > > +found = true;
> > > +goto out;
> > > +}
> > > +}
> > > +}
> > > +}
> > > +
> > > +out:
> > > +sset_destroy(_names);
> > > +sset_destroy(_types);
> > > +return found;
> > > +}
> >
> > Thanks for working to make ovs-dpctl error messages better!
> >
> > I don't see why one would bother to enumerate the types here.  There is
> > only one type that could be of interest.  I think you said that you
> > don't want to just try to blindly open (type,name), but even if so, why
> > not just try to enumerate the names within 'type'?
> >
> 
> 
> Because the type (i.e. queried_type) passed is often/usually bogus and
> therefore
> enumerating names with this bogus 'type' only serves to generate the
> appropriate warning logs,
> yielding a false positive.

OK, in that case, then use sset_contains() to check for type in
dpif_types; there's no reason to enumerate all the names in every type,
only in the one we care about.  Right?

I was thinking that the number of datapaths is typically ‘1’, so, somehow, I 
had little
practical concern about enumerating for this use case.

But sure, that is generally better; I made the adjustment.

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org

https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs-devdata=02%7C01%7Cdball%40vmware.com%7Cbd37aa3529b7424419c608d5fd5ab7d9%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636693488467972500sdata=8c1NbVZOj%2BTBhqFmLBctytMzM7u7MmD42GRY%2Furz0Ww%3Dreserved=0










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

[ovs-dev] [patch v6] dpctl: Make opt_dpif_open() more general.

[Darrell Ball]

By making opt_dpif_open() more general, it can be used effectively
by all potential callers and avoids trying to open potentially bogus
datapaths provided by the user. Also, the error handling is improved by
having more specific errors.

Signed-off-by: Darrell Ball 
---
 lib/dpctl.c | 61 +
 tests/system-traffic.at | 10 
 2 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/lib/dpctl.c b/lib/dpctl.c
index c600eeb..6f50d89 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -187,18 +187,69 @@ parsed_dpif_open(const char *arg_, bool create, struct 
dpif **dpifp)
 return result;
 }
 
+static bool
+dp_exists(const char *queried_dp)
+{
+struct sset dpif_names = SSET_INITIALIZER(_names),
+dpif_types = SSET_INITIALIZER(_types);
+bool found = false;
+char *queried_name, *queried_type;
+
+dp_parse_name(queried_dp, _name, _type);
+dp_enumerate_types(_types);
+
+if (!sset_contains(_types, queried_type)) {
+goto out;
+}
+
+int error = dp_enumerate_names(queried_type, _names);
+if (!error) {
+const char *name;
+SSET_FOR_EACH (name, _names) {
+if (!strcmp(name, queried_name)) {
+found = true;
+goto out;
+}
+}
+}
+
+out:
+sset_destroy(_names);
+sset_destroy(_types);
+return found;
+}
+
+static bool
+dp_arg_exists(int argc, const char *argv[])
+{
+if (argc > 1 && dp_exists(argv[1])) {
+return true;
+}
+return false;
+}
+
 /* Open a dpif with an optional name argument.
  *
- * The datapath name is not a mandatory parameter for this command.  If
- * it is not specified -- so 'argc' < 'max_args' -- we retrieve it from
- * the current setup, assuming only one exists.  On success stores the
- * opened dpif in '*dpifp'. */
+ * The datapath name is not a mandatory parameter for this command.  If it is
+ * not specified, we retrieve it from the current setup, assuming only one
+ * exists.  On success stores the opened dpif in '*dpifp'. */
 static int
 opt_dpif_open(int argc, const char *argv[], struct dpctl_params *dpctl_p,
   uint8_t max_args, struct dpif **dpifp)
 {
+char *dpname;
+if (dp_arg_exists(argc, argv)) {
+dpname = xstrdup(argv[1]);
+} else if (argc != max_args) {
+dpname = get_one_dp(dpctl_p);
+} else {
+/* If the arguments are the maximum possible number and there is no
+ * valid datapath argument, then we fall into the case of dpname is
+ * NULL, since this is an error. */
+dpname = NULL;
+}
+
 int error = 0;
-char *dpname = argc >= max_args ? xstrdup(argv[1]) : get_one_dp(dpctl_p);
 if (!dpname) {
 error = EINVAL;
 dpctl_error(dpctl_p, error, "datapath not found");
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index cbd9542..a7c8a24 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -1040,7 +1040,7 @@ AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=5 
$ICMP_TUPLE])
 AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.2,"], 
[1], [dnl
 ])
 
-OVS_TRAFFIC_VSWITCHD_STOP(["/could not create datapath/d"])
+OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([conntrack - IPv4 ping])
@@ -1124,17 +1124,17 @@ ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
 AT_CHECK([ovs-appctl dpctl/ct-set-maxconns one-bad-dp 10], [2], [], [dnl
-ovs-vswitchd: opening datapath (Address family not supported by protocol)
+ovs-vswitchd: datapath not found (Invalid argument)
 ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
 AT_CHECK([ovs-appctl dpctl/ct-get-maxconns one-bad-dp], [2], [], [dnl
-ovs-vswitchd: opening datapath (Address family not supported by protocol)
+ovs-vswitchd: datapath not found (Invalid argument)
 ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
 AT_CHECK([ovs-appctl dpctl/ct-get-nconns one-bad-dp], [2], [], [dnl
-ovs-vswitchd: opening datapath (Address family not supported by protocol)
+ovs-vswitchd: datapath not found (Invalid argument)
 ovs-appctl: ovs-vswitchd: server returned an error
 ])
 
@@ -1164,7 +1164,7 @@ AT_CHECK([ovs-appctl dpctl/ct-get-maxconns], [], [dnl
 10
 ])
 
-OVS_TRAFFIC_VSWITCHD_STOP(["/could not create datapath one-bad-dp of unknown 
type system/d"])
+OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([conntrack - IPv6 ping])
-- 
1.9.1

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

Re: [ovs-dev] [PATCH] ofctl: Fixup compare_flows function

[Alin Gabriel Serdean]

On 7 Aug 2018, at 19:36, Ben Pfaff  wrote:
> 
> Signed-off-by: Ben Pfaff 
> 
> On Tue, Aug 07, 2018 at 03:59:59PM +0300, aserd...@ovn.org wrote:
>> Can you also provide a Signed-off-by pls?
>> 
>>> -Mesaj original-
>>> De la: ovs-dev-boun...@openvswitch.org >> boun...@openvswitch.org> În numele Ben Pfaff
>>> Trimis: Tuesday, August 7, 2018 12:40 AM
>>> Către: Alin Gabriel Serdean 
>>> Cc: d...@openvswitch.org
>>> Subiect: Re: [ovs-dev] [PATCH] ofctl: Fixup compare_flows function
>>> 
>>> On Tue, Aug 07, 2018 at 12:34:45AM +0300, Alin Gabriel Serdean wrote:
 In the case there was no sorting criteria the flows on Windows were
 being rearranged because it was always returning zero.
 
 Also check if there we need sorting to save a few cycles.
 
 CC: Ben Pfaff 
 Co-authored-by: Ben Pfaff 
 Signed-off-by: Alin Gabriel Serdean 
>>> 
>>> Acked-by: Ben Pfaff 
>>> ___
>>> dev mailing list
>>> d...@openvswitch.org
>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>> 

Thanks! I applied this from master to branch-2.7
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 0/9] tests: Clean up syslog.

[Ben Pfaff]

On Wed, Aug 08, 2018 at 03:07:04PM +0300, Ilya Maximets wrote:
> On 07.08.2018 00:12, Ben Pfaff wrote:
> > On Mon, Aug 06, 2018 at 06:18:19PM +0300, Ilya Maximets wrote:
> >> On 04.08.2018 03:17, Ben Pfaff wrote:
> >>> On Wed, Aug 01, 2018 at 05:00:09PM +0300, Ilya Maximets wrote:
>  Each run of the testsuite produces millions lines in a system
>  log. This is completely unnecessary and makes it difficult to
>  use system logs on test / build servers.
> 
>  This series is aimed to disable most of the syslog messages.
>  There are still few logs that requires significant changes in
>  tests or code to disable. They will be removed separately if
>  needed.
> >>>
> >>> This series seems technically high-quality.  Thank you.
> >>>
> >>> I find myself wondering whether we should just introduce an environment
> >>> variable to allowing configuring logging defaults.  Then we would cover
> >>> literally everything with a single line in atlocal.in.
> >>>
> >>> Any opinions?
> >>
> >> I thought about this. And implementation will have few issues:
> >>
> >> 1. Many daemons already has logging related options in cmdlines.
> >>This means that it should not be "OVS_DEFAULT_LOGGING_OPTIONS",
> >>but "OVS_FORCE_LOGGING_OPTIONS". i.e. the value in the environment
> >>should have highest priority.
> >> 2. Leads from the 1st. I'm not sure that we should expose variables
> >>like this for the end users, because there will be no way to change
> >>the logging level for the targets forced by the environment.
> >>So, it should be test only variable like
> >>"TEST_OVS_FORCE_DEFAULT_OPTIONS". But we still can't be sure that
> >>no one will use it in production.
> >> 3. Some tests (like some from vlog.at) requires ability to change the
> >>log level for syslog, for example. We'll need to unset the variable
> >>for these tests or change the tests themselves.
> >> 4. Modifications for all the utils/daemons and/or the vlog subsystem
> >>are required to implement this environment variable which is
> >>intended for test purposes only
> >>
> >> I'm not sure which solution is better.
> >> What do you think?
> > 
> > Most of these points seem to follow from #1, but I don't understand #1.
> > Why shouldn't the environment variable be just a default that can be
> > overridden by command-line arguments?  That is usually what environment
> > settings do.
> 
> Yes, you're right that having environment variable which could be overridden
> by command-line arguments is a common and usual thing. But tests contains a
> lot of daemons'/control utils' invocations like this:
> ovs-vswitchd --enable-dummy=system --disable-system --detach \
>  --no-chdir --pidfile --log-file \
>  -vvconn -vofproto_dpif
> or
> ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg
> 
> In above cases environment variable will be overridden and we will have
> debug logs in syslog. So, we'll have to apply at least a half of current
> patch set to disable syslog in these cases. See patches 4, 5, 6 and 8.

Mmm, you're right, thanks.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 2/2] table: fix html buffer output

[Ben Pfaff]

On Wed, Aug 08, 2018 at 09:59:40AM -0400, Aaron Conole wrote:
> Aaron Conole  writes:
> 
> > Prior to this commit, html output exhibiits a doppler effect for
> 
> Ironically, I duplicated an 'i' in the word exhibits here.  Let me know
> if you want a respin or if it can be fixed while applying. :-x

I fixed that up and applied this to master and branch-2.10.  Thank you!
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v2 1/2] table: append newline when printing tables

[Ben Pfaff]

On Wed, Aug 08, 2018 at 10:15:35AM -0500, Terry Wilson wrote:
> On Tue, Aug 7, 2018 at 7:34 PM, Aaron Conole  wrote:
> > With commit cb139fa8b3a1 ("table: New function table_format() for
> > formatting a table as a string.") a new mechanism for formatting
> > tables was introduced, and the table_print method was refactored to
> > use this.
> >
> > During that refactor, calls to 'puts' were replaced with
> > 'ds_put_cstr', and table print was changed to use 'fputs(...,
> > stdout)'.  Unfortunately, fputs() does not append a newline to the
> > string provided, and changes the output strings of, for example,
> > ovsdb-client dump to print all on one line.  This means
> > post-processing scripts that are chained after ovsdb-client would
> > either block indefinitely (if they don't detect EOF), or process the
> > entire bundle at once (rather than seeing each table on a separate
> > line).
> >
> > Fixes: cb139fa8b3a1 ("table: New function table_format() for formatting a 
> > table as a string.")
> > Cc: Ben Pfaff 
> > Cc: Jakub Sitnicki 
> > Reported-by: Terry Wilson 
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1608508
> > Signed-off-by: Aaron Conole 
> > Suggested-by: Ben Pfaff 
> > ---
> >  lib/table.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/table.c b/lib/table.c
> > index cd811caf5..19bf89262 100644
> > --- a/lib/table.c
> > +++ b/lib/table.c
> > @@ -547,6 +547,7 @@ table_print_json__(const struct table *table, const 
> > struct table_style *style,
> >  json_object_put(json, "data", data);
> >
> >  json_to_ds(json, style->json_flags, s);
> > +ds_put_char(s, '\n');
> >  json_destroy(json);
> >  }
> >
> > --
> > 2.14.3
> >
> 
> I can verify that this re-adds the newline and that it fixes the issue
> in neutron's ovs-agent.
> 
> Acked-by: Terry Wilson 
> Tested-by: Terry Wilson 

This now has the world's longest collection of tags for a one-line
change:

Fixes: cb139fa8b3a1 ("table: New function table_format() for formatting a table 
as a string.")
Cc: Ben Pfaff 
Cc: Jakub Sitnicki 
Reported-by: Terry Wilson 
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1608508
Signed-off-by: Aaron Conole 
Suggested-by: Ben Pfaff 
Signed-off-by: Ben Pfaff 
Acked-by: Terry Wilson 
Tested-by: Terry Wilson 

I applied this to master and branch-2.10.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [patch v5] dpctl: Make opt_dpif_open() more general.

[Ben Pfaff]

On Tue, Aug 07, 2018 at 09:49:38PM -0700, Darrell Ball wrote:
> On Tue, Aug 7, 2018 at 4:20 PM, Ben Pfaff  wrote:
> 
> > On Mon, Aug 06, 2018 at 07:24:39PM -0700, Darrell Ball wrote:
> > > By making opt_dpif_open() more general, it can be used effectively
> > > by all potential callers and avoids trying to open potentially bogus
> > > datapaths provided by the user. Also, the error handling is improved by
> > > having more specific errors.
> > >
> > > Signed-off-by: Darrell Ball 
> > > ---
> > >  lib/dpctl.c | 61 ++
> > +++
> > >  tests/system-traffic.at |  8 +++
> > >  2 files changed, 60 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/lib/dpctl.c b/lib/dpctl.c
> > > index c600eeb..b8a8dbf 100644
> > > --- a/lib/dpctl.c
> > > +++ b/lib/dpctl.c
> > > @@ -187,18 +187,69 @@ parsed_dpif_open(const char *arg_, bool create,
> > struct dpif **dpifp)
> > >  return result;
> > >  }
> > >
> > > +static bool
> > > +dp_exists(const char *queried_dp)
> > > +{
> > > +struct sset dpif_names = SSET_INITIALIZER(_names),
> > > +dpif_types = SSET_INITIALIZER(_types);
> > > +bool found = false;
> > > +char *queried_name, *queried_type;
> > > +
> > > +dp_parse_name(queried_dp, _name, _type);
> > > +dp_enumerate_types(_types);
> > > +
> > > +const char *type;
> > > +SSET_FOR_EACH (type, _types) {
> > > +int error = dp_enumerate_names(type, _names);
> > > +if (!error) {
> > > +const char *name;
> > > +SSET_FOR_EACH (name, _names) {
> > > +if (!strcmp(type, queried_type) &&
> > > +!strcmp(name, queried_name)) {
> > > +found = true;
> > > +goto out;
> > > +}
> > > +}
> > > +}
> > > +}
> > > +
> > > +out:
> > > +sset_destroy(_names);
> > > +sset_destroy(_types);
> > > +return found;
> > > +}
> >
> > Thanks for working to make ovs-dpctl error messages better!
> >
> > I don't see why one would bother to enumerate the types here.  There is
> > only one type that could be of interest.  I think you said that you
> > don't want to just try to blindly open (type,name), but even if so, why
> > not just try to enumerate the names within 'type'?
> >
> 
> 
> Because the type (i.e. queried_type) passed is often/usually bogus and
> therefore
> enumerating names with this bogus 'type' only serves to generate the
> appropriate warning logs,
> yielding a false positive.

OK, in that case, then use sset_contains() to check for type in
dpif_types; there's no reason to enumerate all the names in every type,
only in the one we care about.  Right?

Thanks,

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

Re: [ovs-dev] [PATCH 2/2] ofp-port: Drop of useless indirection in ofputil_pull_ofp14_port_stats().

[Ben Pfaff]

On Wed, Aug 08, 2018 at 09:26:11AM +0100, Ian Stokes wrote:
> On 7/27/2018 7:14 PM, Ben Pfaff wrote:
> >Signed-off-by: Ben Pfaff 
> 
> Seems straight forward enough. LGTM. I can add this to this weeks pull
> request.

Thanks.  This is just refactoring, so I would expect it to go to master
only.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] s/rhel/rpm/?

[Ben Pfaff]

[asking some random SuSE and Red Hat people]

It had somehow slipped past my notice before that the spec files we have
are useful for SuSE as well as Red Hat.  Should we make the directory or
file names more generic?

Thanks,

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

Re: [ovs-dev] [PATCH v3] rhel: Use correct user in the logrotate configuration file

[Ben Pfaff]

On Wed, Aug 08, 2018 at 05:52:55PM +0200, Timothy Redaelli wrote:
> On Wed,  8 Aug 2018 17:27:25 +0300
> Markos Chandras  wrote:
> 
> > The /var/log/openvswitch directory is owned by the openvswitch user
> > but logrotate could be running as root or as another user. As a
> > result of which, rpmlint prints the following warning when building
> > the spec file on SUSE Linux Enterprise:
> > 
> > openvswitch.x86_64: W:
> > suse-logrotate-user-writable-log-dir /var/log/openvswitch
> > openvswitch:openvswitch 0750 The log directory is writable by
> > unprivileged users. Please fix the permissions so only root can write
> > there or add the 'su' option to your logrotate config
> > 
> > In order to fix that, we should run the logrotate script as the same
> > user which runs the various Open vSwitch daemons. If this is a new
> > installation, then this user is the 'openvswitch' one, but if we are
> > upgrading from an older release, then the user is normally 'root'.
> > As such, we set the initial user to 'root' and we fix this up in the
> > %post scriptlet.
> > 
> > Cc: Aaron Conole 
> > Cc: Timothy Redaelli 
> > Signed-off-by: Markos Chandras 
> 
> Acked-by: Timothy Redaelli 

Thanks Markos and Timothy.  I applied this to master and branch-2.10.
If should be backported further, please let me know.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] infiniband (IPoIB) support

[Ben Pfaff]

On Wed, Aug 08, 2018 at 09:16:11AM -0700, Gregory Rose wrote:
> On 8/7/2018 3:18 PM, Ben Pfaff wrote:
> >I don't know how many people on this list know anything about IPoIB.  I
> >know that I don't.  You might not be getting an answer simply because
> >it's such a specialty topic.  Maybe there is a place where people talk
> >about IPoIB software; maybe they would know something.
> 
> I was under the impression we only support Ethernet at the physical layer.

Yes.

In theory OpenFlow could support Infiniband, but OVS has never been
extended in that direction, or if someone has done it they have never
contributed it back.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v7 07/13] dp-packet: Handle multi-seg mubfs in shift() func.

[Ian Stokes]

On 8/8/2018 5:06 PM, Darrell Ball wrote:



On Wed, Aug 8, 2018 at 3:17 AM, Ian Stokes > wrote:


On 8/7/2018 6:13 PM, Darrell Ball wrote:



On Tue, Aug 7, 2018 at 5:08 AM, Stokes, Ian
mailto:ian.sto...@intel.com>
>> wrote:

     > In its current implementation dp_packet_shift() is also
unaware of multi-
     > seg mbufs (that holds data in memory non-contiguously)
and assumes that
     > data exists contiguously in memory, memmove'ing data to
perform the shift.
     >     > To add support for multi-seg mbuds a new set of
functions was introduced,
     > dp_packet_mbuf_shift() and dp_packet_mbuf_write(). These
functions are
     > used by dp_packet_shift(), when handling multi-seg mbufs,
to shift and
     > write data within a chain of mbufs.
     >
     Hi Tiago,

     After applying this patch I see the following error during
compilation.

     lib/conntrack.c: In function 'handle_ftp_ctl':
     lib/conntrack.c:3154:11: error:
'addr_offset_from_ftp_data_start'
     may be used uninitialized in this function
[-Werror=maybe-uninitialized]
           char *pkt_addr_str = ftp_data_start +
     addr_offset_from_ftp_data_start;
                 ^~~~
     lib/conntrack.c:3173:12: note:
'addr_offset_from_ftp_data_start' was
     declared here
           size_t addr_offset_from_ftp_data_start;

     It can be fixed with the following incremental.

     diff --git a/lib/conntrack.c b/lib/conntrack.c
     index 974f985..7cd1fc9 100644
     --- a/lib/conntrack.c
     +++ b/lib/conntrack.c
     @@ -3170,7 +3170,7 @@ handle_ftp_ctl(struct conntrack *ct,
const
     struct conn_lookup_ctx *ctx,
           struct ip_header *l3_hdr = dp_packet_l3(pkt);
           ovs_be32 v4_addr_rep = 0;
           struct ct_addr v6_addr_rep;
     -    size_t addr_offset_from_ftp_data_start;
     +    size_t addr_offset_from_ftp_data_start = 0;
           size_t addr_size = 0;
           char *ftp_data_start;
           bool do_seq_skew_adj = true;

     If there are no objections I can roll this into the patch upon
     application to dpdk merge.

     Ian



hmm, I test with 4 major versions of GCC (4-7) with different
flags, including O3 and I don't see these errors.
I use 6.4 for the '6' major version

What flags are you using ?


I've compiled with and without the following flags -g -Ofast
-march=native.


I use -g and -march=native.
But not -Ofast, since it uses non-standard optimizations and did not 
find it benefits much.


Good point, I've seen some issues with it in the past, it's more of an 
artifact from the OVS docs at this stage.




I doubt any base gcc version would not be able to understand branching,
If anything, a non-standard optimization level would be the only 
impactful difference.



In both cases the warning still occurs.

I'm using gcc (GCC) 6.3.1. Tiago also spotted the same issue
occurring on GCC 5.4 so it doesn't seem isolated to 6.3.1.



I have a VM with 5.4.0 and also added 6.3.0 on another VM, but don't an 
issue with "-g -Ofast -march=native"
I did not find the sources for 6.3.1, at least easily and I doubt I will 
be installing 2 year old Fedora to get 6.3.1 anytime soon :-)


:), Yes an upgrade to a newer target is called for at this point, was 
hoping to get around to it after the 2.10 release.




If this is causing you grieve and you don't want to use a more recent 
gcc, initialization is ok for 'infrequent code paths' such as we

discuss here.


Thanks for the input Darrell, I'll roll in the change so to the patch to 
keep compilation for gcc similarly affected.


Thanks
Ian






I am building some versions from source; are you doing the same /

Is it possible that your GCC 6.3.1 was not built correctly ?


I'm not building gcc from source, I'm using Fedora 24 and the GCC
installed from the fedora repo.

Ian




     > Signed-off-by: Tiago Lam mailto:tiago@intel.com> >>
     > Acked-by: Eelco Chaudron mailto:echau...@redhat.com> >>

     > ---
     >  lib/dp-packet.c | 97
     > +
     >  lib/dp-packet.h | 10 ++
     >  2 files changed, 107 insertions(+)
     >     > diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index 2aaeaae..d6e19eb
     > 100644
   

Re: [ovs-dev] [PATCH] rhel: Install the network scripts in a new subpackage

[Flavio Leitner]

Actually, we also have dependencies in the systemd service
to the network service:

[Unit]  

Description=Open vSwitch Forwarding Unit
After=ovsdb-server.service network-pre.target systemd-udev-settle.service
Before=network.target network.service
  ^^^ 

Looks like that would be gone, right?

fbl

On Wed, Aug 08, 2018 at 01:38:07PM -0300, Flavio Leitner wrote:
> On Tue, Aug 07, 2018 at 06:52:10PM +0200, Timothy Redaelli wrote:
> > Starting from Fedora 29, the legacy network scripts are installed in
> > the "network-scripts" package and so the network scripts ("ifup-ovs",
> > "ifdown-ovs") should be installed only when the "network-scripts" package
> > is installed.
> > 
> > This commit introduces (on Fedora 29+) a new subpackage
> > (network-scripts-openvswitch). This subpackage is installed, by default, 
> > only
> > if the "network-scripts" package is installed too (reverse weak dependency).
> 
> The package's name is following a new format that other projects are
> using too.
> 
> The dnf installs weak dependencies unless told otherwise.
> 
> It requires the network-scripts if installed.
> 
> Conditionals look okay.
> 
> Looks correct if I read the guidelines correctly:
> https://fedoraproject.org/wiki/Packaging:Guidelines 
> 
> It build the rpms correctly over here.
> 
> Acked-by: Flavio Leitner 
> 
> Thanks Timothy and Lubomir.
> fbl
> 
> > Reported-by: Lubomir Rintel 
> > Reported-at: https://src.fedoraproject.org/rpms/openvswitch/pull-request/4
> > Signed-off-by: Timothy Redaelli 
> > ---
> >  rhel/openvswitch-fedora.spec.in | 20 
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/rhel/openvswitch-fedora.spec.in 
> > b/rhel/openvswitch-fedora.spec.in
> > index 9f8664e95..eead10069 100644
> > --- a/rhel/openvswitch-fedora.spec.in
> > +++ b/rhel/openvswitch-fedora.spec.in
> > @@ -162,6 +162,18 @@ Provides: openvswitch-static = %{version}-%{release}
> >  This provides static library, libopenswitch.a and the openvswitch header
> >  files needed to build an external application.
> >  
> > +%if 0%{?rhel} > 7 || 0%{?fedora} > 28
> > +%package -n network-scripts-%{name}
> > +Summary: Open vSwitch legacy network service support
> > +License: ASL 2.0
> > +Requires: network-scripts
> > +Supplements: (%{name} and network-scripts)
> > +
> > +%description -n network-scripts-%{name}
> > +This provides the ifup and ifdown scripts for use with the legacy network
> > +service.
> > +%endif
> > +
> >  %package ovn-central
> >  Summary: Open vSwitch - Open Virtual Network support
> >  License: ASL 2.0
> > @@ -529,6 +541,12 @@ fi
> >  %{_includedir}/openflow/*
> >  %{_includedir}/ovn/*
> >  
> > +%if 0%{?rhel} > 7 || 0%{?fedora} > 28
> > +%files -n network-scripts-%{name}
> > +%{_sysconfdir}/sysconfig/network-scripts/ifup-ovs
> > +%{_sysconfdir}/sysconfig/network-scripts/ifdown-ovs
> > +%endif
> > +
> >  %files
> >  %defattr(-,openvswitch,openvswitch)
> >  %dir %{_sysconfdir}/openvswitch
> > @@ -546,8 +564,10 @@ fi
> >  %{_unitdir}/ovs-vswitchd.service
> >  %{_unitdir}/ovs-delete-transient-ports.service
> >  %{_datadir}/openvswitch/scripts/openvswitch.init
> > +%if ! (0%{?rhel} > 7 || 0%{?fedora} > 28)
> >  %{_sysconfdir}/sysconfig/network-scripts/ifup-ovs
> >  %{_sysconfdir}/sysconfig/network-scripts/ifdown-ovs
> > +%endif
> >  %{_datadir}/openvswitch/bugtool-plugins/
> >  %{_datadir}/openvswitch/scripts/ovs-bugtool-*
> >  %{_datadir}/openvswitch/scripts/ovs-check-dead-ifs
> > -- 
> > 2.17.1
> > 
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
> -- 
> Flavio
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

-- 
Flavio

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

Re: [ovs-dev] [PATCH] rhel: Install the network scripts in a new subpackage

[Flavio Leitner]

On Tue, Aug 07, 2018 at 06:52:10PM +0200, Timothy Redaelli wrote:
> Starting from Fedora 29, the legacy network scripts are installed in
> the "network-scripts" package and so the network scripts ("ifup-ovs",
> "ifdown-ovs") should be installed only when the "network-scripts" package
> is installed.
> 
> This commit introduces (on Fedora 29+) a new subpackage
> (network-scripts-openvswitch). This subpackage is installed, by default, only
> if the "network-scripts" package is installed too (reverse weak dependency).

The package's name is following a new format that other projects are
using too.

The dnf installs weak dependencies unless told otherwise.

It requires the network-scripts if installed.

Conditionals look okay.

Looks correct if I read the guidelines correctly:
https://fedoraproject.org/wiki/Packaging:Guidelines 

It build the rpms correctly over here.

Acked-by: Flavio Leitner 

Thanks Timothy and Lubomir.
fbl

> Reported-by: Lubomir Rintel 
> Reported-at: https://src.fedoraproject.org/rpms/openvswitch/pull-request/4
> Signed-off-by: Timothy Redaelli 
> ---
>  rhel/openvswitch-fedora.spec.in | 20 
>  1 file changed, 20 insertions(+)
> 
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
> index 9f8664e95..eead10069 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -162,6 +162,18 @@ Provides: openvswitch-static = %{version}-%{release}
>  This provides static library, libopenswitch.a and the openvswitch header
>  files needed to build an external application.
>  
> +%if 0%{?rhel} > 7 || 0%{?fedora} > 28
> +%package -n network-scripts-%{name}
> +Summary: Open vSwitch legacy network service support
> +License: ASL 2.0
> +Requires: network-scripts
> +Supplements: (%{name} and network-scripts)
> +
> +%description -n network-scripts-%{name}
> +This provides the ifup and ifdown scripts for use with the legacy network
> +service.
> +%endif
> +
>  %package ovn-central
>  Summary: Open vSwitch - Open Virtual Network support
>  License: ASL 2.0
> @@ -529,6 +541,12 @@ fi
>  %{_includedir}/openflow/*
>  %{_includedir}/ovn/*
>  
> +%if 0%{?rhel} > 7 || 0%{?fedora} > 28
> +%files -n network-scripts-%{name}
> +%{_sysconfdir}/sysconfig/network-scripts/ifup-ovs
> +%{_sysconfdir}/sysconfig/network-scripts/ifdown-ovs
> +%endif
> +
>  %files
>  %defattr(-,openvswitch,openvswitch)
>  %dir %{_sysconfdir}/openvswitch
> @@ -546,8 +564,10 @@ fi
>  %{_unitdir}/ovs-vswitchd.service
>  %{_unitdir}/ovs-delete-transient-ports.service
>  %{_datadir}/openvswitch/scripts/openvswitch.init
> +%if ! (0%{?rhel} > 7 || 0%{?fedora} > 28)
>  %{_sysconfdir}/sysconfig/network-scripts/ifup-ovs
>  %{_sysconfdir}/sysconfig/network-scripts/ifdown-ovs
> +%endif
>  %{_datadir}/openvswitch/bugtool-plugins/
>  %{_datadir}/openvswitch/scripts/ovs-bugtool-*
>  %{_datadir}/openvswitch/scripts/ovs-check-dead-ifs
> -- 
> 2.17.1
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

-- 
Flavio

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

Re: [ovs-dev] infiniband (IPoIB) support

[Gregory Rose]

On 8/7/2018 3:18 PM, Ben Pfaff wrote:

I don't know how many people on this list know anything about IPoIB.  I
know that I don't.  You might not be getting an answer simply because
it's such a specialty topic.  Maybe there is a place where people talk
about IPoIB software; maybe they would know something.


I was under the impression we only support Ethernet at the physical layer.

- Greg



On Wed, Aug 08, 2018 at 12:59:36AM +0300, Vasiliy Tolstov wrote:

Does anybody can helps me and say, how to do connectivity from ovn
network to physical? if ovn network and phisical have the same subnet.
пн, 6 авг. 2018 г. в 23:35, Vasiliy Tolstov :

пн, 6 авг. 2018 г. в 17:34, Vasiliy Tolstov :

And if IPoIB device cant't be added to openvswitch bridge, how can i
connect virtual network with physical in such setup:

i have logical switch extnet with vm ports, each vm via ovn dhcp have
ip address from external network.
I want to route all traffic from this extnet via last ip address from
/24 subnet (254).
Does i need to create logical router for such thing?
Can you explain me how can i solve this?
пн, 6 авг. 2018 г. в 17:11, Vasiliy Tolstov :

Hi. I know about dpdk, but i have mellanox connectx-2 card with IPoIB in linux.
And i want to add it to openvswitch bridge.
I found topics (7 years ago) that says about no plans to add support
for IPoIB devices.
How can i add to ovs bridge IPoIB device?

--
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru



--
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru

I'm partially solve problem by adding to extnet gw port with type localnet.
so connection from vm to external world works fine, but from external
world to vm not:
ovs-vsctl show
f39cbc63-2dd2-486e-a2c8-1b7faee48535
 Bridge br-ext
 Port patch-gw-to-br-int
 Interface patch-gw-to-br-int
 type: patch
 options: {peer=patch-br-int-to-gw}
 Port br-ext
 Interface br-ext
 type: internal
 Bridge br-int
 fail_mode: secure
 Port patch-br-int-to-gw
 Interface patch-br-int-to-gw
 type: patch
 options: {peer=patch-gw-to-br-int}
 Port "vnet0"
 Interface "vnet0"
 Port br-int
 Interface br-int
 type: internal
 ovs_version: "2.8.1"

 port gw
 type: localnet
 addresses: ["unknown"]
 port d2b171b6-c501-4e8c-b29d-adc79e573d4c
 addresses: ["52:54:00:08:43:25 xx.xx.xx.xx"]
--
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru



--
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru
___
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


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

Re: [ovs-dev] [PATCH v7 07/13] dp-packet: Handle multi-seg mubfs in shift() func.

[Darrell Ball]

On Wed, Aug 8, 2018 at 3:17 AM, Ian Stokes  wrote:

> On 8/7/2018 6:13 PM, Darrell Ball wrote:
>
>
>>
>> On Tue, Aug 7, 2018 at 5:08 AM, Stokes, Ian > > wrote:
>>
>> > In its current implementation dp_packet_shift() is also unaware of
>> multi-
>> > seg mbufs (that holds data in memory non-contiguously) and assumes
>> that
>> > data exists contiguously in memory, memmove'ing data to perform the
>> shift.
>> > > To add support for multi-seg mbuds a new set of functions was
>> introduced,
>> > dp_packet_mbuf_shift() and dp_packet_mbuf_write(). These functions
>> are
>> > used by dp_packet_shift(), when handling multi-seg mbufs, to shift
>> and
>> > write data within a chain of mbufs.
>> >
>> Hi Tiago,
>>
>> After applying this patch I see the following error during
>> compilation.
>>
>> lib/conntrack.c: In function 'handle_ftp_ctl':
>> lib/conntrack.c:3154:11: error: 'addr_offset_from_ftp_data_start'
>> may be used uninitialized in this function
>> [-Werror=maybe-uninitialized]
>>   char *pkt_addr_str = ftp_data_start +
>> addr_offset_from_ftp_data_start;
>> ^~~~
>> lib/conntrack.c:3173:12: note: 'addr_offset_from_ftp_data_start' was
>> declared here
>>   size_t addr_offset_from_ftp_data_start;
>>
>> It can be fixed with the following incremental.
>>
>> diff --git a/lib/conntrack.c b/lib/conntrack.c
>> index 974f985..7cd1fc9 100644
>> --- a/lib/conntrack.c
>> +++ b/lib/conntrack.c
>> @@ -3170,7 +3170,7 @@ handle_ftp_ctl(struct conntrack *ct, const
>> struct conn_lookup_ctx *ctx,
>>   struct ip_header *l3_hdr = dp_packet_l3(pkt);
>>   ovs_be32 v4_addr_rep = 0;
>>   struct ct_addr v6_addr_rep;
>> -size_t addr_offset_from_ftp_data_start;
>> +size_t addr_offset_from_ftp_data_start = 0;
>>   size_t addr_size = 0;
>>   char *ftp_data_start;
>>   bool do_seq_skew_adj = true;
>>
>> If there are no objections I can roll this into the patch upon
>> application to dpdk merge.
>>
>> Ian
>>
>>
>>
>> hmm, I test with 4 major versions of GCC (4-7) with different flags,
>> including O3 and I don't see these errors.
>> I use 6.4 for the '6' major version
>>
>> What flags are you using ?
>>
>
> I've compiled with and without the following flags -g -Ofast -march=native.
>

I use -g and -march=native.
But not -Ofast, since it uses non-standard optimizations and did not find
it benefits much.

I doubt any base gcc version would not be able to understand branching,
If anything, a non-standard optimization level would be the only impactful
difference.


>
> In both cases the warning still occurs.
>
> I'm using gcc (GCC) 6.3.1. Tiago also spotted the same issue occurring on
> GCC 5.4 so it doesn't seem isolated to 6.3.1.



I have a VM with 5.4.0 and also added 6.3.0 on another VM, but don't an
issue with "-g -Ofast -march=native"
I did not find the sources for 6.3.1, at least easily and I doubt I will be
installing 2 year old Fedora to get 6.3.1 anytime soon :-)

If this is causing you grieve and you don't want to use a more recent gcc,
initialization is ok for 'infrequent code paths' such as we
discuss here.



>
>
>
>
>> I am building some versions from source; are you doing the same /
>>
>> Is it possible that your GCC 6.3.1 was not built correctly ?
>>
>
> I'm not building gcc from source, I'm using Fedora 24 and the GCC
> installed from the fedora repo.
>
> Ian
>
>>
>>
>>
>> > Signed-off-by: Tiago Lam > tiago@intel.com>>
>> > Acked-by: Eelco Chaudron > echau...@redhat.com>>
>>
>> > ---
>> >  lib/dp-packet.c | 97
>> > +
>> >  lib/dp-packet.h | 10 ++
>> >  2 files changed, 107 insertions(+)
>> > > diff --git a/lib/dp-packet.c b/lib/dp-packet.c index
>> 2aaeaae..d6e19eb
>> > 100644
>> > --- a/lib/dp-packet.c
>> > +++ b/lib/dp-packet.c
>> > @@ -294,6 +294,97 @@ dp_packet_prealloc_headroom(struct dp_packet
>> *b,
>> > size_t size)
>> >  }
>> >  }
>> > > +#ifdef DPDK_NETDEV
>> > +/* Write len data bytes in a mbuf at specified offset.
>> > + *
>> > + * 'mbuf', pointer to the destination mbuf where 'ofs' is, and the
>> mbuf
>>  > +where
>> > + * the data will first be written.
>> > + * 'ofs', the offset within the provided 'mbuf' where 'data' is to
>> be
>> > written.
>> > + * 'len', the size of the to be written 'data'.
>> > + * 'data', pointer to the to be written bytes.
>> > + *
>> > + * XXX: This function is the counterpart of the
>> `rte_pktmbuf_read()`
>>  > +function
>>  > + * available with DPDK, in the rte_mbuf.h */ void
>> > +dp_packet_mbuf_write(struct rte_mbuf *mbuf, int16_t ofs, uint32_t
>> len,
>> > + const void *data)
>> > +{
>> > +char *dst_addr;
>> 

Re: [ovs-dev] [PATCH v3] rhel: Use correct user in the logrotate configuration file

[Timothy Redaelli]

On Wed,  8 Aug 2018 17:27:25 +0300
Markos Chandras  wrote:

> The /var/log/openvswitch directory is owned by the openvswitch user
> but logrotate could be running as root or as another user. As a
> result of which, rpmlint prints the following warning when building
> the spec file on SUSE Linux Enterprise:
> 
> openvswitch.x86_64: W:
> suse-logrotate-user-writable-log-dir /var/log/openvswitch
> openvswitch:openvswitch 0750 The log directory is writable by
> unprivileged users. Please fix the permissions so only root can write
> there or add the 'su' option to your logrotate config
> 
> In order to fix that, we should run the logrotate script as the same
> user which runs the various Open vSwitch daemons. If this is a new
> installation, then this user is the 'openvswitch' one, but if we are
> upgrading from an older release, then the user is normally 'root'.
> As such, we set the initial user to 'root' and we fix this up in the
> %post scriptlet.
> 
> Cc: Aaron Conole 
> Cc: Timothy Redaelli 
> Signed-off-by: Markos Chandras 

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

[ovs-dev] [RFC 2/2] netdev-dpdk: Enable TSO when using multi-seg mbufs

[Tiago Lam]

TCP Segmentation Offload (TSO) is a feature which enables the TCP/IP
network stack to delegate segmentation of a TCP segment to the hardware
NIC, thus saving compute resources. This may improve performance
significantly for TCP workload in virtualized environments.

While a previous commit already added the necesary logic to netdev-dpdk
to deal with packets marked for TSO, this set of changes enables TSO by
default when using multi-segment mbufs.

Thus, to enable TSO on the physical DPDK interfaces, only the following
command needs to be issued before starting OvS:
ovs-vsctl set Open_vSwitch . other_config:dpdk-multi-seg-mbufs=true

Co-authored-by: Mark Kavanagh 

Signed-off-by: Mark Kavanagh 
Signed-off-by: Tiago Lam 
---
 Documentation/topics/dpdk/phy.rst | 64 +++
 lib/netdev-dpdk.c | 52 ++-
 2 files changed, 108 insertions(+), 8 deletions(-)

diff --git a/Documentation/topics/dpdk/phy.rst 
b/Documentation/topics/dpdk/phy.rst
index 1470623..980a629 100644
--- a/Documentation/topics/dpdk/phy.rst
+++ b/Documentation/topics/dpdk/phy.rst
@@ -248,3 +248,67 @@ Command to set interrupt mode for a specific interface::
 
 Command to set polling mode for a specific interface::
 $ ovs-vsctl set interface  options:dpdk-lsc-interrupt=false
+
+TCP Segmentation Offload (TSO)
+--
+
+Overview
+
+
+TCP Segmentation Offload (TSO) enables a network stack to delegate
+segmentation of an oversized TCP segment to the underlying physical NIC.
+Offload of frame segmentation achieves computational savings in the core,
+freeing up CPU cycles for more useful work.
+
+DPDK v16.07 added support for `TSO` in the vHost user backend; as such, a
+guest's virtual network interfaces may avail of `TSO`. In such a setup, the
+aforementioned computational savings are made in the core acting as the VM's
+virtual CPU, typically resulting in improved TCP throughput.
+
+To enable TSO in a guest, the underlying NIC must first support `TSO` -
+consult your controller's datasheet for compatibility. Secondly, the NIC
+must have an associated DPDK Poll Mode Driver (PMD) which supports `TSO`.
+
+Enabling TSO
+
+
+TSO may be enabled in one of two ways, as follows:
+
+  1. QEMU Command Line Parameter:
+
+  ```
+  sudo $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64 \
+  ...
+  -device virtio-net-pci,mac=00:00:00:00:00:01,netdev=mynet1,\
+  mrg_rxbuf=on,csum=on,gso=on,guest_csum=on,guest_tso4=on,\
+  guest_tso6=on,guest_ecn=on \
+  ...
+  ```
+
+  2. ethtool
+
+`TSO` is enabled in OvS by the DPDK vHost User backend; when a new guest
+connection is established, `TSO` is advertised to the guest as an available
+feature. Assuming that the guest's OS also supports `TSO`, ethtool can be used
+to enable same:
+
+  ```
+  ethtool -K eth0 sg on # scatter-gather is a prerequisite for TSO
+  ethtool -K eth0 tso on
+  ethtool -k eth0   # verify that TSO is reported as 'on'
+  ```
+
+  Note: In both methods, `mergeable buffers` are required:
+  ```
+  sudo $QEMU_DIR/x86_64-softmmu/qemu-system-x86_64 \
+  ...
+  mrg_rxbuf=on,\
+  ...
+  ```
+
+Limitations
+~~~
+
+The current OvS `TSO` implementation supports flat and VLAN networks only
+(i.e. no support for `TSO` over tunneled connection [VxLAN, GRE, IPinIP,
+etc.]).
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 5da5996..20d4fd5 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -379,6 +379,7 @@ struct ingress_policer {
 enum dpdk_hw_ol_features {
 NETDEV_RX_CHECKSUM_OFFLOAD = 1 << 0,
 NETDEV_RX_HW_CRC_STRIP = 1 << 1,
+NETDEV_TX_TSO_OFFLOAD = 1 << 2,
 };
 
 /*
@@ -1003,6 +1004,8 @@ dpdk_eth_dev_port_config(struct netdev_dpdk *dev, int 
n_rxq, int n_txq)
 struct rte_eth_dev_info info;
 uint16_t conf_mtu;
 
+rte_eth_dev_info_get(dev->port_id, );
+
 /* As of DPDK 17.11.1 a few PMDs require to explicitly enable
  * scatter to support jumbo RX. Checking the offload capabilities
  * is not an option as PMDs are not required yet to report
@@ -1010,7 +1013,6 @@ dpdk_eth_dev_port_config(struct netdev_dpdk *dev, int 
n_rxq, int n_txq)
  * (testing or code review). Listing all such PMDs feels harder
  * than highlighting the one known not to need scatter */
 if (dev->mtu > ETHER_MTU) {
-rte_eth_dev_info_get(dev->port_id, );
 if (strncmp(info.driver_name, "net_nfp", 7)) {
 conf.rxmode.enable_scatter = 1;
 }
@@ -1018,14 +1020,28 @@ dpdk_eth_dev_port_config(struct netdev_dpdk *dev, int 
n_rxq, int n_txq)
 
 /* Multi-segment-mbuf-specific setup. */
 if (dpdk_multi_segment_mbufs) {
-/* DPDK PMDs typically attempt to use simple or vectorized
- * transmit functions, neither of which are compatible with
- * multi-segment mbufs. Ensure that these are disabled when
- * 

[ovs-dev] [RFC 1/2] netdev-dpdk: Consider packets marked for TSO.

[Tiago Lam]

Previously, TSO was being explicity disabled on vhost interfaces,
meaning the guests wouldn't have TSO support negotiated in. With TSO
negotiated and enabled, packets are now marked for TSO, through the
PKT_TX_TCP_SEG flag.

In order to deal with this type of packets, a new function,
netdev_dpdk_prep_tso_packet(), has been introduced, with the main
purpose of setting correctly the l2, l3 and l4 length members of the
mbuf struct, and the appropriate ol_flags. This function supports TSO
both in IPv4 and IPv6.

netdev_dpdk_prep_tso_packet() is then only called when packets are
marked with the PKT_TX_TCP_SEG flag, meaning they have been marked for
TSO, and when the packet will be traversing the NIC.

Co-authored-by: Mark Kavanagh 

Signed-off-by: Mark Kavanagh 
Signed-off-by: Tiago Lam 
---
 lib/dp-packet.c   |   5 ++-
 lib/netdev-dpdk.c | 120 +-
 2 files changed, 96 insertions(+), 29 deletions(-)

diff --git a/lib/dp-packet.c b/lib/dp-packet.c
index 6773535..412c553 100644
--- a/lib/dp-packet.c
+++ b/lib/dp-packet.c
@@ -56,7 +56,6 @@ dp_packet_copy_mbuf_flags(struct dp_packet *dst, const struct 
dp_packet *src)
 struct rte_mbuf *buf_dst = &(dst->mbuf);
 struct rte_mbuf buf_src = src->mbuf;
 
-buf_dst->nb_segs = buf_src.nb_segs;
 buf_dst->ol_flags = buf_src.ol_flags;
 buf_dst->packet_type = buf_src.packet_type;
 buf_dst->tx_offload = buf_src.tx_offload;
@@ -184,6 +183,7 @@ dp_packet_clone_with_headroom(const struct dp_packet *b, 
size_t headroom) {
 /* copy multi-seg data */
 if (b->source == DPBUF_DPDK && b->mbuf.nb_segs > 1) {
 void *dst = NULL;
+struct rte_mbuf *new_mbuf = NULL;
 struct rte_mbuf *mbuf = CONST_CAST(struct rte_mbuf *, >mbuf);
 
 new_buffer = dp_packet_new_with_headroom(pkt_len, headroom);
@@ -193,6 +193,9 @@ dp_packet_clone_with_headroom(const struct dp_packet *b, 
size_t headroom) {
 if (!rte_pktmbuf_read(mbuf, 0, pkt_len, dst)) {
 return NULL;
 }
+
+new_mbuf = CONST_CAST(struct rte_mbuf *, _buffer->mbuf);
+new_mbuf->nb_segs = 1;
 } else {
 new_buffer = dp_packet_clone_data_with_headroom(dp_packet_data(b),
 dp_packet_size(b),
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index b18b768..5da5996 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -28,6 +28,8 @@
 
 #include 
 #include 
+#include "rte_ip.h"
+#include "rte_tcp.h"
 #include 
 #include 
 #include 
@@ -1375,16 +1377,6 @@ netdev_dpdk_vhost_construct(struct netdev *netdev)
 goto out;
 }
 
-err = rte_vhost_driver_disable_features(dev->vhost_id,
-1ULL << VIRTIO_NET_F_HOST_TSO4
-| 1ULL << VIRTIO_NET_F_HOST_TSO6
-| 1ULL << VIRTIO_NET_F_CSUM);
-if (err) {
-VLOG_ERR("rte_vhost_driver_disable_features failed for vhost user "
- "port: %s\n", name);
-goto out;
-}
-
 err = rte_vhost_driver_start(dev->vhost_id);
 if (err) {
 VLOG_ERR("rte_vhost_driver_start failed for vhost user "
@@ -2019,6 +2011,57 @@ netdev_dpdk_rxq_dealloc(struct netdev_rxq *rxq)
 rte_free(rx);
 }
 
+/* Should only be called if PKT_TX_TCP_SEG is set in ol_flags.
+ * Furthermore, it also sets the PKT_TX_TCP_CKSUM and PKT_TX_IP_CKSUM flags,
+ * and PKT_TX_IPV4 and PKT_TX_IPV6 in case the packet is IPv4 or IPv6,
+ * respectiveoly. */
+static void
+netdev_dpdk_prep_tso_packet(struct rte_mbuf *mbuf, int mtu)
+{
+struct dp_packet *pkt;
+struct tcp_header *th;
+struct ether_hdr *m_eth_hdr;
+struct tcp_hdr *m_tcp_hdr;
+char *m_l3_hdr;
+
+pkt = CONTAINER_OF(mbuf, struct dp_packet, mbuf);
+mbuf->l2_len = (char *) dp_packet_l3(pkt) - (char *) dp_packet_eth(pkt);
+mbuf->l3_len = (char *) dp_packet_l4(pkt) - (char *) dp_packet_l3(pkt);
+th = dp_packet_l4(pkt);
+/* There's no layer 4 in the packet */
+if (!th) {
+return;
+}
+mbuf->l4_len = TCP_OFFSET(th->tcp_ctl) * 4;
+mbuf->outer_l2_len = 0;
+mbuf->outer_l3_len = 0;
+
+if (!(mbuf->ol_flags & PKT_TX_TCP_SEG)) {
+return;
+}
+
+m_eth_hdr = rte_pktmbuf_mtod(mbuf, struct ether_hdr *);
+m_l3_hdr = (char *) m_eth_hdr + mbuf->l2_len;
+m_tcp_hdr = (struct tcp_hdr *) ((char *) m_l3_hdr + mbuf->l3_len);
+
+mbuf->ol_flags |= PKT_TX_TCP_CKSUM;
+mbuf->ol_flags |= PKT_TX_IP_CKSUM;
+
+/* Set the size of each TCP segment, based on the MTU of the device */
+mbuf->tso_segsz = mtu - mbuf->l3_len - mbuf->l4_len;
+
+if (mbuf->ol_flags & PKT_TX_IPV4) {
+/* IPv4 packet */
+struct ipv4_hdr *ipv4_hdr = (struct ipv4_hdr *) m_l3_hdr;
+ipv4_hdr->hdr_checksum = 0;
+m_tcp_hdr->cksum = (rte_ipv4_phdr_cksum(ipv4_hdr, mbuf->ol_flags));
+} else {
+/* IPv6 packet */
+struct ipv6_hdr *ipv6_hdr = (struct 

[ovs-dev] [RFC 0/2] dpdk: Add support for TSO

[Tiago Lam]

Enabling TSO offload allows a host stack to delegate the segmentation of
oversized TCP packets to the underlying physical NIC, if supported. In the case
of a VM this means that the segmentation of the packets is not performed by the
guest kernel, but by the host NIC itself. In turn, since the TSO calculations
and checksums are being performed in hardware, this alleviates the CPU load on
the host system. In inter VM communication this might account to significant
savings, and higher throughput, even more so if the VMs are running on the same
host.

Thus, although inter VM communication is already possible as is, there's a
sacrifice in terms of CPU, which may affect the overall throughput.

This series proposes to add support for TSO in OvS-DPDK, by making use of the
TSO offloading feature already supported by DPDK vhost backend, having the
following scenarios in mind:
- Inter VM communication on the same host;
- Inter VM communication on different hosts;
- The same two use cases above, but on a VLAN network.

The work is based on [1]; It has been rebased to run on top of the
multi-segment mbufs work (v7) [2] and re-worked to use the new Tx offload API
as per [3].

[1] https://patchwork.ozlabs.org/patch/749564/
[2] https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/350081.html
[3] http://dpdk.readthedocs.io/en/v17.11/rel_notes/deprecation.html

Considerations:
- This series depends on the multi-segment mbuf series (v7) and can't be
  applied on master as is;
- Right now TSO is enabled by default when using multi-segment mbufs, or
  otherwise TSO is disabled and can't be set on its own. I'm open to opinions
  on this, though. My main idea to stick TSO behind multi-segment mbufs is:
  - The performance enhancements enabled by DPDK PMDs, such as vectorization,
that are disabled when using multi-segment mbufs are also disabled when
using some of the offload features;
  - By enabling the multi-segment mbuf flag, the packets have a default size of
2048B. This might be higher if not using multi-segments mbufs, since the
size of the packet will be ajusted to the MTU (such as 9000B). This might
lead to a higher waste of memory when appending mbufs to each other as we
will be increamenting larger ammounts.
- There's some initial documentation on patch 1/2 (which came form [1]), but
  needs improving.

Tiago Lam (2):
  netdev-dpdk: Consider packets marked for TSO.
  netdev-dpdk: Enable TSO when using multi-seg mbufs

 Documentation/topics/dpdk/phy.rst |  64 ++
 lib/dp-packet.c   |   5 +-
 lib/netdev-dpdk.c | 172 ++
 3 files changed, 204 insertions(+), 37 deletions(-)

-- 
2.7.4

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

Re: [ovs-dev] [PATCH v2 1/2] table: append newline when printing tables

[Terry Wilson]

On Tue, Aug 7, 2018 at 7:34 PM, Aaron Conole  wrote:
> With commit cb139fa8b3a1 ("table: New function table_format() for
> formatting a table as a string.") a new mechanism for formatting
> tables was introduced, and the table_print method was refactored to
> use this.
>
> During that refactor, calls to 'puts' were replaced with
> 'ds_put_cstr', and table print was changed to use 'fputs(...,
> stdout)'.  Unfortunately, fputs() does not append a newline to the
> string provided, and changes the output strings of, for example,
> ovsdb-client dump to print all on one line.  This means
> post-processing scripts that are chained after ovsdb-client would
> either block indefinitely (if they don't detect EOF), or process the
> entire bundle at once (rather than seeing each table on a separate
> line).
>
> Fixes: cb139fa8b3a1 ("table: New function table_format() for formatting a 
> table as a string.")
> Cc: Ben Pfaff 
> Cc: Jakub Sitnicki 
> Reported-by: Terry Wilson 
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1608508
> Signed-off-by: Aaron Conole 
> Suggested-by: Ben Pfaff 
> ---
>  lib/table.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/lib/table.c b/lib/table.c
> index cd811caf5..19bf89262 100644
> --- a/lib/table.c
> +++ b/lib/table.c
> @@ -547,6 +547,7 @@ table_print_json__(const struct table *table, const 
> struct table_style *style,
>  json_object_put(json, "data", data);
>
>  json_to_ds(json, style->json_flags, s);
> +ds_put_char(s, '\n');
>  json_destroy(json);
>  }
>
> --
> 2.14.3
>

I can verify that this re-adds the newline and that it fixes the issue
in neutron's ovs-agent.

Acked-by: Terry Wilson 
Tested-by: Terry Wilson 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] datapath: meter: Fix setting meter id for new entries

[Ben Pfaff]

Justin, I already applied this as a straightforward backport.  Hope it
doesn't disrupt your work.

On Tue, Aug 07, 2018 at 08:31:35PM -0700, Justin Pettit wrote:
> Thanks, Greg. I actually have this queued up with another patch that will 
> disable meters entirely on broken kernels. I plan to send that out tomorrow. 
> 
> --Justin
> 
> 
> > On Aug 7, 2018, at 4:45 PM, Greg Rose  wrote:
> > 
> > From: Justin Pettit 
> > 
> > Upstream commit:
> >From: Justin Pettit 
> >Date: Sat, 28 Jul 2018 15:26:01 -0700
> >Subject: [PATCH] openvswitch: meter: Fix setting meter id for new entries
> > 
> >The meter code would create an entry for each new meter.  However, it
> >would not set the meter id in the new entry, so every meter would appear
> >to have a meter id of zero.  This commit properly sets the meter id when
> >adding the entry.
> > 
> >Fixes: 96fbc13d7e77 ("openvswitch: Add meter infrastructure")
> >Signed-off-by: Justin Pettit 
> >Cc: Andy Zhou 
> >Signed-off-by: David S. Miller 
> > 
> > Cc: Justin Pettit 
> > Signed-off-by: Greg Rose 
> > ---
> > datapath/meter.c | 10 +-
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/datapath/meter.c b/datapath/meter.c
> > index 1c2ed46..281d869 100644
> > --- a/datapath/meter.c
> > +++ b/datapath/meter.c
> > @@ -221,6 +221,7 @@ static struct dp_meter *dp_meter_create(struct nlattr 
> > **a)
> >if (!meter)
> >return ERR_PTR(-ENOMEM);
> > 
> > +meter->id = nla_get_u32(a[OVS_METER_ATTR_ID]);
> >meter->used = div_u64(ktime_get_ns(), 1000 * 1000);
> >meter->kbps = a[OVS_METER_ATTR_KBPS] ? 1 : 0;
> >meter->keep_stats = !a[OVS_METER_ATTR_CLEAR];
> > @@ -290,6 +291,10 @@ static int ovs_meter_cmd_set(struct sk_buff *skb, 
> > struct genl_info *info)
> >u32 meter_id;
> >bool failed;
> > 
> > +if (!a[OVS_METER_ATTR_ID]) {
> > +return -ENODEV;
> > +}
> > +
> >meter = dp_meter_create(a);
> >if (IS_ERR_OR_NULL(meter))
> >return PTR_ERR(meter);
> > @@ -308,11 +313,6 @@ static int ovs_meter_cmd_set(struct sk_buff *skb, 
> > struct genl_info *info)
> >goto exit_unlock;
> >}
> > 
> > -if (!a[OVS_METER_ATTR_ID]) {
> > -err = -ENODEV;
> > -goto exit_unlock;
> > -}
> > -
> >meter_id = nla_get_u32(a[OVS_METER_ATTR_ID]);
> > 
> >/* Cannot fail after this. */
> > -- 
> > 1.8.3.1
> > 
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [ovs-dev, v3] rhel: Use correct user in the logrotate configuration file

[0-day Robot]

Bleep bloop.  Greetings Markos Chandras, 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 84 characters long (recommended limit is 79)
#82 FILE: rhel/usr_lib_systemd_system_ovsdb-server.service:13:
ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch 
/var/log/openvswitch

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


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

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

Re: [ovs-dev] [PATCH v6] dpif-netdev: Avoid reordering of packets in a batch with same megaflow

[Ilya Maximets]

On 08.08.2018 12:06, Ian Stokes wrote:
> On 7/27/2018 7:26 PM, Vishal Deep Ajmera wrote:
>> OVS reads packets in batches from a given port and packets in the
>> batch are subjected to potentially 3 levels of lookups to identify
>> the datapath megaflow entry (or flow) associated with the packet.
>> Each megaflow entry has a dedicated buffer in which packets that match
>> the flow classification criteria are collected. This buffer helps OVS
>> perform batch processing for all packets associated with a given flow.
>>
>> Each packet in the received batch is first subjected to lookup in the
>> Exact Match Cache (EMC). Each EMC entry will point to a flow. If the
>> EMC lookup is successful, the packet is moved from the rx batch to the
>> per-flow buffer.
>>
>> Packets that did not match any EMC entry are rearranged in the rx batch
>> at the beginning and are now subjected to a lookup in the megaflow cache.
>> Packets that match a megaflow cache entry are *appended* to the per-flow
>> buffer.
>>
>> Packets that do not match any megaflow entry are subjected to slow-path
>> processing through the upcall mechanism. This cannot change the order of
>> packets as by definition upcall processing is only done for packets
>> without matching megaflow entry.
>>
>> The EMC entry match fields encompass all potentially significant header
>> fields, typically more than specified in the associated flow's match
>> criteria. Hence, multiple EMC entries can point to the same flow. Given
>> that per-flow batching happens at each lookup stage, packets belonging
>> to the same megaflow can get re-ordered because some packets match EMC
>> entries while others do not.
>>
>> The following example can illustrate the issue better. Consider
>> following batch of packets (labelled P1 to P8) associated with a single
>> TCP connection and associated with a single flow. Let us assume that
>> packets with just the ACK bit set in TCP flags have been received in a
>> prior batch also and a corresponding EMC entry exists.
>>
>> 1. P1 (TCP Flag: ACK)
>> 2. P2 (TCP Flag: ACK)
>> 3. P3 (TCP Flag: ACK)
>> 4. P4 (TCP Flag: ACK, PSH)
>> 5. P5 (TCP Flag: ACK)
>> 6. P6 (TCP Flag: ACK)
>> 7. P7 (TCP Flag: ACK)
>> 8. P8 (TCP Flag: ACK)
>>
>> The megaflow classification criteria does not include TCP flags while
>> the EMC match criteria does. Thus, all packets other than P4 match
>> the existing EMC entry and are moved to the per-flow packet batch.
>> Subsequently, packet P4 is moved to the same per-flow packet batch as
>> a result of the megaflow lookup. Though the packets have all been
>> correctly classified as being associated with the same flow, the
>> packet order has not been preserved because of the per-flow batching
>> performed during the EMC lookup stage. This packet re-ordering has
>> performance implications for TCP applications.
>>
>> This patch preserves the packet ordering by performing the per-flow
>> batching after both the EMC and megaflow lookups are complete. As an
>> optimization, packets are flow-batched in emc processing till any
>> packet in the batch has an EMC miss.
>>
>> A new flow map is maintained to keep the original order of packet
>> along with flow information. Post fastpath processing, packets from
>> flow map are *appended* to per-flow buffer.
>>
> 
> Thanks for the V6 Vishal, looking at this today myself.
> 
> Ilya, has the v6 addressed your concerns from the v5?

Hi.
Ian, I had no much time. So, I'm still looking at the patch.
Vishal, It'll be nice if you'll add version history for your
patches under the cut line. It's hard to track all the changes
directly from the code.

Best regards, Ilya Maximets.

> Thanks
> Ian
> 
>> Signed-off-by: Vishal Deep Ajmera 
>> Co-authored-by: Venkatesan Pradeep 
>> Signed-off-by: Venkatesan Pradeep 
>> ---
>>   lib/dpif-netdev.c | 125 
>> +-
>>   1 file changed, 106 insertions(+), 19 deletions(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index 13a20f0..3ea25e2 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -244,6 +244,13 @@ struct dpcls_rule {
>>   /* 'flow' must be the last field, additional space is allocated here. 
>> */
>>   };
>>   +/* Data structure to keep packet order till fastpath processing. */
>> +struct dp_packet_flow_map {
>> +    struct dp_packet *packet;
>> +    struct dp_netdev_flow *flow;
>> +    uint16_t tcp_flags;
>> +};
>> +
>>   static void dpcls_init(struct dpcls *);
>>   static void dpcls_destroy(struct dpcls *);
>>   static void dpcls_sort_subtable_vector(struct dpcls *);
>> @@ -5774,6 +5781,19 @@ dp_netdev_queue_batches(struct dp_packet *pkt,
>>   packet_batch_per_flow_update(batch, pkt, tcp_flags);
>>   }
>>   +static inline void
>> +packet_enqueue_to_flow_map(struct dp_packet *packet,
>> +   struct dp_netdev_flow *flow,
>> +   uint16_t tcp_flags,
>> +   struct dp_packet_flow_map *flow_map,
>> 

[ovs-dev] [PATCH] ovn-ctl: allow configuring user:group for daemons

[Aaron Conole]

Add two options, one for controlling the ovs daemon user/group, and the
other for controlling the ovn daemon user/group.  This allows a fine-grained
split between OVN and OVS daemons, and keeps the syntax and user/group
separation from ovs-ctl when running ovn-ctl.

Signed-off-by: Aaron Conole 
---
 NEWS|  3 ++-
 ovn/utilities/ovn-ctl   | 14 ++
 ovn/utilities/ovn-ctl.8.xml |  2 ++
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 7875f6673..64d4ed5e3 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,7 @@
 Post-v2.10.0
 -
-
+   - ovn:
+ * ovn-ctl: allow passing user:group ids to the OVN daemons.
 
 v2.10.0 - xx xxx 
 -
diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
index 296e5b82c..3ff0df68e 100755
--- a/ovn/utilities/ovn-ctl
+++ b/ovn/utilities/ovn-ctl
@@ -172,6 +172,8 @@ $cluster_remote_port
 set "$@" --remote=punix:$sock --pidfile=$pid
 set "$@" --unixctl=ovn${db}_db.ctl
 
+[ "$OVS_USER" != "" ] && set "$@" --user "$OVS_USER"
+
 if test X"$detach" != Xno; then
 set "$@" --detach --monitor
 else
@@ -293,6 +295,8 @@ start_northd () {
 set "$@" --log-file=$OVN_NORTHD_LOGFILE
 fi
 
+[ "$OVN_USER" != "" ] && set "$@" --user "$OVN_USER"
+
 set "$@" $OVN_NORTHD_LOG $ovn_northd_params
 
 OVS_RUNDIR=${OVN_RUNDIR} start_daemon "$OVN_NORTHD_PRIORITY" 
"$OVN_NORTHD_WRAPPER" "$@"
@@ -314,6 +318,9 @@ start_controller () {
 if test X"$OVN_CONTROLLER_SSL_BOOTSTRAP_CA_CERT" != X; then
 set "$@" --bootstrap-ca-cert=$OVN_CONTROLLER_SSL_BOOTSTRAP_CA_CERT
 fi
+
+[ "$OVN_USER" != "" ] && set "$@" --user "$OVN_USER"
+
 OVS_RUNDIR=${OVN_RUNDIR} start_daemon "$OVN_CONTROLLER_PRIORITY" 
"$OVN_CONTROLLER_WRAPPER" "$@"
 }
 
@@ -332,6 +339,9 @@ start_controller_vtep () {
 if test X"$OVN_CONTROLLER_SSL_BOOTSTRAP_CA_CERT" != X; then
 set "$@" --bootstrap-ca-cert=$OVN_CONTROLLER_SSL_BOOTSTRAP_CA_CERT
 fi
+
+[ "$OVN_USER" != "" ] && set "$@" --user "$OVN_USER"
+
 OVS_RUNDIR=${OVN_RUNDIR} start_daemon "$OVN_CONTROLLER_PRIORITY" 
"$OVN_CONTROLLER_WRAPPER" "$@"
 }
 
@@ -429,6 +439,8 @@ set_defaults () {
 
 OVS_RUNDIR=${OVS_RUNDIR:-${rundir}}
 OVN_RUNDIR=${OVN_RUNDIR:-${OVS_RUNDIR}}
+OVN_USER=
+OVS_USER=
 
 OVN_CONTROLLER_LOG="-vconsole:emer -vsyslog:err -vfile:info"
 OVN_NORTHD_LOG="-vconsole:emer -vsyslog:err -vfile:info"
@@ -535,6 +547,8 @@ Options:
   --ovn-northd-logfile=STRINGovn northd process log file (default: 
$OVN_NORTHD_LOGFILE)
   --ovn-nb-log=STRING ovn NB ovsdb-server processes logging params 
(default: $OVN_NB_LOG)
   --ovn-sb-log=STRING ovn SB ovsdb-server processes logging params 
(default: $OVN_SB_LOG)
+  --ovn-user="user[:group]"  pass the --user flag to the ovn daemons
+  --ovs-user="user[:group]"  pass the --user flag to ovs daemons
   -h, --help display this help message
 
 File location options:
diff --git a/ovn/utilities/ovn-ctl.8.xml b/ovn/utilities/ovn-ctl.8.xml
index 02235fe1e..3b0e67a45 100644
--- a/ovn/utilities/ovn-ctl.8.xml
+++ b/ovn/utilities/ovn-ctl.8.xml
@@ -44,6 +44,8 @@
 --ovn-northd-wrapper=WRAPPER
 --ovn-controller-priority=NICE
 --ovn-controller-wrapper=WRAPPER
+--ovn-user=USER:GROUP
+--ovs-user=USER:GROUP
 -h | --help
 
 File location options
-- 
2.14.3

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

[ovs-dev] [PATCH v3] rhel: Use correct user in the logrotate configuration file

[Markos Chandras]

The /var/log/openvswitch directory is owned by the openvswitch user but
logrotate could be running as root or as another user. As a result of
which, rpmlint prints the following warning when building the spec file
on SUSE Linux Enterprise:

openvswitch.x86_64: W: suse-logrotate-user-writable-log-dir 
/var/log/openvswitch openvswitch:openvswitch 0750
The log directory is writable by unprivileged users. Please fix the
permissions so only root can write there or add the 'su' option
to your logrotate config

In order to fix that, we should run the logrotate script as the same
user which runs the various Open vSwitch daemons. If this is a new
installation, then this user is the 'openvswitch' one, but if we are
upgrading from an older release, then the user is normally 'root'.
As such, we set the initial user to 'root' and we fix this up in the
%post scriptlet.

Cc: Aaron Conole 
Cc: Timothy Redaelli 
Signed-off-by: Markos Chandras 
---
 rhel/etc_logrotate.d_openvswitch | 1 +
 rhel/openvswitch-fedora.spec.in  | 4 +++-
 rhel/usr_lib_systemd_system_ovsdb-server.service | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/rhel/etc_logrotate.d_openvswitch b/rhel/etc_logrotate.d_openvswitch
index ed7d733c9..f4302ffbc 100644
--- a/rhel/etc_logrotate.d_openvswitch
+++ b/rhel/etc_logrotate.d_openvswitch
@@ -6,6 +6,7 @@
 # without warranty of any kind.
 
 /var/log/openvswitch/*.log {
+su root root
 daily
 compress
 sharedscripts
diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 9f8664e95..c2d3200e1 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -405,6 +405,7 @@ exit 0
 %post
 if [ $1 -eq 1 ]; then
 sed -i 's:^#OVS_USER_ID=:OVS_USER_ID=:' /etc/sysconfig/openvswitch
+sed -i 's:\(.*su\).*:\1 openvswitch openvswitch:' 
%{_sysconfdir}/logrotate.d/openvswitch
 
 %if %{with dpdk}
 sed -i \
@@ -414,6 +415,7 @@ if [ $1 -eq 1 ]; then
 
 # In the case of upgrade, this is not needed.
 chown -R openvswitch:openvswitch /etc/openvswitch
+chown -R openvswitch:openvswitch /var/log/openvswitch
 fi
 
 %if 0%{?systemd_post:1}
@@ -601,7 +603,7 @@ fi
 %endif
 %doc NOTICE README.rst NEWS rhel/README.RHEL.rst
 /var/lib/openvswitch
-%attr(750,openvswitch,openvswitch) /var/log/openvswitch
+%attr(750,root,root) /var/log/openvswitch
 %ghost %attr(755,root,root) %{_rundir}/openvswitch
 
 %files ovn-docker
diff --git a/rhel/usr_lib_systemd_system_ovsdb-server.service 
b/rhel/usr_lib_systemd_system_ovsdb-server.service
index 0fa57a925..70da1ec95 100644
--- a/rhel/usr_lib_systemd_system_ovsdb-server.service
+++ b/rhel/usr_lib_systemd_system_ovsdb-server.service
@@ -10,7 +10,7 @@ Type=forking
 Restart=on-failure
 EnvironmentFile=/etc/openvswitch/default.conf
 EnvironmentFile=-/etc/sysconfig/openvswitch
-ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch
+ExecStartPre=/usr/bin/chown ${OVS_USER_ID} /var/run/openvswitch 
/var/log/openvswitch
 ExecStartPre=/bin/sh -c 'rm -f /run/openvswitch/useropts; if [ 
"$${OVS_USER_ID/:*/}" != "root" ]; then /usr/bin/echo 
"OVSUSER=--ovs-user=${OVS_USER_ID}" > /run/openvswitch/useropts; fi'
 EnvironmentFile=-/run/openvswitch/useropts
 ExecStart=/usr/share/openvswitch/scripts/ovs-ctl \
-- 
2.16.4

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

Re: [ovs-dev] [PATCH v2 2/2] table: fix html buffer output

[Aaron Conole]

Aaron Conole  writes:

> Prior to this commit, html output exhibiits a doppler effect for

Ironically, I duplicated an 'i' in the word exhibits here.  Let me know
if you want a respin or if it can be fixed while applying. :-x

> content by continually printing strings passed from
> table_print_html_cell.
>
> Fixes: cb139fa8b3a1 ("table: New function table_format() for formatting a 
> table as a string.")
> Cc: Ben Pfaff 
> Cc: Jakub Sitnicki 
> Signed-off-by: Aaron Conole 
> ---
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCHv3] netdev-dpdk: Fix failure to configure flow control at netdev-init.

[Stokes, Ian]

> Configuring flow control at ixgbe netdev-init is throwing error in port
> start.
> 
> For eg: without this fix, user cannot configure flow control on ixgbe dpdk
> port as below,
> 
> "
> ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \
> options:dpdk-devargs=:05:00.1 options:rx-flow-ctrl=true "
> 
> Instead,  it must be configured as two different commands,
> 
> "
> ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \
>options:dpdk-devargs=:05:00.1
> ovs-vsctl set Interface dpdk0 options:rx-flow-ctrl=true "
> 
> The DPDK ixgbe driver is now validating all the 'rte_eth_fc_conf' fields
> before trying to configuring the dpdk ethdev. Hence OVS can no longer set
> the 'dont care' fields to just '0' as before. This commit make sure all
> the 'rte_eth_fc_conf' fields are populated with default values before the
> dev init.
> Also to avoid read error on unsupported ports, the flow control parameters
> are now read only when user is trying to configure/update it.
> 
> Signed-off-by: Sugesh Chandran 
> ---
> V1 -> V2
> Read DPDK port flow-control parameters only when reconfiguration is
> required.
> This will avoid flow control read error on unsupported ports.
> 
> V2 -> V3
>  Minor modifications to incorporate review comments from Ian.
>  Merged to latest master.
> 
> ---

Thanks for the work on this Sugesh, I'll add this to the pull request this 
week. I've backported it to 2.20, 2.9 also. However the flow control logic 
changed between then and 2.8/2.7. Can you check this approach against those 
branches if a backported is required there?

Thanks
Ian
>  lib/netdev-dpdk.c | 14 ++
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 9bf2185..e40b03a
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -1065,14 +1065,6 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
> 
>  mbp_priv = rte_mempool_get_priv(dev->dpdk_mp->mp);
>  dev->buf_size = mbp_priv->mbuf_data_room_size - RTE_PKTMBUF_HEADROOM;
> -
> -/* Get the Flow control configuration for DPDK-ETH */
> -diag = rte_eth_dev_flow_ctrl_get(dev->port_id, >fc_conf);
> -if (diag) {
> -VLOG_DBG("cannot get flow control parameters on port
> "DPDK_PORT_ID_FMT
> - ", err=%d", dev->port_id, diag);
> -}
> -
>  return 0;
>  }
> 
> @@ -1776,6 +1768,12 @@ netdev_dpdk_set_config(struct netdev *netdev, const
> struct smap *args,
>  if (dev->fc_conf.mode != fc_mode || autoneg != dev->fc_conf.autoneg)
> {
>  dev->fc_conf.mode = fc_mode;
>  dev->fc_conf.autoneg = autoneg;
> +/* Get the Flow control configuration for DPDK-ETH */
> +err = rte_eth_dev_flow_ctrl_get(dev->port_id, >fc_conf);
> +if (err) {
> +VLOG_WARN("Cannot get flow control parameters on port "
> +DPDK_PORT_ID_FMT", err=%d", dev->port_id, err);
> +}
>  dpdk_eth_flow_ctrl_setup(dev);
>  }
> 
> --
> 2.7.4

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

Re: [ovs-dev] [PATCH v4 1/2] ovs python: ovs.stream.open_block() returns success even if the remote is unreachable

[Mark Michelson]

On 08/08/2018 03:24 AM, Numan Siddique wrote:



On Wed, Aug 8, 2018 at 2:58 AM Ben Pfaff > wrote:


On Tue, Aug 07, 2018 at 05:12:32PM -0400, Mark Michelson wrote:
 > On 08/07/2018 07:37 AM, nusid...@redhat.com
 wrote:
 > >From: Numan Siddique mailto:nusid...@redhat.com>>
 > >
 > >The python function
ovs.socket_util.check_connection_completion() uses select()
 > >(provided by python) to monitor the socket file descriptor. The
select()
 > >returns 1 when the file descriptor becomes ready. For error
cases like -
 > >111 (Connection refused) and 113 (No route to host) (POLLERR),
ovs.poller._SelectSelect.poll()
 > >expects the exceptfds list to be set by select(). But that is
not the case.
 > >As per the select() man page, writefds list will be set for POLLERR.
 > >Please see "Correspondence between select() and poll()
notifications" section of select(2)
 > >man page.
 > >
 > >Because of this behavior,
ovs.socket_util.check_connection_completion() returns success
 > >even if the remote is unreachable or not listening on the port.
 > >
 > >This patch fixes this issue by using poll() to check the
connection status similar to
 > >the C implementation of check_connection_completion().
 > >
 > >A new function 'get_system_poll() is added in ovs/poller.py
which returns the
 > >select.poll() object. If select.poll is monkey patched by
eventlet/gevent, it
 > >gets the original select.poll() and returns it.
 >
 > Is this safe? My concern is that eventlet/gevent monkey patches
 > select.poll() so that the green thread yields properly. Using the
system
 > select.poll() might lead to unexpected blocking.

gevent monkey patches Python to get rid of poll() because poll() might
block and thereby stall Python.  This use of poll() is OK because it
will never block (because it uses a timeout of 0).

 > I read your conversation with Ben on the previous iteration of
this series.
 > It looks like you attempted to use the system select.poll() and
ran into
 > this blocking problem (the pastebin for your patch is now deleted
so I can't
 > see what you actually tried). Why would this approach work
differently?

I don't recall what was in the pastebin, but the use of poll() here
should be sound.  We use the same approach in C and we don't want to
block in C either.


When I tried last time, I modified the class 'SelectPoll' class here
- https://github.com/openvswitch/ovs/blob/master/python/ovs/poller.py#L145
to use system.poll and this blocked the whole process. But as Ben 
suggested earlier
and mentioned above , we use select.poll only when checking the 
connection status
with timeout of 0. So we are fine. I tested this patch with openstack 
neutron and it worked

fine.

Thanks
Numan



Thanks, guys. I'll give the patchset a more thorough review now.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v3 9/9] tests: Disable syslog for ovsdb-tool.

[Ilya Maximets]

This is the only place where ovsdb-tool produces some logs.
Also, it does not support '--timeout' option thus we can't
just add it to the utils list. Let's add syslog option inplace.

Signed-off-by: Ilya Maximets 
Acked-by: Aaron Conole 
---
 tests/ovsdb-cluster.at | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/ovsdb-cluster.at b/tests/ovsdb-cluster.at
index a85a945..4632772 100644
--- a/tests/ovsdb-cluster.at
+++ b/tests/ovsdb-cluster.at
@@ -72,7 +72,7 @@ ovsdb_torture_test () {
 local variant=$3# 'kill' and restart or 'remove' and add
 cp $top_srcdir/ovn/ovn-sb.ovsschema schema
 schema=`ovsdb-tool schema-name schema`
-AT_CHECK([ovsdb-tool '-vPATTERN:console:%c|%p|%m' create-cluster s1.db 
schema unix:s1.raft], [0], [], [dnl
+AT_CHECK([ovsdb-tool '-vPATTERN:console:%c|%p|%m' -vsyslog:off 
create-cluster s1.db schema unix:s1.raft], [0], [], [dnl
 ovsdb|WARN|schema: changed 2 columns in 'OVN_Southbound' database from 
ephemeral to persistent, including 'status' column in 'Connection' table, 
because clusters do not support ephemeral columns
 ])
 
-- 
2.7.4

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

[ovs-dev] [PATCH v3 7/9] tests: Disable syslog for test utils.

[Ilya Maximets]

This disables syslog logging for:
* ovs-testcontroller
* test-netflow
* test-ovsdb
* test-sflow
* test-unixctl (for cases where it's not needed)

Signed-off-by: Ilya Maximets 
Acked-by: Aaron Conole 
---
 tests/bridge.at   |  3 ++-
 tests/ofproto-dpif.at | 32 
 tests/ovsdb-idl.at| 29 +
 tests/ovsdb.at|  2 +-
 tests/vlog.at | 11 +++
 5 files changed, 55 insertions(+), 22 deletions(-)

diff --git a/tests/bridge.at b/tests/bridge.at
index 1c36185..35fed02 100644
--- a/tests/bridge.at
+++ b/tests/bridge.at
@@ -48,7 +48,8 @@ OVS_VSWITCHD_START(
 set bridge br1 datapath-type=dummy other-config:datapath-id=1234 ])
 
 dnl Start ovs-testcontroller
-AT_CHECK([ovs-testcontroller --detach punix:controller --pidfile], [0], 
[ignore])
+AT_CHECK([ovs-testcontroller -vsyslog:off --detach punix:controller --pidfile],
+ [0], [ignore])
 on_exit 'kill `cat ovs-testcontroller.pid`'
 OVS_WAIT_UNTIL([test -e controller])
 
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index f0fca22..dd38d24 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -5991,7 +5991,9 @@ m4_define([CHECK_SFLOW_SAMPLING_PACKET],
   OVS_VSWITCHD_START([set Bridge br0 fail-mode=standalone])
 
   on_exit 'kill `cat test-sflow.pid`'
-  AT_CHECK([ovstest test-sflow --log-file --detach --no-chdir --pidfile 0:$1 > 
sflow.log], [0], [], [ignore])
+  AT_CHECK([ovstest test-sflow -vsyslog:off --log-file \
+   --detach --no-chdir --pidfile 0:$1 > sflow.log],
+   [0], [], [ignore])
   AT_CAPTURE_FILE([sflow.log])
   PARSE_LISTENING_PORT([test-sflow.log], [SFLOW_PORT])
   ovs-appctl time/stop
@@ -6542,7 +6544,9 @@ OVS_VSWITCHD_START([dnl
 other_config:lacp-aggregation-key= ])
 
 on_exit 'kill `cat test-sflow.pid`'
-AT_CHECK([ovstest test-sflow --log-file --detach --no-chdir --pidfile 
0:127.0.0.1 > sflow.log], [0], [], [ignore])
+AT_CHECK([ovstest test-sflow -vsyslog:off --log-file --detach --no-chdir \
+ --pidfile 0:127.0.0.1 > sflow.log],
+ [0], [], [ignore])
 AT_CAPTURE_FILE([sflow.log])
 PARSE_LISTENING_PORT([test-sflow.log], [SFLOW_PORT])
 
@@ -6586,7 +6590,9 @@ AT_XFAIL_IF([test "$IS_WIN32" = "yes"])
 OVS_VSWITCHD_START([set Bridge br0 fail-mode=standalone])
 
 dnl set up sFlow logging
-AT_CHECK([ovstest test-sflow --log-file --detach --no-chdir --pidfile 
0:127.0.0.1 > sflow.log], [0], [], [ignore])
+AT_CHECK([ovstest test-sflow -vsyslog:off --log-file --detach --no-chdir \
+ --pidfile 0:127.0.0.1 > sflow.log],
+ [0], [], [ignore])
 AT_CAPTURE_FILE([sflow.log])
 PARSE_LISTENING_PORT([test-sflow.log], [SFLOW_PORT])
 ovs-appctl time/stop
@@ -6656,7 +6662,9 @@ AT_XFAIL_IF([test "$IS_WIN32" = "yes"])
 OVS_VSWITCHD_START([add-port br0 p0 -- set Interface p0 type=dummy 
ofport_request=1 options:ifindex=1010])
 
 dnl set up sFlow logging
-AT_CHECK([ovstest test-sflow --log-file --detach --no-chdir --pidfile 
0:127.0.0.1 > sflow.log], [0], [], [ignore])
+AT_CHECK([ovstest test-sflow -vsyslog:off --log-file --detach --no-chdir \
+ --pidfile 0:127.0.0.1 > sflow.log],
+ [0], [], [ignore])
 AT_CAPTURE_FILE([sflow.log])
 PARSE_LISTENING_PORT([test-sflow.log], [SFLOW_PORT])
 ovs-appctl time/stop
@@ -6764,7 +6772,9 @@ table=0 dl_src=50:54:00:00:00:0b actions=pop_mpls:0x0800,2
 AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 
 dnl set up sFlow logging
-AT_CHECK([ovstest test-sflow --log-file --detach --no-chdir --pidfile 
0:127.0.0.1 > sflow.log], [0], [], [ignore])
+AT_CHECK([ovstest test-sflow -vsyslog:off --log-file --detach --no-chdir \
+ --pidfile 0:127.0.0.1 > sflow.log],
+ [0], [], [ignore])
 AT_CAPTURE_FILE([sflow.log])
 PARSE_LISTENING_PORT([test-sflow.log], [SFLOW_PORT])
 ovs-appctl time/stop
@@ -6858,7 +6868,9 @@ m4_define([CHECK_NETFLOW_EXPIRATION],
 
   ovs-appctl time/stop
   on_exit 'kill `cat test-netflow.pid`'
-  AT_CHECK([ovstest test-netflow --log-file --detach --no-chdir --pidfile 0:$1 
> netflow.log], [0], [], [ignore])
+  AT_CHECK([ovstest test-netflow -vsyslog:off --log-file --detach --no-chdir \
+ --pidfile 0:$1 > netflow.log],
+   [0], [], [ignore])
   AT_CAPTURE_FILE([netflow.log])
   PARSE_LISTENING_PORT([test-netflow.log], [NETFLOW_PORT])
 
@@ -6905,7 +6917,9 @@ m4_define([CHECK_NETFLOW_ACTIVE_EXPIRATION],
   add_of_ports br0 1 2
 
   on_exit 'kill `cat test-netflow.pid`'
-  AT_CHECK([ovstest test-netflow --log-file --detach --no-chdir --pidfile 0:$1 
> netflow.log], [0], [], [ignore])
+  AT_CHECK([ovstest test-netflow -vsyslog:off --log-file --detach --no-chdir \
+ --pidfile 0:$1 > netflow.log],
+   [0], [], [ignore])
   AT_CAPTURE_FILE([netflow.log])
   PARSE_LISTENING_PORT([test-netflow.log], 

[ovs-dev] [PATCH v3 8/9] tests: Reorder logging args for ovn-sbctl in a subshell.

[Ilya Maximets]

'--log-file' should go after '-v' arguments to avoid unwanted
'opened log file' messages.
Execution is in a subshell and not covered by aliases.

Signed-off-by: Ilya Maximets 
Acked-by: Aaron Conole 
---
 tests/ovsdb-cluster.at | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/ovsdb-cluster.at b/tests/ovsdb-cluster.at
index 01650df..a85a945 100644
--- a/tests/ovsdb-cluster.at
+++ b/tests/ovsdb-cluster.at
@@ -136,7 +136,11 @@ ovsdb|WARN|schema: changed 2 columns in 'OVN_Southbound' 
database from ephemeral
 for i in $(seq 0 $(expr $n1 - 1) ); do
 (for j in $(seq $n2); do
  : > $i-$j.running
- run_as "ovn-sbctl($i-$j)" ovn-sbctl 
"-vPATTERN:console:ovn-sbctl($i-$j)|%D{%H:%M:%S}|%05N|%c|%p|%m" 
--log-file=$i-$j.log -vfile -vsyslog:off -vtimeval:off --timeout=120 
--no-leader-only add SB_Global . external_ids $i-$j=$i-$j
+ run_as "ovn-sbctl($i-$j)" \
+ovn-sbctl 
"-vPATTERN:console:ovn-sbctl($i-$j)|%D{%H:%M:%S}|%05N|%c|%p|%m" \
+-vfile -vsyslog:off -vtimeval:off --log-file=$i-$j.log \
+--timeout=120 --no-leader-only \
+add SB_Global . external_ids $i-$j=$i-$j
  status=$?
  if test $status != 0; then
  echo "$i-$j exited with status $status" > $i-$j:$status
-- 
2.7.4

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

[ovs-dev] [PATCH v3 5/9] tests: Enable only file logging by vlog/set appctl.

[Ilya Maximets]

Logs enabled by 'appctl vlog/set' are commonly only used for
'check_logs' at the end of the test. No need to enable any
other logs except for file.

Patch made automatically by sed replace.

Signed-off-by: Ilya Maximets 
Acked-by: Aaron Conole 
---
 tests/dpif-netdev.at  |  10 ++--
 tests/learn.at|   6 +--
 tests/mpls-xlate.at   |   4 +-
 tests/ofproto-dpif.at | 106 +-
 tests/ofproto.at  |   4 +-
 tests/ovs-ofctl.at|   4 +-
 tests/pmd.at  |  20 
 tests/stp.at  |   6 +--
 tests/system-traffic.at   |   4 +-
 tests/tunnel-push-pop-ipv6.at |   2 +-
 tests/tunnel-push-pop.at  |   2 +-
 11 files changed, 84 insertions(+), 84 deletions(-)

diff --git a/tests/dpif-netdev.at b/tests/dpif-netdev.at
index fff395d..d850350 100644
--- a/tests/dpif-netdev.at
+++ b/tests/dpif-netdev.at
@@ -50,7 +50,7 @@ m4_divert_pop([PREPARE_TESTS])
 AT_SETUP([dpif-netdev - netdev-dummy/receive])
 # Create br0 with interfaces p0
 OVS_VSWITCHD_START([add-port br0 p1 -- set interface p1 type=dummy 
ofport_request=1 -- ])
-AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
+AT_CHECK([ovs-appctl vlog/set dpif:file:dbg dpif_netdev:file:dbg])
 
 AT_CHECK([ovs-ofctl add-flow br0 action=normal])
 ovs-appctl time/stop
@@ -83,7 +83,7 @@ m4_define([DPIF_NETDEV_DUMMY_IFACE],
   add-port br1 p2 -- set interface p2 type=$1 
options:stream=unix:$OVS_RUNDIR/p0.sock ofport_request=2 -- \
   add-port br1 p8 -- set interface p8 ofport_request=8 type=$1 --], [], [],
   [m4_if([$1], [dummy-pmd], [--dummy-numa="0,0,0,0,1,1,1,1"], [])])
-   AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
+   AT_CHECK([ovs-appctl vlog/set dpif:file:dbg dpif_netdev:file:dbg])
 
AT_CHECK([ovs-ofctl add-flow br0 action=normal])
AT_CHECK([ovs-ofctl add-flow br1 action=normal])
@@ -113,7 +113,7 @@ m4_define([DPIF_NETDEV_MISS_FLOW_INSTALL],
  [add-port br0 p1 -- set interface p1 type=$1 
options:pstream=punix:$OVS_RUNDIR/p0.sock
   set bridge br0 datapath-type=dummy other-config:datapath-id=1234 
fail-mode=secure], [], [],
   [m4_if([$1], [dummy-pmd], [--dummy-numa="0,0,0,0,1,1,1,1"], [])])
-   AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
+   AT_CHECK([ovs-appctl vlog/set dpif:file:dbg dpif_netdev:file:dbg])
 
AT_CHECK([ovs-ofctl add-flow br0 action=normal])
AT_CHECK([ovs-appctl netdev-dummy/receive p1 
'in_port(1),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
@@ -155,7 +155,7 @@ m4_define([DPIF_NETDEV_MISS_FLOW_DUMP],
   [m4_if([$1], [dummy-pmd], [--dummy-numa="0,0,0,0,1,1,1,1"], [])])
AT_CHECK([ovs-appctl upcall/disable-ufid], [0], [Datapath dumping tersely 
using UFID disabled
 ], [])
-   AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
+   AT_CHECK([ovs-appctl vlog/set dpif:file:dbg dpif_netdev:file:dbg])
 
AT_CHECK([ovs-ofctl add-flow br0 action=normal])
AT_CHECK([ovs-appctl netdev-dummy/receive p1 
'in_port(1),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.0.0.2,dst=10.0.0.1,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'])
@@ -203,7 +203,7 @@ OVS_VSWITCHD_START(
   fail-mode=secure -- \
add-port br1 p2 -- set interface p2 type=dummy 
options:stream=unix:$OVS_RUNDIR/p0.sock ofport_request=2 -- \
add-port br1 p8 -- set interface p8 ofport_request=8 type=dummy --])
-AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
+AT_CHECK([ovs-appctl vlog/set dpif:file:dbg dpif_netdev:file:dbg])
 
 AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br0 'meter=1 pktps burst stats 
bands=type=drop rate=1 burst_size=1'])
 AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br0 'meter=2 kbps burst stats 
bands=type=drop rate=1 burst_size=2'])
diff --git a/tests/learn.at b/tests/learn.at
index 5f1d6df..ab38d8a 100644
--- a/tests/learn.at
+++ b/tests/learn.at
@@ -665,7 +665,7 @@ AT_CLEANUP
 
 AT_SETUP([learning action - limit])
 OVS_VSWITCHD_START
-AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
+AT_CHECK([ovs-appctl vlog/set dpif:file:dbg dpif_netdev:file:dbg])
 add_of_ports br0 1 2
 AT_DATA([flows.txt], [dnl
 table=0 in_port=1 actions=learn(table=1,dl_dst=dl_src,cookie=0x1,limit=1),2
@@ -704,7 +704,7 @@ AT_CLEANUP
 
 AT_SETUP([learning action - limit result_dst])
 OVS_VSWITCHD_START
-AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
+AT_CHECK([ovs-appctl vlog/set dpif:file:dbg dpif_netdev:file:dbg])
 add_of_ports br0 1
 AT_DATA([flows.txt], [dnl
 table=0 in_port=1 
actions=learn(table=1,dl_dst=dl_src,cookie=0x1,limit=1,result_dst=reg0[[0]]),controller
@@ -737,7 +737,7 @@ AT_CLEANUP
 
 AT_SETUP([learning action - different limits])
 OVS_VSWITCHD_START
-AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
+AT_CHECK([ovs-appctl vlog/set 

[ovs-dev] [PATCH v3 6/9] tests: Drop full logging for ovs-ofctl.

[Ilya Maximets]

'-v' option removed. Found no reason to have fully verbose
output from these ovs-ofctl calls.

Signed-off-by: Ilya Maximets 
Acked-by: Aaron Conole 
---
 tests/ofproto.at | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tests/ofproto.at b/tests/ofproto.at
index d6c6e9a..f57c15b 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -3291,14 +3291,14 @@ check_async () {
 : > expout
 
 # OFPT_PACKET_IN, OFPR_ACTION (controller_id=0)
-ovs-ofctl -v packet-out br0 controller controller 
'0001020304050010203040501234'
+ovs-ofctl packet-out br0 controller controller 
'0001020304050010203040501234'
 if test X"$1" = X"OFPR_ACTION"; then shift;
 echo >>expout "OFPT_PACKET_IN: total_len=14 in_port=CONTROLLER (via 
action) data_len=14 (unbuffered)
 
vlan_tci=0x,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234"
 fi
 
 # OFPT_PACKET_IN, OFPR_NO_MATCH (controller_id=123)
-ovs-ofctl -v packet-out br0 controller 
'controller(reason=no_match,id=123)' '0001020304050010203040501234'
+ovs-ofctl packet-out br0 controller 'controller(reason=no_match,id=123)' 
'0001020304050010203040501234'
 if test X"$1" = X"OFPR_NO_MATCH"; then shift;
 echo >>expout "OFPT_PACKET_IN: total_len=14 in_port=CONTROLLER (via 
no_match) data_len=14 (unbuffered)
 
vlan_tci=0x,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234"
@@ -3394,14 +3394,14 @@ check_async () {
 : > expout
 
 # OFPT_PACKET_IN, OFPR_ACTION (controller_id=0)
-ovs-ofctl -O OpenFlow12 -v packet-out br0 none controller 
'0001020304050010203040501234'
+ovs-ofctl -O OpenFlow12 packet-out br0 none controller 
'0001020304050010203040501234'
 if test X"$1" = X"OFPR_ACTION"; then shift;
 echo >>expout "OFPT_PACKET_IN (OF1.2): total_len=14 in_port=ANY (via 
action) data_len=14 (unbuffered)
 
vlan_tci=0x,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234"
 fi
 
 # OFPT_PACKET_IN, OFPR_NO_MATCH (controller_id=123)
-ovs-ofctl -O OpenFlow12 -v packet-out br0 none 
'controller(reason=no_match,id=123)' '0001020304050010203040501234'
+ovs-ofctl -O OpenFlow12 packet-out br0 none 
'controller(reason=no_match,id=123)' '0001020304050010203040501234'
 if test X"$1" = X"OFPR_NO_MATCH"; then shift;
 echo >>expout "OFPT_PACKET_IN (OF1.2): total_len=14 in_port=ANY (via 
no_match) data_len=14 (unbuffered)
 
vlan_tci=0x,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234"
@@ -3500,7 +3500,7 @@ check_async () {
 
 # OFPT_PACKET_IN, OFPR_ACTION (controller_id=0)
 # OFPR_ACTION_SET is treated as OFPR_ACTION in OpenFlow 1.3
-ovs-ofctl -O OpenFlow13 -v packet-out br0 none controller 
'0001020304050010203040501234'
+ovs-ofctl -O OpenFlow13 packet-out br0 none controller 
'0001020304050010203040501234'
 ovs-ofctl -O OpenFlow13 add-flow br0 'in_port=10 
actions=write_actions(output(CONTROLLER))'
 ovs-appctl netdev-dummy/receive p1 
'in_port(10),eth(src=00:10:20:30:40:50,dst=00:01:02:03:04:05),eth_type(0x1234)'
 if test X"$1" = X"OFPR_ACTION"; then shift;
@@ -3511,7 +3511,7 @@ 
vlan_tci=0x,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234
 fi
 
 # OFPT_PACKET_IN, OFPR_NO_MATCH (controller_id=123)
-ovs-ofctl -O OpenFlow13 -v packet-out br0 none 
'controller(reason=no_match,id=123)' '0001020304050010203040501234'
+ovs-ofctl -O OpenFlow13 packet-out br0 none 
'controller(reason=no_match,id=123)' '0001020304050010203040501234'
 if test X"$1" = X"OFPR_NO_MATCH"; then shift;
 echo >>expout "OFPT_PACKET_IN (OF1.3): total_len=14 in_port=ANY (via 
no_match) data_len=14 (unbuffered)
 
vlan_tci=0x,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234"
@@ -3614,7 +3614,7 @@ check_async () {
 : > expout
 
 # OFPT_PACKET_IN, OFPR_PACKET_OUT (controller_id=0)
-ovs-ofctl -O OpenFlow14 -v packet-out br0 none controller 
'0001020304050010203040501234'
+ovs-ofctl -O OpenFlow14 packet-out br0 none controller 
'0001020304050010203040501234'
 if test X"$1" = X"OFPR_PACKET_OUT"; then shift;
 echo >>expout "OFPT_PACKET_IN (OF1.4): total_len=14 in_port=ANY (via 
packet_out) data_len=14 (unbuffered)
 
vlan_tci=0x,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234"
@@ -3629,7 +3629,7 @@ 
vlan_tci=0x,dl_src=00:10:20:30:40:50,dl_dst=00:01:02:03:04:05,dl_type=0x1234
 fi
 
 # OFPT_PACKET_IN, OFPR_NO_MATCH (controller_id=123)
-ovs-ofctl -O OpenFlow14 -v packet-out br0 none 
'controller(reason=no_match,id=123)' '0001020304050010203040501234'
+ovs-ofctl -O OpenFlow14 packet-out br0 none 
'controller(reason=no_match,id=123)' '0001020304050010203040501234'
 if test X"$1" = X"OFPR_NO_MATCH"; then shift;
 echo >>expout "OFPT_PACKET_IN (OF1.4): total_len=14 in_port=ANY (via 
no_match) data_len=14 (unbuffered)
 

[ovs-dev] [PATCH v3 4/9] tests: Disable syslog for daemons.

[Ilya Maximets]

We can not just make an alias for daemons because many of them
has logging options in their command lines.
Let's handle them one by one. Additionally, it's a good chance
to wrap all the very long lines for better readability.

Signed-off-by: Ilya Maximets 
Acked-by: Aaron Conole 
---
 tests/ofproto-macros.at  |  10 +-
 tests/ovn-controller-vtep.at |  35 +--
 tests/ovn-nbctl.at   |   4 +-
 tests/ovn-sbctl.at   |  19 +++-
 tests/ovs-vsctl.at   |  21 +++-
 tests/ovs-vswitchd.at|  17 +++-
 tests/ovsdb-idl.at   |   8 +-
 tests/ovsdb-lock.at  |   6 +-
 tests/ovsdb-monitor.at   |  23 -
 tests/ovsdb-rbac.at  |   3 +-
 tests/ovsdb-server.at| 227 ---
 tests/vlog.at|   4 +-
 tests/vtep-ctl.at|   4 +-
 13 files changed, 291 insertions(+), 90 deletions(-)

diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index 287d16c..2649f13 100644
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -54,7 +54,7 @@ m4_define([PARSE_LISTENING_PORT],
 [OVS_WAIT_UNTIL([$2=`sed -n 's/.*0:.*: listening on port 
\([[0-9]]*\)$/\1/p' "$1"` && test X != X"[$]$2"])])
 
 start_daemon () {
-"$@" -vconsole:off --detach --no-chdir --pidfile --log-file
+"$@" -vconsole:off -vsyslog:off --detach --no-chdir --pidfile --log-file
 pidfile="$OVS_RUNDIR"/$1.pid
 on_exit "test -e \"$pidfile\" && kill \`cat \"$pidfile\"\`"
 }
@@ -335,7 +335,9 @@ m4_define([_OVS_VSWITCHD_START],
AT_CHECK([ovsdb-tool create conf.db 
$abs_top_srcdir/vswitchd/vswitch.ovsschema])
 
dnl Start ovsdb-server.
-   AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --log-file 
--remote=punix:$OVS_RUNDIR/db.sock], [0], [], [stderr])
+   AT_CHECK([ovsdb-server -vsyslog:off --detach --no-chdir --pidfile \
+  --log-file --remote=punix:$OVS_RUNDIR/db.sock],
+[0], [], [stderr])
on_exit "kill `cat ovsdb-server.pid`"
AT_CHECK([[sed < stderr '
 /vlog|INFO|opened log file/d
@@ -346,7 +348,9 @@ m4_define([_OVS_VSWITCHD_START],
AT_CHECK([ovs-vsctl --no-wait init $2])
 
dnl Start ovs-vswitchd.
-   AT_CHECK([ovs-vswitchd $1 --detach --no-chdir --pidfile --log-file -vvconn 
-vofproto_dpif -vunixctl], [0], [], [stderr])
+   AT_CHECK([ovs-vswitchd $1 -vvconn -vofproto_dpif -vunixctl -vsyslog:off \
+ --detach --no-chdir --pidfile --log-file],
+[0], [], [stderr])
AT_CAPTURE_FILE([ovs-vswitchd.log])
on_exit "kill_ovs_vswitchd `cat ovs-vswitchd.pid`"
AT_CHECK([[sed < stderr '
diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at
index 416e954..e4f93a8 100644
--- a/tests/ovn-controller-vtep.at
+++ b/tests/ovn-controller-vtep.at
@@ -24,9 +24,21 @@ m4_define([OVN_CONTROLLER_VTEP_START],
done
 
dnl Start ovsdb-server.
-   AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --log-file 
--remote=punix:$OVS_RUNDIR/db.sock vswitchd.db vtep.db], [0], [], [stderr])
-   AT_CHECK([ovsdb-server --detach --no-chdir --pidfile=ovsdb-nb-server.pid 
--log-file=ovsdb-nb-server.log --remote=punix:$OVS_RUNDIR/ovnnb_db.sock 
ovn-nb.db], [0], [], [stderr])
-   AT_CHECK([ovsdb-server --detach --no-chdir --pidfile=ovsdb-sb-server.pid 
--log-file=ovsdb-sb-server.log --remote=punix:$OVS_RUNDIR/ovnsb_db.sock 
ovn-sb.db ovn-sb.db], [0], [], [stderr])
+   AT_CHECK([ovsdb-server -vsyslog:off --detach --no-chdir \
+  --pidfile --log-file \
+  --remote=punix:$OVS_RUNDIR/db.sock vswitchd.db 
vtep.db],
+[0], [], [stderr])
+   AT_CHECK([ovsdb-server -vsyslog:off --detach --no-chdir \
+  --pidfile=ovsdb-nb-server.pid\
+  --log-file=ovsdb-nb-server.log   \
+  --remote=punix:$OVS_RUNDIR/ovnnb_db.sock ovn-nb.db],
+[0], [], [stderr])
+   AT_CHECK([ovsdb-server -vsyslog:off --detach --no-chdir \
+  --pidfile=ovsdb-sb-server.pid\
+  --log-file=ovsdb-sb-server.log   \
+  --remote=punix:$OVS_RUNDIR/ovnsb_db.sock \
+  ovn-sb.db ovn-sb.db],
+[0], [], [stderr])
on_exit "kill `cat ovsdb-server.pid` `cat ovsdb-nb-server.pid` `cat 
ovsdb-sb-server.pid`"
AT_CHECK([[sed < stderr '
 /vlog|INFO|opened log file/d
@@ -34,7 +46,10 @@ m4_define([OVN_CONTROLLER_VTEP_START],
AT_CAPTURE_FILE([ovsdb-server.log])
 
dnl Start ovs-vswitchd.
-   AT_CHECK([ovs-vswitchd --enable-dummy=system --disable-system --detach 
--no-chdir --pidfile --log-file -vvconn -vofproto_dpif], [0], [], [stderr])
+   AT_CHECK([ovs-vswitchd -vvconn -vofproto_dpif -vsyslog:off   \
+  --enable-dummy=system --disable-system\
+  --detach --no-chdir --pidfile 

[ovs-dev] [PATCH v3 3/9] tests: Disable syslog by default for control utils.

[Ilya Maximets]

syslog messages from unit tests are not useful and only litter the
system logs on build / test machines.

Signed-off-by: Ilya Maximets 
Acked-by: Aaron Conole 
---
 tests/ofproto-macros.at | 2 +-
 tests/ovs-macros.at | 6 --
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index df8e7c4..287d16c 100644
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -112,7 +112,7 @@ as() {
  cmd=$1; shift;
  for util in $OVS_UTILS_LIST; do
  if test "X$util" = "X$cmd"; then
- $cmd --timeout=$OVS_TIMEOUT "$@"
+ $cmd $OVS_UTILS_DEFAULT_OPTIONS "$@"
  exit "$?"
  fi
  done
diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
index e55d6d1..e654259 100644
--- a/tests/ovs-macros.at
+++ b/tests/ovs-macros.at
@@ -114,7 +114,7 @@ if test "$IS_WIN32" = "yes"; then
 }
 fi
 
-# Try to add a default timeout for the following control utilities:
+# Try to add a default options for the following control utilities:
 # - ovs-vsctl
 # - ovs-ofctl
 # - ovs-appctl
@@ -123,11 +123,13 @@ fi
 # - vtep-ctl
 # Set default timeout for 30 seconds.
 # This should be sufficient on all platforms.
+# Disable unwanted logging to syslog.
 OVS_TIMEOUT=30
+OVS_UTILS_DEFAULT_OPTIONS="-vsyslog:off --timeout=$OVS_TIMEOUT"
 OVS_UTILS_LIST="ovs-vsctl ovs-ofctl ovs-appctl ovn-sbctl ovn-nbctl
 vtep-ctl ovsdb-client"
 for util in $OVS_UTILS_LIST; do
-alias $util="$util --timeout=$OVS_TIMEOUT" >/dev/null 2>&1
+alias $util="$util $OVS_UTILS_DEFAULT_OPTIONS" >/dev/null 2>&1
 done
 
 # parent_pid PID
-- 
2.7.4

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

[ovs-dev] [PATCH v3 2/9] tests: Set default timeout for utils in subshell.

[Ilya Maximets]

Aliases are not inheritable. To add a default options for utils
executed in subshell we may try to catch them here and append
options explicitly.

There are still few cases with utils invocation in subshell inside
the functions that we can not track this way, but they are not
very frequent.

Signed-off-by: Ilya Maximets 
---
 tests/ofproto-macros.at | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index 2a56ae6..df8e7c4 100644
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -107,7 +107,17 @@ sim_add () {
 # there.
 as() {
 if test "X$2" != X; then
-(ovs_setenv $1; shift; "$@")
+(
+ ovs_setenv $1; shift;
+ cmd=$1; shift;
+ for util in $OVS_UTILS_LIST; do
+ if test "X$util" = "X$cmd"; then
+ $cmd --timeout=$OVS_TIMEOUT "$@"
+ exit "$?"
+ fi
+ done
+ $cmd "$@"
+)
 else
 ovs_setenv $1
 fi
-- 
2.7.4

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

[ovs-dev] [PATCH v3 1/9] tests: Simplify the setting of aliases.

[Ilya Maximets]

There is no need to create a separate function for each alias.
This will simplify adding new default options and utils.

Signed-off-by: Ilya Maximets 
Acked-by: Aaron Conole 
---
 tests/ovs-macros.at | 35 +--
 1 file changed, 5 insertions(+), 30 deletions(-)

diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
index 677eea7..e55d6d1 100644
--- a/tests/ovs-macros.at
+++ b/tests/ovs-macros.at
@@ -124,36 +124,11 @@ fi
 # Set default timeout for 30 seconds.
 # This should be sufficient on all platforms.
 OVS_TIMEOUT=30
-alias ovs-vsctl='OVS_VSCTL_TIMEOUT' >/dev/null 2>&1
-if [ $? -eq 0 ]; then
-OVS_VSCTL_TIMEOUT () {
-command ovs-vsctl --timeout=$OVS_TIMEOUT "$@"
-}
-alias ovs-ofctl='OVS_OFCTL_TIMEOUT'
-alias ovs-appctl='OVS_APPCTL_TIMEOUT'
-alias ovn-sbctl='OVS_SBCTL_TIMEOUT'
-alias ovn-nbctl='OVN_NBCTL_TIMEOUT'
-alias vtep-ctl='VTEP_CTL_TIMEOUT'
-alias ovsdb-client='OVSDB_CLIENT_TIMEOUT'
-OVS_OFCTL_TIMEOUT () {
-command ovs-ofctl --timeout=$OVS_TIMEOUT "$@"
-}
-OVS_APPCTL_TIMEOUT () {
-command ovs-appctl --timeout=$OVS_TIMEOUT "$@"
-}
-OVS_SBCTL_TIMEOUT () {
-command ovn-sbctl --timeout=$OVS_TIMEOUT "$@"
-}
-OVN_NBCTL_TIMEOUT () {
-command ovn-nbctl --timeout=$OVS_TIMEOUT "$@"
-}
-VTEP_CTL_TIMEOUT () {
-command vtep-ctl --timeout=$OVS_TIMEOUT "$@"
-}
-OVSDB_CLIENT_TIMEOUT () {
-command ovsdb-client --timeout=$OVS_TIMEOUT "$@"
-}
-fi
+OVS_UTILS_LIST="ovs-vsctl ovs-ofctl ovs-appctl ovn-sbctl ovn-nbctl
+vtep-ctl ovsdb-client"
+for util in $OVS_UTILS_LIST; do
+alias $util="$util --timeout=$OVS_TIMEOUT" >/dev/null 2>&1
+done
 
 # parent_pid PID
 #
-- 
2.7.4

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

[ovs-dev] [PATCH v3 0/9] tests: Clean up syslog.

[Ilya Maximets]

Each run of the testsuite produces millions lines in a system
log. This is completely unnecessary and makes it difficult to
use system logs on test / build servers.

This series is aimed to disable most of the syslog messages.
There are still few logs that requires significant changes in
tests or code to disable. They will be removed separately if
needed.

Some testing results:
  OS : RHEL 7.5
  CPU: Xeon E5 v4 2.6GHz
  Cmd: make check TESTSUITEFLAGS='-j20'

  Without patches:
  * 3.350.097 Lines of logs in journalctl
  * Execution time: 11 minutes
  * journald eats 100% of one cpu core.

  With patch-set applied:
  * 226 Lines of logs in journalctl
  * Execution time: 2.5 minutes

So, in addition to clean logs, this patch-set significantly
speeds up the testsuite execution in parralel builds (more
than 4 times! in my case).

Side effects:
  * default timeout applied to control utils in a subshell.
  * tests refactored to be more readable.
  * testsuite execution speed up.


Version 3:
  * Replaced bash extention '==' with '=' in patch #2. [Timothy]
  * Rebased on current master.
  * Added ACK from Aaron to all patches except #2, which was
a bit modified.

Version 2:
  * Fixed accidentially missed '--timeout' in patches 1 and 2. [Aaron]


Ilya Maximets (9):
  tests: Simplify the setting of aliases.
  tests: Set default timeout for utils in subshell.
  tests: Disable syslog by default for control utils.
  tests: Disable syslog for daemons.
  tests: Enable only file logging by vlog/set appctl.
  tests: Drop full logging for ovs-ofctl.
  tests: Disable syslog for test utils.
  tests: Reorder logging args for ovn-sbctl in a subshell.
  tests: Disable syslog for ovsdb-tool.

 tests/bridge.at   |   3 +-
 tests/dpif-netdev.at  |  10 +-
 tests/learn.at|   6 +-
 tests/mpls-xlate.at   |   4 +-
 tests/ofproto-dpif.at | 138 +
 tests/ofproto-macros.at   |  22 +++-
 tests/ofproto.at  |  20 ++--
 tests/ovn-controller-vtep.at  |  35 +--
 tests/ovn-nbctl.at|   4 +-
 tests/ovn-sbctl.at|  19 +++-
 tests/ovs-macros.at   |  39 ++--
 tests/ovs-ofctl.at|   4 +-
 tests/ovs-vsctl.at|  21 +++-
 tests/ovs-vswitchd.at |  17 +++-
 tests/ovsdb-cluster.at|   8 +-
 tests/ovsdb-idl.at|  37 +--
 tests/ovsdb-lock.at   |   6 +-
 tests/ovsdb-monitor.at|  23 -
 tests/ovsdb-rbac.at   |   3 +-
 tests/ovsdb-server.at | 227 --
 tests/ovsdb.at|   2 +-
 tests/pmd.at  |  20 ++--
 tests/stp.at  |   6 +-
 tests/system-traffic.at   |   4 +-
 tests/tunnel-push-pop-ipv6.at |   2 +-
 tests/tunnel-push-pop.at  |   2 +-
 tests/vlog.at |  15 +--
 tests/vtep-ctl.at |   4 +-
 28 files changed, 463 insertions(+), 238 deletions(-)

-- 
2.7.4

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

Re: [ovs-dev] [PATCH v2 0/9] tests: Clean up syslog.

[Ilya Maximets]

On 07.08.2018 00:12, Ben Pfaff wrote:
> On Mon, Aug 06, 2018 at 06:18:19PM +0300, Ilya Maximets wrote:
>> On 04.08.2018 03:17, Ben Pfaff wrote:
>>> On Wed, Aug 01, 2018 at 05:00:09PM +0300, Ilya Maximets wrote:
 Each run of the testsuite produces millions lines in a system
 log. This is completely unnecessary and makes it difficult to
 use system logs on test / build servers.

 This series is aimed to disable most of the syslog messages.
 There are still few logs that requires significant changes in
 tests or code to disable. They will be removed separately if
 needed.
>>>
>>> This series seems technically high-quality.  Thank you.
>>>
>>> I find myself wondering whether we should just introduce an environment
>>> variable to allowing configuring logging defaults.  Then we would cover
>>> literally everything with a single line in atlocal.in.
>>>
>>> Any opinions?
>>
>> I thought about this. And implementation will have few issues:
>>
>> 1. Many daemons already has logging related options in cmdlines.
>>This means that it should not be "OVS_DEFAULT_LOGGING_OPTIONS",
>>but "OVS_FORCE_LOGGING_OPTIONS". i.e. the value in the environment
>>should have highest priority.
>> 2. Leads from the 1st. I'm not sure that we should expose variables
>>like this for the end users, because there will be no way to change
>>the logging level for the targets forced by the environment.
>>So, it should be test only variable like
>>"TEST_OVS_FORCE_DEFAULT_OPTIONS". But we still can't be sure that
>>no one will use it in production.
>> 3. Some tests (like some from vlog.at) requires ability to change the
>>log level for syslog, for example. We'll need to unset the variable
>>for these tests or change the tests themselves.
>> 4. Modifications for all the utils/daemons and/or the vlog subsystem
>>are required to implement this environment variable which is
>>intended for test purposes only
>>
>> I'm not sure which solution is better.
>> What do you think?
> 
> Most of these points seem to follow from #1, but I don't understand #1.
> Why shouldn't the environment variable be just a default that can be
> overridden by command-line arguments?  That is usually what environment
> settings do.

Yes, you're right that having environment variable which could be overridden
by command-line arguments is a common and usual thing. But tests contains a
lot of daemons'/control utils' invocations like this:
ovs-vswitchd --enable-dummy=system --disable-system --detach \
 --no-chdir --pidfile --log-file \
 -vvconn -vofproto_dpif
or
ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg

In above cases environment variable will be overridden and we will have
debug logs in syslog. So, we'll have to apply at least a half of current
patch set to disable syslog in these cases. See patches 4, 5, 6 and 8.

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

[ovs-dev] [PATCH] releases: Add 2.10 entry for supported DPDK versions.

[Ian Stokes]

This commit adds an entry for OVS 2.10 and the supported DPDK version in
releases.rst.

Signed-off-by: Ian Stokes 
---
 Documentation/faq/releases.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst
index 0f3bf0c..50ca5f6 100644
--- a/Documentation/faq/releases.rst
+++ b/Documentation/faq/releases.rst
@@ -167,6 +167,7 @@ Q: What DPDK version does each Open vSwitch release work 
with?
 2.7.x16.11.7
 2.8.x17.05.2
 2.9.x17.11.3
+2.10.x   17.11.3
  ===
 
 Q: Are all the DPDK releases that OVS versions work with maintained?
-- 
2.7.5

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

Re: [ovs-dev] [PATCH] netdev-dpdk: Use hex for PCI vendor ID.

[Ian Stokes]

On 8/3/2018 8:03 PM, Kevin Traynor wrote:

Match the prefix and formatting.

Fixes: 8a9562d21a40 ("dpif-netdev: Add DPDK netdev.")
Cc: pshe...@ovn.org

Signed-off-by: Kevin Traynor 


Thanks Kevin, I'll add this to the pull request, I think it can be 
backported from 2.10 -> 2.6 also.


Ian


---
  lib/netdev-dpdk.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 9bf2185..90a6165 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -3044,5 +3044,5 @@ netdev_dpdk_get_status(const struct netdev *netdev, 
struct smap *args)
  
  if (dev_info.pci_dev) {

-smap_add_format(args, "pci-vendor_id", "0x%u",
+smap_add_format(args, "pci-vendor_id", "0x%x",
  dev_info.pci_dev->id.vendor_id);
  smap_add_format(args, "pci-device_id", "0x%x",



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

Re: [ovs-dev] [PATCH v7 07/13] dp-packet: Handle multi-seg mubfs in shift() func.

[Ian Stokes]

On 8/7/2018 6:13 PM, Darrell Ball wrote:



On Tue, Aug 7, 2018 at 5:08 AM, Stokes, Ian > wrote:


> In its current implementation dp_packet_shift() is also unaware of multi-
> seg mbufs (that holds data in memory non-contiguously) and assumes that
> data exists contiguously in memory, memmove'ing data to perform the shift.
> 
> To add support for multi-seg mbuds a new set of functions was introduced,

> dp_packet_mbuf_shift() and dp_packet_mbuf_write(). These functions are
> used by dp_packet_shift(), when handling multi-seg mbufs, to shift and
> write data within a chain of mbufs.
> 


Hi Tiago,

After applying this patch I see the following error during compilation.

lib/conntrack.c: In function 'handle_ftp_ctl':
lib/conntrack.c:3154:11: error: 'addr_offset_from_ftp_data_start'
may be used uninitialized in this function [-Werror=maybe-uninitialized]
      char *pkt_addr_str = ftp_data_start +
addr_offset_from_ftp_data_start;
            ^~~~
lib/conntrack.c:3173:12: note: 'addr_offset_from_ftp_data_start' was
declared here
      size_t addr_offset_from_ftp_data_start;

It can be fixed with the following incremental.

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 974f985..7cd1fc9 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -3170,7 +3170,7 @@ handle_ftp_ctl(struct conntrack *ct, const
struct conn_lookup_ctx *ctx,
      struct ip_header *l3_hdr = dp_packet_l3(pkt);
      ovs_be32 v4_addr_rep = 0;
      struct ct_addr v6_addr_rep;
-    size_t addr_offset_from_ftp_data_start;
+    size_t addr_offset_from_ftp_data_start = 0;
      size_t addr_size = 0;
      char *ftp_data_start;
      bool do_seq_skew_adj = true;

If there are no objections I can roll this into the patch upon
application to dpdk merge.

Ian



hmm, I test with 4 major versions of GCC (4-7) with different flags, 
including O3 and I don't see these errors.

I use 6.4 for the '6' major version

What flags are you using ?


I've compiled with and without the following flags -g -Ofast -march=native.

In both cases the warning still occurs.

I'm using gcc (GCC) 6.3.1. Tiago also spotted the same issue occurring 
on GCC 5.4 so it doesn't seem isolated to 6.3.1.





I am building some versions from source; are you doing the same /

Is it possible that your GCC 6.3.1 was not built correctly ?


I'm not building gcc from source, I'm using Fedora 24 and the GCC 
installed from the fedora repo.


Ian




> Signed-off-by: Tiago Lam mailto:tiago@intel.com>>
> Acked-by: Eelco Chaudron mailto:echau...@redhat.com>>
> ---
>  lib/dp-packet.c | 97
> +
>  lib/dp-packet.h | 10 ++
>  2 files changed, 107 insertions(+)
> 
> diff --git a/lib/dp-packet.c b/lib/dp-packet.c index 2aaeaae..d6e19eb

> 100644
> --- a/lib/dp-packet.c
> +++ b/lib/dp-packet.c
> @@ -294,6 +294,97 @@ dp_packet_prealloc_headroom(struct dp_packet *b,
> size_t size)
>      }
>  }
> 
> +#ifdef DPDK_NETDEV

> +/* Write len data bytes in a mbuf at specified offset.
> + *
> + * 'mbuf', pointer to the destination mbuf where 'ofs' is, and the mbuf
 > +where
> + * the data will first be written.
> + * 'ofs', the offset within the provided 'mbuf' where 'data' is to be
> written.
> + * 'len', the size of the to be written 'data'.
> + * 'data', pointer to the to be written bytes.
> + *
> + * XXX: This function is the counterpart of the `rte_pktmbuf_read()`
 > +function
 > + * available with DPDK, in the rte_mbuf.h */ void
> +dp_packet_mbuf_write(struct rte_mbuf *mbuf, int16_t ofs, uint32_t len,
> +                     const void *data)
> +{
> +    char *dst_addr;
> +    uint16_t data_len;
> +    int len_copy;
> +    while (mbuf) {
> +        if (len == 0) {
> +            break;
> +        }
> +
> +        dst_addr = rte_pktmbuf_mtod_offset(mbuf, char *, ofs);
> +        data_len = MBUF_BUF_END(mbuf->buf_addr, mbuf->buf_len) -
 > + dst_addr;
> +
> +        len_copy = MIN(len, data_len);
> +        /* We don't know if 'data' is the result of a rte_pktmbuf_read()
> call,
> +         * in which case we may end up writing to the same region of
> memory we
> +         * are reading from and overlapping. Hence the use of memmove()
> here */
> +        memmove(dst_addr, data, len_copy);
> +
> +        data = ((char *) data) + len_copy;
> +        len -= len_copy;
> +        ofs = 0;
> +
> +        mbuf = mbuf->next;
> +    }
> +}
> +
> +static void
> +dp_packet_mbuf_shift_(struct rte_mbuf *dbuf, int16_t dst_ofs,
> +                      const struct rte_mbuf *sbuf, uint16_t src_ofs,

Re: [ovs-dev] infiniband (IPoIB) support

[Vasiliy Tolstov]

ср, 8 авг. 2018 г. в 1:18, Ben Pfaff :
>
> I don't know how many people on this list know anything about IPoIB.  I
> know that I don't.  You might not be getting an answer simply because
> it's such a specialty topic.  Maybe there is a place where people talk
> about IPoIB software; maybe they would know something.
>

Now my question now about IPoIB, as i read - attach IPoIB device not
possible. Also linux does not allow to create vlan on IPoIB (because
IPoIB have pkeys analog to vlans).
My question is - how can i do connectivity if vm attached to br-int
have external ip address for example 1.2.3.100 (provided by ovn dhcp),
and gateway server that have uplink to internet have ip 1.2.3.254.
As i understand logical router needs to be created to route traffic
from diffrent networks...

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v6] dpif-netdev: Avoid reordering of packets in a batch with same megaflow

[Ian Stokes]

On 7/27/2018 7:26 PM, Vishal Deep Ajmera wrote:

OVS reads packets in batches from a given port and packets in the
batch are subjected to potentially 3 levels of lookups to identify
the datapath megaflow entry (or flow) associated with the packet.
Each megaflow entry has a dedicated buffer in which packets that match
the flow classification criteria are collected. This buffer helps OVS
perform batch processing for all packets associated with a given flow.

Each packet in the received batch is first subjected to lookup in the
Exact Match Cache (EMC). Each EMC entry will point to a flow. If the
EMC lookup is successful, the packet is moved from the rx batch to the
per-flow buffer.

Packets that did not match any EMC entry are rearranged in the rx batch
at the beginning and are now subjected to a lookup in the megaflow cache.
Packets that match a megaflow cache entry are *appended* to the per-flow
buffer.

Packets that do not match any megaflow entry are subjected to slow-path
processing through the upcall mechanism. This cannot change the order of
packets as by definition upcall processing is only done for packets
without matching megaflow entry.

The EMC entry match fields encompass all potentially significant header
fields, typically more than specified in the associated flow's match
criteria. Hence, multiple EMC entries can point to the same flow. Given
that per-flow batching happens at each lookup stage, packets belonging
to the same megaflow can get re-ordered because some packets match EMC
entries while others do not.

The following example can illustrate the issue better. Consider
following batch of packets (labelled P1 to P8) associated with a single
TCP connection and associated with a single flow. Let us assume that
packets with just the ACK bit set in TCP flags have been received in a
prior batch also and a corresponding EMC entry exists.

1. P1 (TCP Flag: ACK)
2. P2 (TCP Flag: ACK)
3. P3 (TCP Flag: ACK)
4. P4 (TCP Flag: ACK, PSH)
5. P5 (TCP Flag: ACK)
6. P6 (TCP Flag: ACK)
7. P7 (TCP Flag: ACK)
8. P8 (TCP Flag: ACK)

The megaflow classification criteria does not include TCP flags while
the EMC match criteria does. Thus, all packets other than P4 match
the existing EMC entry and are moved to the per-flow packet batch.
Subsequently, packet P4 is moved to the same per-flow packet batch as
a result of the megaflow lookup. Though the packets have all been
correctly classified as being associated with the same flow, the
packet order has not been preserved because of the per-flow batching
performed during the EMC lookup stage. This packet re-ordering has
performance implications for TCP applications.

This patch preserves the packet ordering by performing the per-flow
batching after both the EMC and megaflow lookups are complete. As an
optimization, packets are flow-batched in emc processing till any
packet in the batch has an EMC miss.

A new flow map is maintained to keep the original order of packet
along with flow information. Post fastpath processing, packets from
flow map are *appended* to per-flow buffer.



Thanks for the V6 Vishal, looking at this today myself.

Ilya, has the v6 addressed your concerns from the v5?

Thanks
Ian


Signed-off-by: Vishal Deep Ajmera 
Co-authored-by: Venkatesan Pradeep 
Signed-off-by: Venkatesan Pradeep 
---
  lib/dpif-netdev.c | 125 +-
  1 file changed, 106 insertions(+), 19 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 13a20f0..3ea25e2 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -244,6 +244,13 @@ struct dpcls_rule {
  /* 'flow' must be the last field, additional space is allocated here. */
  };
  
+/* Data structure to keep packet order till fastpath processing. */

+struct dp_packet_flow_map {
+struct dp_packet *packet;
+struct dp_netdev_flow *flow;
+uint16_t tcp_flags;
+};
+
  static void dpcls_init(struct dpcls *);
  static void dpcls_destroy(struct dpcls *);
  static void dpcls_sort_subtable_vector(struct dpcls *);
@@ -5774,6 +5781,19 @@ dp_netdev_queue_batches(struct dp_packet *pkt,
  packet_batch_per_flow_update(batch, pkt, tcp_flags);
  }
  
+static inline void

+packet_enqueue_to_flow_map(struct dp_packet *packet,
+   struct dp_netdev_flow *flow,
+   uint16_t tcp_flags,
+   struct dp_packet_flow_map *flow_map,
+   size_t index)
+{
+struct dp_packet_flow_map *map = _map[index];
+map->flow = flow;
+map->packet = packet;
+map->tcp_flags = tcp_flags;
+}
+
  /* SMC lookup function for a batch of packets.
   * By doing batching SMC lookup, we can use prefetch
   * to hide memory access latency.
@@ -5783,8 +5803,9 @@ smc_lookup_batch(struct dp_netdev_pmd_thread *pmd,
  struct netdev_flow_key *keys,
  struct netdev_flow_key **missed_keys,
  struct dp_packet_batch *packets_,
-struct 

[ovs-dev] [PATCH v2] rhel: Use correct user in the logrotate configuration file

[Markos Chandras]

The /var/log/openvswitch directory is owned by the openvswitch user but
logrotate could be running as root or as another user. As a result of
which, rpmlint prints the following warning when building the spec file
on SUSE Linux Enterprise:

openvswitch.x86_64: W: suse-logrotate-user-writable-log-dir 
/var/log/openvswitch openvswitch:openvswitch 0750
The log directory is writable by unprivileged users. Please fix the
permissions so only root can write there or add the 'su' option
to your logrotate config

In order to fix that, we should run the logrotate script as the same
user which runs the various Open vSwitch daemons. If this is a new
installation, then this user is the 'openvswitch' one, but if we are
upgrading from an older release, then the user is normally 'root'.
As such, we set the initial user to 'root' and we fix this up in the
%post scriptlet.

Cc: Aaron Conole 
Cc: Timothy Redaelli 
Signed-off-by: Markos Chandras 
---
 rhel/etc_logrotate.d_openvswitch | 1 +
 rhel/openvswitch-fedora.spec.in  | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/rhel/etc_logrotate.d_openvswitch b/rhel/etc_logrotate.d_openvswitch
index ed7d733c9..f4302ffbc 100644
--- a/rhel/etc_logrotate.d_openvswitch
+++ b/rhel/etc_logrotate.d_openvswitch
@@ -6,6 +6,7 @@
 # without warranty of any kind.
 
 /var/log/openvswitch/*.log {
+su root root
 daily
 compress
 sharedscripts
diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index 9f8664e95..c2d3200e1 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -405,6 +405,7 @@ exit 0
 %post
 if [ $1 -eq 1 ]; then
 sed -i 's:^#OVS_USER_ID=:OVS_USER_ID=:' /etc/sysconfig/openvswitch
+sed -i 's:\(.*su\).*:\1 openvswitch openvswitch:' 
%{_sysconfdir}/logrotate.d/openvswitch
 
 %if %{with dpdk}
 sed -i \
@@ -414,6 +415,7 @@ if [ $1 -eq 1 ]; then
 
 # In the case of upgrade, this is not needed.
 chown -R openvswitch:openvswitch /etc/openvswitch
+chown -R openvswitch:openvswitch /var/log/openvswitch
 fi
 
 %if 0%{?systemd_post:1}
@@ -601,7 +603,7 @@ fi
 %endif
 %doc NOTICE README.rst NEWS rhel/README.RHEL.rst
 /var/lib/openvswitch
-%attr(750,openvswitch,openvswitch) /var/log/openvswitch
+%attr(750,root,root) /var/log/openvswitch
 %ghost %attr(755,root,root) %{_rundir}/openvswitch
 
 %files ovn-docker
-- 
2.16.4

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

Re: [ovs-dev] [PATCH 2/2] ofp-port: Drop of useless indirection in ofputil_pull_ofp14_port_stats().

[Ian Stokes]

On 7/27/2018 7:14 PM, Ben Pfaff wrote:

Signed-off-by: Ben Pfaff 


Seems straight forward enough. LGTM. I can add this to this weeks pull 
request.


Thanks
Ian

---
  lib/ofp-port.c | 30 ++
  1 file changed, 2 insertions(+), 28 deletions(-)

diff --git a/lib/ofp-port.c b/lib/ofp-port.c
index 1d864c3a3dc7..2c812f8ecfa3 100644
--- a/lib/ofp-port.c
+++ b/lib/ofp-port.c
@@ -1636,28 +1636,6 @@ parse_intel_port_custom_property(struct ofpbuf *payload,
  return 0;
  }
  
-static enum ofperr

-parse_intel_port_stats_property(struct ofpbuf *payload,
-uint32_t exp_type,
-struct ofputil_port_stats *ops)
-{
-enum ofperr error;
-
-switch (exp_type) {
-case INTEL_PORT_STATS_RFC2819:
-error = parse_intel_port_stats_rfc2819_property(payload, ops);
-break;
-case INTEL_PORT_STATS_CUSTOM:
-error = parse_intel_port_custom_property(payload, ops);
-break;
-default:
-error = OFPERR_OFPBPC_BAD_EXP_TYPE;
-break;
-}
-
-return error;
-}
-
  static enum ofperr
  ofputil_pull_ofp14_port_stats(struct ofputil_port_stats *ops,
struct ofpbuf *msg)
@@ -1705,14 +1683,10 @@ ofputil_pull_ofp14_port_stats(struct ofputil_port_stats 
*ops,
  error = parse_ofp14_port_stats_ethernet_property(, ops);
  break;
  case OFPPROP_EXP(INTEL_VENDOR_ID, INTEL_PORT_STATS_RFC2819):
-error = parse_intel_port_stats_property(,
-INTEL_PORT_STATS_RFC2819,
-ops);
+error = parse_intel_port_stats_rfc2819_property(, ops);
  break;
  case OFPPROP_EXP(INTEL_VENDOR_ID, INTEL_PORT_STATS_CUSTOM):
-error = parse_intel_port_stats_property(,
-INTEL_PORT_STATS_CUSTOM,
-ops);
+error = parse_intel_port_custom_property(, ops);
  break;
  default:
  error = OFPPROP_UNKNOWN(true, "port stats", type);



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

Re: [ovs-dev] [PATCH 1/2] ofp-port: Fix buffer overread parsing Intel custom statistics.

[Ian Stokes]

On 8/7/2018 1:00 AM, Ben Pfaff wrote:

On Fri, Jul 27, 2018 at 11:14:43AM -0700, Ben Pfaff wrote:

CC: Michal Weglicki 
Fixes: 971f4b394c6e ("netdev: Custom statistics.")
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9445
Signed-off-by: Ben Pfaff 


Still needs a review.

Thanks,

Ben.



Thanks for the patch Ben, unfortunately Michal is no longer active on 
the ML and I only just spotted this series, feel free to  cc myself for 
Intel custom stats work in the future.


Patch looks good to me overall, tested fine also. I can include this in 
this weeks pull request.


Thanks
Ian


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



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

Re: [ovs-dev] [PATCH v4 1/2] ovs python: ovs.stream.open_block() returns success even if the remote is unreachable

[Numan Siddique]

On Wed, Aug 8, 2018 at 2:58 AM Ben Pfaff  wrote:

> On Tue, Aug 07, 2018 at 05:12:32PM -0400, Mark Michelson wrote:
> > On 08/07/2018 07:37 AM, nusid...@redhat.com wrote:
> > >From: Numan Siddique 
> > >
> > >The python function ovs.socket_util.check_connection_completion() uses
> select()
> > >(provided by python) to monitor the socket file descriptor. The select()
> > >returns 1 when the file descriptor becomes ready. For error cases like -
> > >111 (Connection refused) and 113 (No route to host) (POLLERR),
> ovs.poller._SelectSelect.poll()
> > >expects the exceptfds list to be set by select(). But that is not the
> case.
> > >As per the select() man page, writefds list will be set for POLLERR.
> > >Please see "Correspondence between select() and poll() notifications"
> section of select(2)
> > >man page.
> > >
> > >Because of this behavior, ovs.socket_util.check_connection_completion()
> returns success
> > >even if the remote is unreachable or not listening on the port.
> > >
> > >This patch fixes this issue by using poll() to check the connection
> status similar to
> > >the C implementation of check_connection_completion().
> > >
> > >A new function 'get_system_poll() is added in ovs/poller.py which
> returns the
> > >select.poll() object. If select.poll is monkey patched by
> eventlet/gevent, it
> > >gets the original select.poll() and returns it.
> >
> > Is this safe? My concern is that eventlet/gevent monkey patches
> > select.poll() so that the green thread yields properly. Using the system
> > select.poll() might lead to unexpected blocking.
>
> gevent monkey patches Python to get rid of poll() because poll() might
> block and thereby stall Python.  This use of poll() is OK because it
> will never block (because it uses a timeout of 0).
>
> > I read your conversation with Ben on the previous iteration of this
> series.
> > It looks like you attempted to use the system select.poll() and ran into
> > this blocking problem (the pastebin for your patch is now deleted so I
> can't
> > see what you actually tried). Why would this approach work differently?
>
> I don't recall what was in the pastebin, but the use of poll() here
> should be sound.  We use the same approach in C and we don't want to
> block in C either.
>

When I tried last time, I modified the class 'SelectPoll' class here
- https://github.com/openvswitch/ovs/blob/master/python/ovs/poller.py#L145
to use system.poll and this blocked the whole process. But as Ben suggested
earlier
and mentioned above , we use select.poll only when checking the connection
status
with timeout of 0. So we are fine. I tested this patch with openstack
neutron and it worked
fine.

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

Re: [ovs-dev] [PATCH] rhel: Use openvswitch user in the logrotate configuration file

[Markos Chandras]

Hi Timothy,

On 08/07/2018 09:01 PM, Timothy Redaelli wrote:
> 
> Hi Markos,
> I agree with you that running logrotate as root is probably bad.
> 
> The problem is that, for backward compatibility, we keep OVS as "root"
> user if you upgrade OVS from an old version (older than the non-root
> user support).

Good point about the backwards compatibility. I will submit a v2

-- 
markos

SUSE LINUX GmbH | GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg) Maxfeldstr. 5, D-90409, Nürnberg
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev