[ovs-dev] OVS+DPDK vhost-user-client port hang

[王志克]

Hi,
I met one issue with qemu2.8.1.1+ovs2.7.0+dpdk16.11.0. The issue is that one 
windows 2008 VM can NOT send/receive packet anymore. I even can not 
re-initialize the virtio adapter in VM (no reponse). I can NOT reproduce it.
>From OVS+DPDK stats, there is no packet from the vhost-user-client port. The 
>port is Up using ovs-ofctl show.
I observed below log in qemu.

2017-11-02T03:48:48.265767Z qemu-kvm: virtio_ioport_write: unexpected address 
0x13 value 0x0
2017-11-02T03:48:50.265766Z qemu-kvm: virtio_ioport_write: unexpected address 
0x13 value 0x0
2017-11-02T03:48:52.265764Z qemu-kvm: virtio_ioport_write: unexpected address 
0x13 value 0x0
2017-11-02T03:48:54.265750Z qemu-kvm: virtio_ioport_write: unexpected address 
0x13 value 0x0
2017-11-02T03:48:55.452554Z qemu-kvm: Failed to set msg fds.
2017-11-02T03:48:55.452591Z qemu-kvm: vhost VQ 0 ring restore failed: -1: 
Resource temporarily unavailable (11)
2017-11-02T03:48:55.452601Z qemu-kvm: Failed to set msg fds.
2017-11-02T03:48:55.452606Z qemu-kvm: vhost VQ 1 ring restore failed: -1: 
Resource temporarily unavailable (11)
2017-11-02T03:49:10.040044Z qemu-kvm: terminating on signal 15 from pid 17716 
(/usr/sbin/libvirtd)

Does some one met similar issue? Or any idea how to solve it? Thanks

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

[ovs-dev] Please help me to debug memory mis-unmap in ovs-dpdk.

[Sam]

Hi all,

I'm debugging a mis-unmap bug in ovs-dpdk, process is this:

1. start ovs-dpdk with 10G socket memory, then use `top`, ovs-vswitchd take
10G memory. command is bellow.
2. start qemu vm1 with vhost device as server to connect port on ovs-dpdk,
command is bellow. qemu share 40G memory with ovs-dpdk, then use `top`,
ovs-vswitchd take 50G memory.
3. start qemu vm2 with vhost device as server to connect port on ovs-dpdk,
command is bellow. qemu share 20G memory with ovs-dpdk, then use `top`,
ovs-vswitchd take 70G memory.
4. stop vm1, then use `top`, ovs-vswitchd take 50G memory (I think it
should be 30G?). use `cat /proc/meminfo`, 70G memory is not released.

Please tell me where is the code ovs-dpdk and qemu communicate with each
other about the map/unmap memory region. Thank you~

Is it old bug? Refer this link:
http://dev.dpdk.narkive.com/m04py1jJ/dpdk-dev-vhost-user-technical-isssues


Command to start ovs-dpdk:
ovs-vswitchd --dpdk -c 0x14 -n 4 --socket-mem 10240 --proc-type secondary
-w :01:00.0 -w :01:00.1 --
unix:/usr/local/var/run/openvswitch/db.sock --pidfile --detach --log-file
--mlockall --no-chdir
--log-file=/usr/local/var/log/openvswitch/ovs-vswitchd.log
--pidfile=/usr/local/var/run/openvswitch/ovs-vswitchd.pid --detach --monitor

Command to start qemu:
/usr/local/bin/qemu-system-x86_64 -enable-kvm -cpu
qemu64,+vmx,+ssse3,+sse4.1,+sse4.2,+x2apic,+aes,+avx,+vme,+pat,+ss,+pclmulqdq,+xsave,level=13
-machine pc,accel=kvm -chardev
socket,id=hmqmondev,port=55919,host=127.0.0.1,nodelay,server,nowait -mon
chardev=hmqmondev,id=hmqmon,mode=readline -rtc
base=utc,clock=host,driftfix=none -usb -device usb-tablet -daemonize
-nodefaults -nodefconfig -no-kvm-pit-reinjection -global
kvm-pit.lost_tick_policy=discard -vga std -k en-us -smp 1 -name
gangyewei-qemutime-07-1 -m 20480 -boot order=cdn -vnc :19,password -drive
file=/opt/cloud/workspace/disks/4a5148da-e34e-4c81-aada-12a247c0337e,if=none,id=drive_0,format=qcow2,cache=none,aio=native
-device virtio-blk-pci,id=dev_drive_0,drive=drive_0,bus=pci.0,addr=0x5
-drive
file=/opt/cloud/workspace/disks/08eb3f72-07ba-44e9-8ee9-4ab662dbbd1f,if=none,id=drive_1,format=qcow2,cache=none,aio=native
-device virtio-blk-pci,id=dev_drive_1,drive=drive_1,bus=pci.0,addr=0x6
-device ide-cd,drive=ide0-cd0,bus=ide.1,unit=1 -drive
id=ide0-cd0,media=cdrom,if=none -chardev
socket,id=char-n-f879ac2f,path=/usr/local/var/run/openvswitch/n-f879ac2f,server
-netdev type=vhost-user,id=n-f879ac2f,chardev=char-n-f879ac2f,vhostforce=on
-device
virtio-net-pci,netdev=n-f879ac2f,mac=00:22:f8:79:ac:2f,id=netdev-n-f879ac2f,addr=0xf,speed=1000
-object
memory-backend-file,id=mem,size=20480M,mem-path=/dev/hugepages,share=on
-mem-prealloc -numa node,memdev=mem -pidfile
/opt/cloud/workspace/servers/63ef2ec4-556f-47ea-93e0-23089bb59be5/pid
-chardev
socket,path=/opt/cloud/workspace/servers/63ef2ec4-556f-47ea-93e0-23089bb59be5/qga.sock,server,nowait,id=qga0
-device virtio-serial -device
virtserialport,chardev=qga0,name=org.qemu.guest_agent.0 -qmp
unix:/opt/cloud/workspace/servers/63ef2ec4-556f-47ea-93e0-23089bb59be5/qmp.sock,server,nowait

Process:
[gangyewei@yf-mos-test-net07 63ef2ec4-556f-47ea-93e0-23089bb59be5]$ top
  PID USER  PR  NIVIRTRESSHR S  %CPU %MEM TIME+ COMMAND
38470 root  10 -10 79.751g 223160  10636 S 200.0  0.2  58:26.77
ovs-vswitchd
 4575 root  20   0 41.338g  90036   2924 S   0.0  0.1   0:21.71
qemu-system-x86
 5914 root  20   0 20.652g  66824   2876 S   0.0  0.1   0:11.90
qemu-system-x86

[gangyewei@yf-mos-test-net07 63ef2ec4-556f-47ea-93e0-23089bb59be5]$ cd
../46c035e2-605c-4997-b4df-15d13f97758f
[gangyewei@yf-mos-test-net07 46c035e2-605c-4997-b4df-15d13f97758f]$ sudo sh
stopvm
Remove PID
/opt/cloud/workspace/servers/46c035e2-605c-4997-b4df-15d13f97758f/pid
Remove VNC
/opt/cloud/workspace/servers/46c035e2-605c-4997-b4df-15d13f97758f/vnc
[gangyewei@yf-mos-test-net07 46c035e2-605c-4997-b4df-15d13f97758f]$ top
  PID USER  PR  NIVIRTRESSHR S  %CPU %MEM TIME+ COMMAND
38470 root  10 -10 55.751g 223160  10636 S 200.3  0.2  59:30.31
ovs-vswitchd
 5914 root  20   0 20.652g  66824   2876 S   0.0  0.1   0:11.92
qemu-system-x86

[gangyewei@yf-mos-test-net07 46c035e2-605c-4997-b4df-15d13f97758f]$ cat
/proc/meminfo
HugePages_Total: 112
HugePages_Free:   42
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] netdev-linux: Fix wrong ceil rate when max-rate less than 8bit.

[fukaige]

From: Kaige Fu 

When max-rate is less than 8bit, the hc->max_rate will be set
as htb->max_rate mistakenly instead of mtu of netdev.

Fixes: 13c1637 ("smap: New function smap_get_ullong().")

Signed-off-by: Kaige Fu 
---
 lib/netdev-linux.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 2ff3e2b..66ca5fe 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -3762,6 +3762,7 @@ htb_parse_class_details__(struct netdev *netdev,
 {
 const struct htb *htb = htb_get__(netdev);
 int mtu, error;
+unsigned long long int max_rate_bit;
 
 error = netdev_linux_get_mtu__(netdev_linux_cast(netdev), );
 if (error) {
@@ -3777,10 +3778,10 @@ htb_parse_class_details__(struct netdev *netdev,
 hc->min_rate = MIN(hc->min_rate, htb->max_rate);
 
 /* max-rate */
-hc->max_rate = smap_get_ullong(details, "max-rate", 0) / 8;
-if (!hc->max_rate) {
-hc->max_rate = htb->max_rate;
-}
+max_rate_bit = smap_get_ullong(details, "max-rate", 0);
+hc->max_rate = (max_rate_bit
+? max_rate_bit / 8
+: htb->max_rate); 
 hc->max_rate = MAX(hc->max_rate, hc->min_rate);
 hc->max_rate = MIN(hc->max_rate, htb->max_rate);
 
-- 
1.8.3.1


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

Re: [ovs-dev] [PATCH net-next v15] openvswitch: enable NSH support

[Yang, Yi]

On Thu, Nov 02, 2017 at 08:52:40AM +0800, Pravin Shelar wrote:
> On Tue, Oct 31, 2017 at 9:03 PM, Yi Yang  wrote:
> >
> > OVS master and 2.8 branch has merged NSH userspace
> > patch series, this patch is to enable NSH support
> > in kernel data path in order that OVS can support
> > NSH in compat mode by porting this.
> >
> > Signed-off-by: Yi Yang 
> > ---
> I have comment related to checksum, otherwise patch looks good to me.

Pravin, thank you for your comments, the below part is incremental patch
for checksum, please help check it, I'll send out v16 with this after
you confirm.

diff --git a/net/nsh/nsh.c b/net/nsh/nsh.c
index 2764682..d7da99a 100644
--- a/net/nsh/nsh.c
+++ b/net/nsh/nsh.c
@@ -36,6 +36,7 @@ int nsh_push(struct sk_buff *skb, const struct nshhdr 
*pushed_nh)
nh = (struct nshhdr *)(skb->data);
memcpy(nh, pushed_nh, length);
nh->np = next_proto;
+   skb_postpush_rcsum(skb, nh, length);
 
skb->protocol = htons(ETH_P_NSH);
skb_reset_mac_header(skb);
@@ -63,7 +64,7 @@ int nsh_pop(struct sk_buff *skb)
if (!inner_proto)
return -EAFNOSUPPORT;
 
-   skb_pull(skb, length);
+   skb_pull_rcsum(skb, length);
skb_reset_mac_header(skb);
skb_reset_network_header(skb);
skb_reset_mac_len(skb);
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index dd1449d..5ba0e35 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -671,6 +671,7 @@ static int set_nsh(struct sk_buff *skb, struct sw_flow_key 
*flow_key,
return err;
 
nh = nsh_hdr(skb);
+   skb_postpull_rcsum(skb, nh, length);
flags = nsh_get_flags(nh);
flags = OVS_MASKED(flags, key.base.flags, mask.base.flags);
flow_key->nsh.base.flags = flags;
@@ -698,6 +699,7 @@ static int set_nsh(struct sk_buff *skb, struct sw_flow_key 
*flow_key,
default:
return -EINVAL;
}
+   skb_postpush_rcsum(skb, nh, length);
return 0;
 }
 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] ovn-northd.8: Fix wrong description

[wei]

---
 ovn/northd/ovn-northd.8.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index e9799e18a..37284301b 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -1422,7 +1422,7 @@ icmp4 {
   B, a priority-100 flow matches ip 
   ip4.dst == B  inport == GW,
   where GW is the logical router gateway port, with an
-  action ct_snat; next;.
+  action ct_snat;.
 
 
 
-- 
2.12.2.windows.2


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

Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to include/openvswitch

[Xiao Liang]

On Thu, Nov 2, 2017 at 5:51 AM, Alin Serdean
 wrote:
>
>
>> -Original Message-
>> From: Ben Pfaff [mailto:b...@ovn.org]
>> Sent: Tuesday, October 31, 2017 9:34 PM
>> To: Xiao Liang ; Alin Serdean
>> 
>> Cc: d...@openvswitch.org
>> Subject: Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to
>> include/openvswitch
>>
>> On Thu, Aug 17, 2017 at 12:06:25AM +0800, Xiao Liang wrote:
>> > Poll-loop is the core to implement main loop. It should be available
>> > in libopenvswitch.
>> >
>> > Signed-off-by: Xiao Liang 
>>
>> I'm concerned about the way that this adds a definition of HANDLE in a public
>> header.  That seems unfriendly to code that might want to include both this
>> header and Win32 headers that properly define HANDLE.
>>
>> Alin, what's the right thing to do here?
> [Alin Serdean] First the type definition is wrong (HANDLE is VOID*).
> I would avoid adding the definition to HANDLE. Maybe add an include to 
>  or  to include/windows/poll.h .

Since include/windows is not installed as public header, is it safe to
remove inclusion of poll.h and add windows.h in poll-loop.h?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] datapath: Remove redundant check for IFF_NO_QUEUE

[Kaige Fu]

On 2017/11/2 4:19, Ben Pfaff wrote:
> On Wed, Nov 01, 2017 at 08:43:20AM -0700, Greg Rose wrote:
>> On 10/30/2017 06:13 PM, fukaige wrote:
>>> From: Kaige Fu 
>>>
>>> IFF_NO_QUEUE is checked twice. This cause a situation
>>> that HAVE_IFF_NO_QUEUE is defined no matter whether
>>> the flag IFF_NO_QUEUE is defined in kernel or not.
>>>
>>> Signed-off-by: Kaige Fu 
>>> ---
>>>  datapath/linux/compat/include/linux/netdevice.h | 3 ---
>>>  1 file changed, 3 deletions(-)
>>>
>>> diff --git a/datapath/linux/compat/include/linux/netdevice.h 
>>> b/datapath/linux/compat/include/linux/netdevice.h
>>> index a5d4ee8..3c3cf42 100644
>>> --- a/datapath/linux/compat/include/linux/netdevice.h
>>> +++ b/datapath/linux/compat/include/linux/netdevice.h
>>> @@ -22,9 +22,6 @@ struct net;
>>>  #define IFF_LIVE_ADDR_CHANGE 0
>>>  #endif
>>> -#ifndef IFF_NO_QUEUE
>>> -#define IFF_NO_QUEUE   0
>>> -#endif
>>>  #ifndef IFF_OPENVSWITCH
>>>  #define IFF_OPENVSWITCH 0
>>>  #endif
>>>
>> Looks correct to me.
>>
>> Reviewed-by: Greg Rose 
> Thanks Kaige and Greg.  I applied this to master.  Let me know if it
> should be backported.
>
> .

Thanks for applying this patch. It should be backported to 
branch-2.8/branch-2.7/branch-2.6.

-- 
Thanks
Kaige Fu


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

Re: [ovs-dev] [PATCH net-next v15] openvswitch: enable NSH support

[Pravin Shelar]

On Tue, Oct 31, 2017 at 9:03 PM, Yi Yang  wrote:
> v14->v15
>  - Check size in nsh_hdr_from_nlattr
>  - Fixed four small issues pointed out By Jiri and Eric
>
> v13->v14
>  - Rename skb_push_nsh to nsh_push per Dave's comment
>  - Rename skb_pop_nsh to nsh_pop per Dave's comment
>
> v12->v13
>  - Fix NSH header length check in set_nsh
>
> v11->v12
>  - Fix missing changes old comments pointed out
>  - Fix new comments for v11
>
> v10->v11
>  - Fix the left three disputable comments for v9
>but not fixed in v10.
>
> v9->v10
>  - Change struct ovs_key_nsh to
>struct ovs_nsh_key_base base;
>__be32 context[NSH_MD1_CONTEXT_SIZE];
>  - Fix new comments for v9
>
> v8->v9
>  - Fix build error reported by daily intel build
>because nsh module isn't selected by openvswitch
>
> v7->v8
>  - Rework nested value and mask for OVS_KEY_ATTR_NSH
>  - Change pop_nsh to adapt to nsh kernel module
>  - Fix many issues per comments from Jiri Benc
>
> v6->v7
>  - Remove NSH GSO patches in v6 because Jiri Benc
>reworked it as another patch series and they have
>been merged.
>  - Change it to adapt to nsh kernel module added by NSH
>GSO patch series
>
> v5->v6
>  - Fix the rest comments for v4.
>  - Add NSH GSO support for VxLAN-gpe + NSH and
>Eth + NSH.
>
> v4->v5
>  - Fix many comments by Jiri Benc and Eric Garver
>for v4.
>
> v3->v4
>  - Add new NSH match field ttl
>  - Update NSH header to the latest format
>which will be final format and won't change
>per its author's confirmation.
>  - Fix comments for v3.
>
> v2->v3
>  - Change OVS_KEY_ATTR_NSH to nested key to handle
>length-fixed attributes and length-variable
>attriubte more flexibly.
>  - Remove struct ovs_action_push_nsh completely
>  - Add code to handle nested attribute for SET_MASKED
>  - Change PUSH_NSH to use the nested OVS_KEY_ATTR_NSH
>to transfer NSH header data.
>  - Fix comments and coding style issues by Jiri and Eric
>
> v1->v2
>  - Change encap_nsh and decap_nsh to push_nsh and pop_nsh
>  - Dynamically allocate struct ovs_action_push_nsh for
>length-variable metadata.
>
> OVS master and 2.8 branch has merged NSH userspace
> patch series, this patch is to enable NSH support
> in kernel data path in order that OVS can support
> NSH in compat mode by porting this.
>
> Signed-off-by: Yi Yang 
> ---
I have comment related to checksum, otherwise patch looks good to me.

>  include/net/nsh.h|   3 +
>  include/uapi/linux/openvswitch.h |  29 
>  net/nsh/nsh.c|  59 
>  net/openvswitch/Kconfig  |   1 +
>  net/openvswitch/actions.c| 119 +++
>  net/openvswitch/flow.c   |  51 +++
>  net/openvswitch/flow.h   |   7 +
>  net/openvswitch/flow_netlink.c   | 315 
> ++-
>  net/openvswitch/flow_netlink.h   |   5 +
>  9 files changed, 588 insertions(+), 1 deletion(-)
>

...
> diff --git a/net/nsh/nsh.c b/net/nsh/nsh.c
> index 58fb827..2764682 100644
> --- a/net/nsh/nsh.c
> +++ b/net/nsh/nsh.c
> @@ -14,6 +14,65 @@
>  #include 
>  #include 
>
> +int nsh_push(struct sk_buff *skb, const struct nshhdr *pushed_nh)
> +{
> +   struct nshhdr *nh;
> +   size_t length = nsh_hdr_len(pushed_nh);
> +   u8 next_proto;
> +
> +   if (skb->mac_len) {
> +   next_proto = TUN_P_ETHERNET;
> +   } else {
> +   next_proto = tun_p_from_eth_p(skb->protocol);
> +   if (!next_proto)
> +   return -EAFNOSUPPORT;
> +   }
> +
> +   /* Add the NSH header */
> +   if (skb_cow_head(skb, length) < 0)
> +   return -ENOMEM;
> +
> +   skb_push(skb, length);
> +   nh = (struct nshhdr *)(skb->data);
> +   memcpy(nh, pushed_nh, length);
> +   nh->np = next_proto;
dont you need to update checksum for CHECKSUM_COMPLETE case?

> +
> +   skb->protocol = htons(ETH_P_NSH);
> +   skb_reset_mac_header(skb);
> +   skb_reset_network_header(skb);
> +   skb_reset_mac_len(skb);
> +
> +   return 0;
> +}
> +EXPORT_SYMBOL_GPL(nsh_push);
> +
> +int nsh_pop(struct sk_buff *skb)
> +{
> +   struct nshhdr *nh;
> +   size_t length;
> +   __be16 inner_proto;
> +
> +   if (!pskb_may_pull(skb, NSH_BASE_HDR_LEN))
> +   return -ENOMEM;
> +   nh = (struct nshhdr *)(skb->data);
> +   length = nsh_hdr_len(nh);
> +   inner_proto = tun_p_to_eth_p(nh->np);
> +   if (!pskb_may_pull(skb, length))
> +   return -ENOMEM;
> +
> +   if (!inner_proto)
> +   return -EAFNOSUPPORT;
> +
> +   skb_pull(skb, length);
same as above, we need to update the checksum.

> +   skb_reset_mac_header(skb);
> +   skb_reset_network_header(skb);
> +   skb_reset_mac_len(skb);
> +   skb->protocol = inner_proto;
> +
> +   return 0;
> +}
> +EXPORT_SYMBOL_GPL(nsh_pop);
> +
>  static struct sk_buff 

[ovs-dev] [PATCH v10 4/4] ovn: Generate Neighbor Solicitation packet for unknown MAC IPv6 packets

[nusiddiq]

From: Numan Siddique 

In the router ingress pipeline, if the destination mac is unresolved
by the time the packet reaches the ARP_REQUEST stage, OVN should generate an
IPv6 Neighbor Solicitation packet to learn the MAC address. This feature is
presently missing. This patch adds this feature.  A new action "nd_ns" is
added  which replaces an IPv6 packet being processed with an IPv6 Neighbor
Solicitation packet. ovn-northd adds a flow in the ARP_REQUEST router ingress
pipeline stage if the eth.dst is zero which applies this action. This action is
similar to the IPv4 counterpart "arp" action.

OVN already has the support to learn the MAC from the IPv6 Neighbor 
Advertisement
packets and storing in the south bound MAC_Binding table.

Signed-off-by: Numan Siddique 
---
 include/ovn/actions.h   |   9 +++-
 ovn/controller/pinctrl.c| 122 +++-
 ovn/lib/actions.c   |  22 
 ovn/northd/ovn-northd.8.xml |  24 ++---
 ovn/northd/ovn-northd.c |   8 ++-
 ovn/ovn-sb.xml  |  37 ++
 ovn/utilities/ovn-trace.c   |  30 +++
 tests/ovn.at| 116 +
 8 files changed, 302 insertions(+), 66 deletions(-)

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 15cee478d..8c7208ffc 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -73,7 +73,8 @@ struct simap;
 OVNACT(SET_QUEUE, ovnact_set_queue)   \
 OVNACT(DNS_LOOKUP,ovnact_dns_lookup)  \
 OVNACT(LOG,   ovnact_log) \
-OVNACT(PUT_ND_RA_OPTS,ovnact_put_opts)
+OVNACT(PUT_ND_RA_OPTS,ovnact_put_opts)\
+OVNACT(ND_NS, ovnact_nest)
 
 /* enum ovnact_type, with a member OVNACT_ for each action. */
 enum OVS_PACKED_ENUM ovnact_type {
@@ -427,6 +428,12 @@ enum action_opcode {
  *   - Any number of ICMPv6 options.
  */
 ACTION_OPCODE_PUT_ND_RA_OPTS,
+
+/* "nd_ns { ...actions... }".
+ *
+ * The actions, in OpenFlow 1.3 format, follow the action_header.
+ */
+ACTION_OPCODE_ND_NS,
 };
 
 /* Header. */
diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 3e470717c..29b2dde0c 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -85,6 +85,9 @@ static void pinctrl_handle_put_nd_ra_opts(
 const struct flow *ip_flow, struct dp_packet *pkt_in,
 struct ofputil_packet_in *pin, struct ofpbuf *userdata,
 struct ofpbuf *continuation);
+static void pinctrl_handle_nd_ns(const struct flow *ip_flow,
+ const struct match *md,
+ struct ofpbuf *userdata);
 
 COVERAGE_DEFINE(pinctrl_drop_put_mac_binding);
 
@@ -132,6 +135,43 @@ set_switch_config(struct rconn *swconn,
 }
 
 static void
+set_actions_and_enqueue_msg(const struct dp_packet *packet,
+   const struct match *md,
+   struct ofpbuf *userdata)
+{
+/* Copy metadata from 'md' into the packet-out via "set_field"
+ * actions, then add actions from 'userdata'.
+ */
+uint64_t ofpacts_stub[4096 / 8];
+struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
+enum ofp_version version = rconn_get_version(swconn);
+
+reload_metadata(, md);
+enum ofperr error = ofpacts_pull_openflow_actions(userdata, userdata->size,
+  version, NULL, NULL,
+  );
+if (error) {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+VLOG_WARN_RL(, "failed to parse actions from userdata (%s)",
+ ofperr_to_string(error));
+ofpbuf_uninit();
+return;
+}
+
+struct ofputil_packet_out po = {
+.packet = dp_packet_data(packet),
+.packet_len = dp_packet_size(packet),
+.buffer_id = UINT32_MAX,
+.ofpacts = ofpacts.data,
+.ofpacts_len = ofpacts.size,
+};
+match_set_in_port(_metadata, OFPP_CONTROLLER);
+enum ofputil_protocol proto = ofputil_protocol_from_ofp_version(version);
+queue_msg(ofputil_encode_packet_out(, proto));
+ofpbuf_uninit();
+}
+
+static void
 pinctrl_handle_arp(const struct flow *ip_flow, const struct match *md,
struct ofpbuf *userdata)
 {
@@ -166,40 +206,8 @@ pinctrl_handle_arp(const struct flow *ip_flow, const 
struct match *md,
   ip_flow->vlans[0].tci);
 }
 
-/* Compose actions.
- *
- * First, copy metadata from 'md' into the packet-out via "set_field"
- * actions, then add actions from 'userdata'.
- */
-uint64_t ofpacts_stub[4096 / 8];
-struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
-enum ofp_version version = rconn_get_version(swconn);
-
-reload_metadata(, md);
-enum ofperr error = 

[ovs-dev] [PATCH v10 3/4] ovn-northd: Add logical flows to support native IPv6 RA

[nusiddiq]

From: Zongkai LI 

This patch adds logical flows which sends IPv6 Router Advertisement
packet in response to the IPv6 Router Solicitation request. It uses
the actions "put_nd_ra_opts" to transform the RS packet to RA packet
in the newly added ingress stage "lr_in_nd_ra_options" in router
pipeline. If the action "put_nd_ra_opts" is successful, it sends the
RA packet back to the originating port in the next ingress stage
"lr_in_nd_ra_response".

A new column "ipv6_ra_configs" is added in the Logical_Router_Port
table, which the CMS is expected to configure IPv6 RA
configurations - "address_mode" and "mtu" for adding these flows.

Co-authored-by: Numan Siddique 
Signed-off-by: Zongkai LI 
Signed-off-by: Numan Siddique 
Acked-by: Miguel Angel Ajo 
---
 ovn/lib/logical-fields.c|   4 +
 ovn/northd/ovn-northd.8.xml |  83 ++-
 ovn/northd/ovn-northd.c | 137 +---
 ovn/ovn-nb.ovsschema|   7 +-
 ovn/ovn-nb.xml  |  39 +++
 ovn/ovn-sb.xml  |   4 +
 tests/ovn.at| 249 
 7 files changed, 500 insertions(+), 23 deletions(-)

diff --git a/ovn/lib/logical-fields.c b/ovn/lib/logical-fields.c
index 26e336f5a..a8b5e3c51 100644
--- a/ovn/lib/logical-fields.c
+++ b/ovn/lib/logical-fields.c
@@ -183,6 +183,10 @@ ovn_init_symtab(struct shash *symtab)
   "icmp6.type == 135 && icmp6.code == 0 && ip.ttl == 255");
 expr_symtab_add_predicate(symtab, "nd_na",
   "icmp6.type == 136 && icmp6.code == 0 && ip.ttl == 255");
+expr_symtab_add_predicate(symtab, "nd_rs",
+  "icmp6.type == 133 && icmp6.code == 0 && ip.ttl == 255");
+expr_symtab_add_predicate(symtab, "nd_ra",
+  "icmp6.type == 134 && icmp6.code == 0 && ip.ttl == 255");
 expr_symtab_add_field(symtab, "nd.target", MFF_ND_TARGET, "nd", false);
 expr_symtab_add_field(symtab, "nd.sll", MFF_ND_SLL, "nd_ns", false);
 expr_symtab_add_field(symtab, "nd.tll", MFF_ND_TLL, "nd_na", false);
diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index e9799e18a..e7e05577c 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -1592,7 +1592,82 @@ icmp4 {
   
 
 
-Ingress Table 5: IP Routing
+Ingress Table 5: IPv6 ND RA option processing
+
+
+  
+
+  A priority-50 logical flow is added for each logical router port
+  configured with IPv6 ND RA options which matches IPv6 ND Router
+  Solicitation packet and applies the action
+  put_nd_ra_opts and advances the packet to the next
+  table.
+
+
+
+reg0[5] = put_nd_ra_opts(options);next;
+
+
+
+  For a valid IPv6 ND RS packet, this transforms the packet into an
+  IPv6 ND RA reply and sets the RA options to the packet and stores 1
+  into reg0[5]. For other kinds of packets, it just stores 0 into
+  reg0[5]. Either way, it continues to the next table.
+
+  
+
+  
+A priority-0 logical flow with match 1 has actions
+next;.
+  
+
+
+Ingress Table 6: IPv6 ND RA responder
+
+
+  This table implements IPv6 ND RA responder for the IPv6 ND RA replies
+  generated by the previous table.
+
+
+
+  
+
+  A priority-50 logical flow is added for each logical router port
+  configured with IPv6 ND RA options which matches IPv6 ND RA
+  packets and reg0[5] == 1 and responds back to the
+  inport after applying these actions.
+  If reg0[5] is set to 1, it means that the action
+  put_nd_ra_opts was successful.
+
+
+
+eth.dst = eth.src;
+eth.src = E;
+ip6.dst = ip6.src;
+ip6.src = I;
+outport = P;
+flags.loopback = 1;
+output;
+
+
+
+  where E is the MAC address and I is the IPv6
+  link local address of the logical router port.
+
+
+
+  (This terminates packet processing in ingress pipeline; the packet
+  does not go to the next ingress table.)
+
+  
+
+  
+A priority-0 logical flow with match 1 has actions
+next;.
+  
+
+
+Ingress Table 7: IP Routing
 
 
   A packet that arrives at this table is an IP packet that should be
@@ -1694,7 +1769,7 @@ next;
   
 
 
-Ingress Table 6: ARP/ND Resolution
+Ingress Table 8: ARP/ND Resolution
 
 
   Any packet that reaches this table is an IP packet whose next-hop
@@ -1787,7 +1862,7 @@ next;
   
 
 
-Ingress Table 7: Gateway Redirect
+Ingress Table 9: Gateway Redirect
 
 
   For distributed logical routers where one of the logical router
@@ -1844,7 +1919,7 @@ next;
   
 
 
-Ingress Table 8: ARP Request
+Ingress Table 10: ARP Request
 
 
 

[ovs-dev] [PATCH v10 0/4] Add Router Solicitation responder support and generate Neighbor Solicitation request for unknown

[nusiddiq]

From: Numan Siddique 

v9 -> v10
-
Resolved merge conflicts in patch 1.

v8 -> v9

Addressed review comments in patch 2 and patch 4.


v7 -> v8

Rebased p2.

v6 -> v7

In version 7, a new patch p4 is added which adds the support to generate a 
Neighbor Solicitation packet for IPv6
packets whose destination MAC is still unresolved by the time it reaches the 
ARP_REQUEST router ingress pipeline.
To support this a new action "nd_ns" is added.

p2 - Added the missing code required to parse 'put_nd_ra_opts' in ovn-trace.c 
which was missed out in the previous versions.


v5 -> v6

Addressed review comments in p2. 'put_nd_ra_opts' action now encodes the ICMPv6 
RA
complete data in the 'userdata' field of the controller action.

v4 -> v5
---
There were some unrelated changes which cropped in when rebasing in v4 p1.
Corrected it in v5.

v3 -> v4
---
Resolved the merge conflicts and rebased it.
Patch 3 - Addressed review comments from v3

v2 -> v3

Fixed the checkpatch errors and review comments from v2

v1 -> v2
---

The patches p1 and p2 of v1 are merged. p3 of v1 is dropped.

v1
--

This patch series supports IPv6 Router solicitation responder support.
This patch series picks up from the patches from here [1] and reset the version
to 1 since few of the patches were accepted.

In this new series continuations are used to support this feature.
When IPv6 RS packet is received, it is sent to ovn-controller which applies
the action 'put_nd_ra_opts' to transform it to an IPv6 RA packet and resumes
the packet. ovn-northd adds the necessary flows to respond the RA packet
back to the originating port.

[1] - https://mail.openvswitch.org/pipermail/ovs-dev/2017-May/332008.html


Numan Siddique (3):
  ovn util: Refactor dhcp_opts_map to make it generic
  ovn-controller: Add a new action - 'put_nd_ra_opts'
  ovn: Generate Neighbor Solicitation packet for unknown MAC IPv6
packets

Zong Kai LI (1):
  ovn-northd: Add logical flows to support native IPv6 RA

 include/ovn/actions.h|  37 +++-
 ovn/controller/lflow.c   |  15 +-
 ovn/controller/pinctrl.c | 218 +++--
 ovn/lib/actions.c| 316 ++-
 ovn/lib/automake.mk  |   2 +-
 ovn/lib/logical-fields.c |   4 +
 ovn/lib/{ovn-dhcp.h => ovn-l7.h} | 115 +---
 ovn/northd/ovn-northd.8.xml  | 107 ++-
 ovn/northd/ovn-northd.c  | 159 +---
 ovn/ovn-nb.ovsschema |   7 +-
 ovn/ovn-nb.xml   |  39 
 ovn/ovn-sb.xml   | 118 
 ovn/utilities/ovn-trace.c|  91 +++--
 tests/ovn.at | 396 +++
 tests/test-ovn.c |  16 +-
 15 files changed, 1443 insertions(+), 197 deletions(-)
 rename ovn/lib/{ovn-dhcp.h => ovn-l7.h} (63%)

-- 
2.13.5

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

[ovs-dev] [PATCH v2] ofproto/trace: Fix memory leak in oftrace_push_ct_state()

[Yi-Hung Wei]

Free the allocated memory in the pop function.

Fixes: 0f2f05bbcf743 ("ofproto/trace: Add --ct-next option to ofproto/trace")
Signed-off-by: Yi-Hung Wei 
---
 ofproto/ofproto-dpif-trace.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
index d8a559ab6394..d5da48e326bb 100644
--- a/ofproto/ofproto-dpif-trace.c
+++ b/ofproto/ofproto-dpif-trace.c
@@ -135,7 +135,9 @@ oftrace_pop_ct_state(struct ovs_list *next_ct_states)
 {
 struct oftrace_next_ct_state *s;
 LIST_FOR_EACH_POP (s, node, next_ct_states) {
-return s->state;
+uint32_t state = s->state;
+free(s);
+return state;
 }
 OVS_NOT_REACHED();
 }
-- 
2.7.4

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

Re: [ovs-dev] [PATCH 1/9] ofproto/trace: Fix memory leak in oftrace_push_ct_state()

[Yi-Hung Wei]

Thanks Greg and Ben for review. It looks like this patch is
independent to the reset of the series. I will pull it out and send
v2.

Thanks,

-Yi-Hung

On Tue, Oct 31, 2017 at 3:18 PM, Ben Pfaff  wrote:
> On Fri, Aug 25, 2017 at 03:51:11PM -0700, Yi-Hung Wei wrote:
>> Free the allocated memory in the pop function.
>>
>> Fixes: 0f2f05bbcf743 ("ofproto/trace: Add --ct-next option to ofproto/trace")
>> Signed-off-by: Yi-Hung Wei 
>> ---
>>  ofproto/ofproto-dpif-trace.c | 13 -
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/ofproto/ofproto-dpif-trace.c b/ofproto/ofproto-dpif-trace.c
>> index 38d11002f290..a45c9cfd9619 100644
>> --- a/ofproto/ofproto-dpif-trace.c
>> +++ b/ofproto/ofproto-dpif-trace.c
>> @@ -128,12 +128,14 @@ oftrace_push_ct_state(struct ovs_list *next_ct_states, 
>> uint32_t ct_state)
>>  ovs_list_push_back(next_ct_states, _ct_state->node);
>>  }
>>
>> -static uint32_t
>> -oftrace_pop_ct_state(struct ovs_list *next_ct_states)
>> +static void
>> +oftrace_pop_ct_state(struct ovs_list *next_ct_states, uint32_t *ct_state)
>>  {
>>  struct oftrace_next_ct_state *s;
>>  LIST_FOR_EACH_POP (s, node, next_ct_states) {
>> -return s->state;
>> +*ct_state = s->state;
>> +free(s);
>> +return;
>>  }
>>  OVS_NOT_REACHED();
>>  }
>
> Thanks for the fix!
>
> I don't understand why the function return type needs to change.  Can
> you change this to preserve the return type, while fixing the memory
> leak?
>
> Thanks,
>
> Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 0/6] installer fixes on msbuild64

[aserdean]

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Ben Pfaff
> Sent: Wednesday, November 1, 2017 12:37 AM
> To: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH 0/6] installer fixes on msbuild64
> 
> On Tue, Oct 31, 2017 at 07:25:31PM +0200, Alin Gabriel Serdean wrote:
> > Installer fixes to allow the MSI build using msbuild(64bit variant).
> >
> > Alin Gabriel Serdean (6):
> >   build-windows: Suppress output from MSBuild
> >   installer-windows: Remove unused entries from WIX project
> >   installer-windows: Resolve WIX solution build type
> >   installer-windows: Call WIX binaries outside of MSBuild on x64
> >   installer-windows: Modify installer so it can be compiled on x64
> >   installer-windows: Add x64 installer build via command line
> 
> All of these seem reasonable to me.  I don't fully understand a lot of
them
> but the commit messages make some sense to me.  Please feel free to seek
> better reviews, if you like.
Thanks for looking over patches.
I hope to get more reviews from the other windows devs (at least somebody
else to test it and double confirm it works).

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

Re: [ovs-dev] [PATCH, RFC] tests: Add a default timeout for control utilities

[Alin Serdean]

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Ben Pfaff
> Sent: Wednesday, November 1, 2017 12:32 AM
> To: Alin Gabriel Serdean 
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH, RFC] tests: Add a default timeout for control
> utilities
> 
> On Sat, Sep 02, 2017 at 08:52:38AM -0700, Ben Pfaff wrote:
> > On Mon, Aug 28, 2017 at 08:14:59PM +0300, Alin Gabriel Serdean wrote:
> > > Let's suppose that ovsdb-server is running properly, but
> > > ovs-vswitchd is not responsive/crashed. We try to add a port via
> > > ovs-vsctl and it will hang.
> > > This patch aims at that scenario and tries to make life easier when
> > > debugging hanging tests.
> > >
> > > Some shells do not allow dashes in function names (default
> > > behavior), we shall try to define an alias to overcome dashes if the shell
> allows it.
> > >
> > > Signed-off-by: Alin Gabriel Serdean 
> > > Suggested-by: Ben Pfaff 
> >
> > Acked-by: Ben Pfaff 
> 
> I expected that you'd apply this, but, anyway, I've done it just now.
[Alin Serdean] Thanks Ben! I was convinced I applied this patch before I went 
on vacation but apparently, I didn't. Sorry!
> ___
> 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 v2 2/4] ovn: Allow ct_lb actions to take IPv6 address arguments.

[Ben Pfaff]

On Tue, Oct 31, 2017 at 09:27:02AM -0500, Mark Michelson wrote:
> The ct_lb action previously assumed that any address arguments were
> IPv4. This patch expands the parsing, formatting, and encoding of ct_lb
> to be amenable to IPv6 addresses as well.
> 
> Signed-off-by: Mark Michelson 

Thanks for working on this.

With this change, I get the following warning from Clang 3.8:

../ovn/lib/actions.c:939:32: error: variable 'dst' is used uninitialized 
whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
../ovn/lib/actions.c:953:30: note: uninitialized use occurs here
../ovn/lib/actions.c:939:28: note: remove the 'if' if its condition is 
always true
../ovn/lib/actions.c:932:21: error: variable 'dst' is used uninitialized 
whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
../ovn/lib/actions.c:953:30: note: uninitialized use occurs here
../ovn/lib/actions.c:932:17: note: remove the 'if' if its condition is 
always true
../ovn/lib/actions.c:886:13: note: variable 'dst' is declared here

I can't figure out what it's talking about, and I can't figure out a
simple way to suppress it.  Do you have any idea?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to include/openvswitch

[Alin Serdean]

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Tuesday, October 31, 2017 9:34 PM
> To: Xiao Liang ; Alin Serdean
> 
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [PATCH 2/2] lib: Move lib/poll-loop.h to
> include/openvswitch
> 
> On Thu, Aug 17, 2017 at 12:06:25AM +0800, Xiao Liang wrote:
> > Poll-loop is the core to implement main loop. It should be available
> > in libopenvswitch.
> >
> > Signed-off-by: Xiao Liang 
> 
> I'm concerned about the way that this adds a definition of HANDLE in a public
> header.  That seems unfriendly to code that might want to include both this
> header and Win32 headers that properly define HANDLE.
> 
> Alin, what's the right thing to do here?
[Alin Serdean] First the type definition is wrong (HANDLE is VOID*).
I would avoid adding the definition to HANDLE. Maybe add an include to 
 or  to include/windows/poll.h .
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] packets: Fix C++ compilation issues when include packets.h

[Yi-Hung Wei]

This patch fixes three C++ compilation errors when it includes
"lib/packets.h".

1) Fix in "include/openvswitch/util.h" is to avoid duplicated
named_member__ in struct pkt_metadata.

2) Fix in "lib/packets.h" is because designated initializers are not
implemented in GNU C++ [1].

3) Fix in "lib/util.h" is because __builtin_types_compatible_p and
__builtin_choose_expr are only supported in GCC. I use one solution
for C++ that is type-safe and works at compile time from [2].

[1]: https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html
[2]: https://goo.gl/xNe48A

Signed-off-by: Yi-Hung Wei 
---
 include/openvswitch/util.h | 12 ++--
 lib/packets.h  |  4 +++-
 lib/util.h | 15 ++-
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/include/openvswitch/util.h b/include/openvswitch/util.h
index 84d7e07ff370..c3e60d56135b 100644
--- a/include/openvswitch/util.h
+++ b/include/openvswitch/util.h
@@ -232,12 +232,12 @@ OVS_NO_RETURN void ovs_assert_failure(const char *, const 
char *, const char *);
 uint8_t PAD_ID[ROUND_UP(sizeof(struct { MEMBERS }), UNIT)]; \
 }
 #else
-#define PADDED_MEMBERS_CACHELINE_MARKER(UNIT, CACHELINE, MEMBERS)   \
-union { \
-OVS_CACHE_LINE_MARKER CACHELINE;\
-struct { MEMBERS }; \
-struct { MEMBERS } named_member__;  \
-uint8_t PAD_ID[ROUND_UP(sizeof named_member__, UNIT)];  \
+#define PADDED_MEMBERS_CACHELINE_MARKER(UNIT, CACHELINE, MEMBERS)   \
+union { \
+OVS_CACHE_LINE_MARKER CACHELINE;\
+struct { MEMBERS }; \
+struct { MEMBERS } named_member_##CACHELINE;\
+uint8_t PAD_ID[ROUND_UP(sizeof named_member_##CACHELINE, UNIT)];\
 }
 #endif
 
diff --git a/lib/packets.h b/lib/packets.h
index 705d0b270966..782fdd7c8bf8 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -1093,7 +1093,9 @@ static inline bool ipv6_addr_is_multicast(const struct 
in6_addr *ip) {
 static inline struct in6_addr
 in6_addr_mapped_ipv4(ovs_be32 ip4)
 {
-struct in6_addr ip6 = { .s6_addr = { [10] = 0xff, [11] = 0xff } };
+struct in6_addr ip6;
+memset(, 0, sizeof(ip6));
+ip6.s6_addr[10] = 0xff, ip6.s6_addr[11] = 0xff;
 memcpy(_addr[12], , 4);
 return ip6;
 }
diff --git a/lib/util.h b/lib/util.h
index 764e0a08a429..3c43c2c355e2 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -31,7 +31,7 @@
 extern char *program_name;
 
 #define __ARRAY_SIZE_NOCHECK(ARRAY) (sizeof(ARRAY) / sizeof((ARRAY)[0]))
-#ifdef __GNUC__
+#if __GNUC__ && !defined(__cplusplus)
 /* return 0 for array types, 1 otherwise */
 #define __ARRAY_CHECK(ARRAY)   \
 !__builtin_types_compatible_p(typeof(ARRAY), typeof([0]))
@@ -41,6 +41,19 @@ extern char *program_name;
 #define __ARRAY_SIZE(ARRAY)\
 __builtin_choose_expr(__ARRAY_CHECK(ARRAY),\
 __ARRAY_SIZE_NOCHECK(ARRAY), __ARRAY_FAIL(ARRAY))
+#elif defined(__cplusplus)
+#define __ARRAY_SIZE(ARRAY) ( \
+   0 * sizeof(reinterpret_cast(ARRAY)) + \
+   0 * sizeof(::Bad_arg_to_ARRAY_SIZE::check_type((ARRAY), &(ARRAY))) + \
+   sizeof(ARRAY) / sizeof((ARRAY)[0]) )
+
+struct Bad_arg_to_ARRAY_SIZE {
+   class Is_pointer;
+   class Is_array {};
+   template 
+   static Is_pointer check_type(const T *, const T * const *);
+   static Is_array check_type(const void *, const void *);
+};
 #else
 #define __ARRAY_SIZE(ARRAY) __ARRAY_SIZE_NOCHECK(ARRAY)
 #endif
-- 
2.7.4

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

Re: [ovs-dev] [PATCH 2/2] OVN: Add support for periodic router advertisements.

[Ben Pfaff]

On Wed, Oct 18, 2017 at 11:53:49AM -0500, Mark Michelson wrote:
> This change adds three new options to the Northbound
> Logical_Router_Port's ipv6_ra_configs option:
> 
> * send_periodic: If set to "true", then OVN will send periodic router
> advertisements out of this router port.
> * max_interval: The maximum amount of time to wait between sending
> periodic router advertisements.
> * min_interval: The minimum amount of time to wait between sending
> periodic router advertisements.
> 
> When send_periodic is true, then IPv6 RA configs, as well as some layer
> 2 and layer 3 information about the router port, are copied to the
> southbound database. From there, ovn-controller can use this information
> to know when to send periodic RAs and what to send in them.
> 
> Because periodic RAs originate from each ovn-controller, the new
> keep-local flag is set on the packet so that ports don't receive an
> overabundance of RAs.
> 
> Signed-off-by: Mark Michelson 

Thanks a lot for working on this.  It addresses a feature we've been
missing for a long time.

I get several application failures:

   Applying: OVN: Add support for periodic router advertisements.
   error: patch failed: ovn/controller/pinctrl.c:88
   error: ovn/controller/pinctrl.c: patch does not apply
   error: patch failed: ovn/northd/ovn-northd.c:4375
   error: ovn/northd/ovn-northd.c: patch does not apply
   error: patch failed: ovn/ovn-nb.xml:1369
   error: ovn/ovn-nb.xml: patch does not apply
   Patch failed at 0001 OVN: Add support for periodic router advertisements.

Would you mind rebasing and reposting?  Thanks a lot.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v4] ovn: Check for known logical switch port types.

[Ben Pfaff]

On Wed, Sep 06, 2017 at 10:31:28AM -0400, Lance Richardson wrote:
> > From: "Mark Michelson" 
> > To: d...@openvswitch.org
> > Sent: Wednesday, September 6, 2017 9:38:40 AM
> > Subject: [ovs-dev] [PATCH v4] ovn: Check for known logical switch port 
> > types.
> > 
> > OVN is lenient with the types of logical switch ports. Maybe too
> > lenient. This patch attempts to solve this problem on two fronts:
> > 
> > 1) In ovn-nbctl, if you attempt to set the port type to an unknown
> > type, the command will not end up setting the type.
> > 2) In northd, when copying the port type from the northbound database to
> > the corresponding port-binding in the southbound database, a warning
> > will be issued if the port is of an unknown type.
> > 
> > Signed-off-by: Mark Michelson 
> > ---
> > v4:
> >   * Fixed failing ovn-controller-vtep test that attempted to
> > set a switch port type to "void"
> > v3:
> >   * OVN_NB_LSP_TYPES declaration is static
> >   * northd will not copy unknown port types to southbound DB
> >   * re-ordered port types in OVN_NB_LSP_TYPES
> > v2:
> >   * Used ARRAY_SIZE to calculate length of OVN_NB_LSP_TYPES
> > v1:
> >   * Initial patch
> > ---
> 
> LGTM.
> 
> Acked-by: Lance Richardson 

Thanks Mark and Lance.

I applied this to master.  I changed the VLOG_WARN in ovn-northd.c to
VLOG_WARN_RL, so that it gets rate-limited, but I otherwise left it as
is.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] Desarrollo Sustentable

[importancia a nivel mundial]

Conozca los ODS y establezca alianzas con las empresas del futuro

Los Objetivos del Desarrollo Sustentable y su importancia en los negocios
16 de noviembre - Mtra. Guillermina Barrera Zaragoza9am-6pm

En septiembre de 2015, se llevó a cabo la Cumbre para el Desarrollo Sostenible, 
en la cual los Estados Miembros de la ONU aprobaron la Agenda 2030 para el 
Desarrollo Sostenible, que incluye un conjunto de 17 Objetivos de Desarrollo 
Sostenible (ODS) para poner fin a la pobreza, luchar contra la desigualdad y la 
injusticia y hacer frente al cambio climático, asegurando la prosperidad para 
todos como parte de una nueva agenda de desarrollo sostenible. Es importante 
saber que será necesario contar con iniciativas que deben ir de la mano de 
estrategias que favorezcan el crecimiento económico, aborden una serie de 
necesidades sociales (educación, salud y oportunidades de empleo entre otras), 
luchen contra el cambio climático y promueven la protección del medio ambiente. 

BENEFICIOS DE ASISTIR: 

a) Conocerá los ODS y su importancia a nivel mundial b) Evaluará la importancia 
que revisten los ODS para su empresa, entorno social y medio ambiente global y 
local. c) Identificará proyectos a desarrollar, relacionados con los ODS en su 
empresa o cadena de valor. d) Considerará una idea de proyecto alcanzable e) 
Analizará los riesgos en la empresa al no implementar dichos objetivos 

¿Requiere la información a la Brevedad? responda este email con la palabra: 
Desarrollo + nombre - teléfono - correo.


centro telefónico:018002120744


 


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

Re: [ovs-dev] [PATCH v5 1/2] nsh: add new flow key 'ttl'

[Ben Pfaff]

On Wed, Aug 30, 2017 at 02:21:01AM +0800, Yi Yang wrote:
> IETF NSH draft will be approved by end of August, NSH header
> format has been finalized and won't be change anymore, so we
> need to follow this final spec to implement nsh.
> 
> kernel data path also needs finalized uAPIs, they can't be
> changed once they are merged.
> 
> This patch adds new nsh key 'ttl', bits of flags and mdtype
> fields are also changed to follow the final spec.
> 
> A new action dec_nsh_ttl will be added in the following patch.
> 
> Signed-off-by: Yi Yang 

It's time to take another look at NSH, by adding support for the latest
draft.  I posted a patch to kick this off:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-November/340365.html

This is the sort of patch that could be backported to 2.8 if that is
desirable.

After we get the protocol format fixed, I suggest rebasing and reposting
these patches so that we can get enhanced support into 2.9.

Thanks,

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

[ovs-dev] [PATCH] nsh: Fix packet format to match IETF NSH draft.

[Ben Pfaff]

The NSH draft added a TTL field.  This adds basic support.

CC: Yi Yang 
CC: Jan Scheurich 
Signed-off-by: Ben Pfaff 
---
 include/openvswitch/nsh.h | 11 +++
 lib/flow.c|  2 +-
 lib/odp-execute.c | 10 +-
 lib/packets.c |  4 +++-
 4 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/include/openvswitch/nsh.h b/include/openvswitch/nsh.h
index a3611d0896b2..4d83bad1af81 100644
--- a/include/openvswitch/nsh.h
+++ b/include/openvswitch/nsh.h
@@ -62,7 +62,7 @@ struct nsh_md2_tlv {
 };
 
 struct nsh_hdr {
-ovs_be16 ver_flags_len;
+ovs_be16 ver_flags_ttl_len;
 uint8_t md_type;
 uint8_t next_proto;
 ovs_16aligned_be32 path_hdr;
@@ -75,8 +75,10 @@ struct nsh_hdr {
 /* Masking NSH header fields. */
 #define NSH_VER_MASK   0xc000
 #define NSH_VER_SHIFT  14
-#define NSH_FLAGS_MASK 0x3fc0
-#define NSH_FLAGS_SHIFT6
+#define NSH_FLAGS_MASK 0x3000
+#define NSH_FLAGS_SHIFT12
+#define NSH_TTL_MASK   0x0fc0
+#define NSH_TTL_SHIFT  6
 #define NSH_LEN_MASK   0x003f
 #define NSH_LEN_SHIFT  0
 
@@ -113,7 +115,8 @@ struct nsh_hdr {
 static inline uint16_t
 nsh_hdr_len(const struct nsh_hdr *nsh)
 {
-return ((ntohs(nsh->ver_flags_len) & NSH_LEN_MASK) >> NSH_LEN_SHIFT) << 2;
+return ((ntohs(nsh->ver_flags_ttl_len) & NSH_LEN_MASK)
+>> NSH_LEN_SHIFT) << 2;
 }
 
 static inline struct nsh_md1_ctx *
diff --git a/lib/flow.c b/lib/flow.c
index 4d2b7747a124..57b6c597d207 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -546,7 +546,7 @@ parse_nsh(const void **datap, size_t *sizep, struct 
flow_nsh *key)
 
 memset(key, 0, sizeof(struct flow_nsh));
 
-ver_flags_len = ntohs(nsh->ver_flags_len);
+ver_flags_len = ntohs(nsh->ver_flags_ttl_len);
 version = (ver_flags_len & NSH_VER_MASK) >> NSH_VER_SHIFT;
 flags = (ver_flags_len & NSH_FLAGS_MASK) >> NSH_FLAGS_SHIFT;
 
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index 5f4d23a91a3e..d5542bde8b02 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -279,8 +279,8 @@ odp_set_nsh(struct dp_packet *packet, const struct 
ovs_key_nsh *key,
 struct nsh_hdr *nsh = dp_packet_l3(packet);
 
 if (!mask) {
-nsh->ver_flags_len = htons(key->flags << NSH_FLAGS_SHIFT) |
- (nsh->ver_flags_len & ~htons(NSH_FLAGS_MASK));
+nsh->ver_flags_ttl_len = htons(key->flags << NSH_FLAGS_SHIFT) |
+(nsh->ver_flags_ttl_len & ~htons(NSH_FLAGS_MASK));
 put_16aligned_be32(>path_hdr, key->path_hdr);
 switch (nsh->md_type) {
 case NSH_M_TYPE1:
@@ -294,11 +294,11 @@ odp_set_nsh(struct dp_packet *packet, const struct 
ovs_key_nsh *key,
 break;
 }
 } else {
-uint8_t flags = (ntohs(nsh->ver_flags_len) & NSH_FLAGS_MASK) >>
+uint8_t flags = (ntohs(nsh->ver_flags_ttl_len) & NSH_FLAGS_MASK) >>
 NSH_FLAGS_SHIFT;
 flags = key->flags | (flags & ~mask->flags);
-nsh->ver_flags_len = htons(flags << NSH_FLAGS_SHIFT) |
- (nsh->ver_flags_len & ~htons(NSH_FLAGS_MASK));
+nsh->ver_flags_ttl_len = htons(flags << NSH_FLAGS_SHIFT) |
+(nsh->ver_flags_ttl_len & ~htons(NSH_FLAGS_MASK));
 
 ovs_be32 path_hdr = get_16aligned_be32(>path_hdr);
 path_hdr = key->path_hdr | (path_hdr & ~mask->path_hdr);
diff --git a/lib/packets.c b/lib/packets.c
index 74d87eda89e1..a26b6c33e41c 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -427,7 +427,9 @@ encap_nsh(struct dp_packet *packet, const struct 
ovs_action_encap_nsh *encap)
 }
 
 nsh = (struct nsh_hdr *) dp_packet_push_uninit(packet, length);
-nsh->ver_flags_len = htons(encap->flags << NSH_FLAGS_SHIFT | length >> 2);
+nsh->ver_flags_ttl_len = htons((encap->flags << NSH_FLAGS_SHIFT)
+   | (63 << NSH_TTL_SHIFT)
+   | ((length >> 2) << NSH_LEN_SHIFT));
 nsh->next_proto = next_proto;
 put_16aligned_be32(>path_hdr, encap->path_hdr);
 nsh->md_type = encap->mdtype;
-- 
2.10.2

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

Re: [ovs-dev] [PATCH 2/2] lib/netlink: Use correct netlink max message size

[Greg Rose]

On 11/01/2017 01:33 PM, Ben Pfaff wrote:

On Wed, Nov 01, 2017 at 01:17:04PM -0700, Greg Rose wrote:

On 11/01/2017 01:01 PM, Ben Pfaff wrote:

On Wed, Nov 01, 2017 at 10:33:25AM -0700, Greg Rose wrote:

On 11/01/2017 09:38 AM, Ben Pfaff wrote:

On Wed, Nov 01, 2017 at 08:28:48AM -0700, Greg Rose wrote:

On 10/31/2017 12:53 PM, Ben Pfaff wrote:

On Fri, Sep 22, 2017 at 07:44:53AM -0700, Greg Rose wrote:

The maximum message size for recent Linux kernels is 32Kb and in older
kernels it is 16KB.

See http://www.spinics.net/lists/netdev/msg431592.html

Adjust the size checked and update a comment.

Signed-off-by: Greg Rose 


...


diff --git a/lib/netlink.c b/lib/netlink.c
index de3ebcd..04310ff 100644
--- a/lib/netlink.c
+++ b/lib/netlink.c
@@ -570,7 +570,7 @@ nl_msg_next(struct ofpbuf *buffer, struct ofpbuf *msg)
   bool
   nl_attr_oversized(size_t payload_size)
   {
-return payload_size > UINT16_MAX - NLA_HDRLEN;
+return payload_size > INT16_MAX - NLA_HDRLEN;
   }


Thanks for the patch!

I am confused by a difference between the commit message and the code.
Before this patch, nl_attr_oversized() considered an attribute of about
64 kB to be oversize; after this patch, about 32 kB.  Shouldn't the new
value be about 16 kB?

Thanks,

Ben.



IIRC this is in user space so we prepare for whatever the maximum size we might
get from a kernel, which is 32KB.

We could have just left it at 64KB but we don't ever need that much.


This response implies that nl_attr_oversized() has something to do with
Netlink attributes for messages that userspace receives from the kernel.
This is not the case.  OVS userspace is prepared to handle any length
attribute in Netlink messages that it receives.  It won't apply
nl_attr_oversized() to received attributes, it will just go ahead and
process them.

On the contrary, userspace uses nl_attr_oversized() to limit the maximum
size of a Netlink attribute it *sends to* the kernel.  If the kernel
only supports a maximum 16 kB or 32 kB attribute, according to its
version, then the function needs to apply this limit.  The current code
assumes that the kernel can process anything that can actually be
formulated into Netlink.

What actual limit does Linux apply to Netlink messages, for sending and
receiving?  I am beginning to believe that this function is a red
herring and does not need to change.


So you're right, I didn't recall correctly... that's what I get for quick 
answers.

In net/netlink/af_netlink.c the core netlink handler will attempt  to copy the
maximum length of the message and if it is not able to then it will set the
MSG_TRUNC flag and attempt to process whatever it can.  The netlink
skb's are processed as datagrams.

Generally the large size messages come from the kernel, not to the kernel
so it is not much of a concern for any applications I'm aware of.

Does OVS send large messages to the kernel?


One example is when OVS sends a large number of actions as part of
OVS_FLOW_CMD_NEW or OVS_FLOW_CMD_SET or OVS_PACKET_CMD_EXECUTE.  OVS has
a fallback for that, but it needs to know when to use it, that is, it
needs to know how large is too large.



I see.  Then we should probably use libnl's nl_socket_set_buffer_size() to
try setting 32KB buffers and then fall back to 16KB buffers if the call
isn't successful.

http://www.infradead.org/~tgr/libnl/doc/api/group__socket.html#gaec9b1f3b0fdbf4e6e0fb10a233b5df68

I'm in the middle of some other stuff right now, some internal bugs, so
when I'm done I'll try out a patch to see if it works.


I don't see the value of changing the sndbuf value.  The default is more
than 32 kB.

As far as I can tell from net/core/af_netlink.c, it should work fine to
send a 64 kB netlink message to the kernel, so I don't think anything
needs to change in OVS userspace.



That works for me.

Thanks,

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

Re: [ovs-dev] [PATCH 2/2] lib/netlink: Use correct netlink max message size

[Ben Pfaff]

On Wed, Nov 01, 2017 at 01:17:04PM -0700, Greg Rose wrote:
> On 11/01/2017 01:01 PM, Ben Pfaff wrote:
> >On Wed, Nov 01, 2017 at 10:33:25AM -0700, Greg Rose wrote:
> >>On 11/01/2017 09:38 AM, Ben Pfaff wrote:
> >>>On Wed, Nov 01, 2017 at 08:28:48AM -0700, Greg Rose wrote:
> On 10/31/2017 12:53 PM, Ben Pfaff wrote:
> >On Fri, Sep 22, 2017 at 07:44:53AM -0700, Greg Rose wrote:
> >>The maximum message size for recent Linux kernels is 32Kb and in older
> >>kernels it is 16KB.
> >>
> >>See http://www.spinics.net/lists/netdev/msg431592.html
> >>
> >>Adjust the size checked and update a comment.
> >>
> >>Signed-off-by: Greg Rose 
> >
> >...
> >
> >>diff --git a/lib/netlink.c b/lib/netlink.c
> >>index de3ebcd..04310ff 100644
> >>--- a/lib/netlink.c
> >>+++ b/lib/netlink.c
> >>@@ -570,7 +570,7 @@ nl_msg_next(struct ofpbuf *buffer, struct ofpbuf 
> >>*msg)
> >>   bool
> >>   nl_attr_oversized(size_t payload_size)
> >>   {
> >>-return payload_size > UINT16_MAX - NLA_HDRLEN;
> >>+return payload_size > INT16_MAX - NLA_HDRLEN;
> >>   }
> >
> >Thanks for the patch!
> >
> >I am confused by a difference between the commit message and the code.
> >Before this patch, nl_attr_oversized() considered an attribute of about
> >64 kB to be oversize; after this patch, about 32 kB.  Shouldn't the new
> >value be about 16 kB?
> >
> >Thanks,
> >
> >Ben.
> >
> 
> IIRC this is in user space so we prepare for whatever the maximum size we 
> might
> get from a kernel, which is 32KB.
> 
> We could have just left it at 64KB but we don't ever need that much.
> >>>
> >>>This response implies that nl_attr_oversized() has something to do with
> >>>Netlink attributes for messages that userspace receives from the kernel.
> >>>This is not the case.  OVS userspace is prepared to handle any length
> >>>attribute in Netlink messages that it receives.  It won't apply
> >>>nl_attr_oversized() to received attributes, it will just go ahead and
> >>>process them.
> >>>
> >>>On the contrary, userspace uses nl_attr_oversized() to limit the maximum
> >>>size of a Netlink attribute it *sends to* the kernel.  If the kernel
> >>>only supports a maximum 16 kB or 32 kB attribute, according to its
> >>>version, then the function needs to apply this limit.  The current code
> >>>assumes that the kernel can process anything that can actually be
> >>>formulated into Netlink.
> >>>
> >>>What actual limit does Linux apply to Netlink messages, for sending and
> >>>receiving?  I am beginning to believe that this function is a red
> >>>herring and does not need to change.
> >>>
> >>So you're right, I didn't recall correctly... that's what I get for quick 
> >>answers.
> >>
> >>In net/netlink/af_netlink.c the core netlink handler will attempt  to copy 
> >>the
> >>maximum length of the message and if it is not able to then it will set the
> >>MSG_TRUNC flag and attempt to process whatever it can.  The netlink
> >>skb's are processed as datagrams.
> >>
> >>Generally the large size messages come from the kernel, not to the kernel
> >>so it is not much of a concern for any applications I'm aware of.
> >>
> >>Does OVS send large messages to the kernel?
> >
> >One example is when OVS sends a large number of actions as part of
> >OVS_FLOW_CMD_NEW or OVS_FLOW_CMD_SET or OVS_PACKET_CMD_EXECUTE.  OVS has
> >a fallback for that, but it needs to know when to use it, that is, it
> >needs to know how large is too large.
> >
> 
> I see.  Then we should probably use libnl's nl_socket_set_buffer_size() to
> try setting 32KB buffers and then fall back to 16KB buffers if the call
> isn't successful.
> 
> http://www.infradead.org/~tgr/libnl/doc/api/group__socket.html#gaec9b1f3b0fdbf4e6e0fb10a233b5df68
> 
> I'm in the middle of some other stuff right now, some internal bugs, so
> when I'm done I'll try out a patch to see if it works.

I don't see the value of changing the sndbuf value.  The default is more
than 32 kB.

As far as I can tell from net/core/af_netlink.c, it should work fine to
send a 64 kB netlink message to the kernel, so I don't think anything
needs to change in OVS userspace.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] datapath: Remove redundant check for IFF_NO_QUEUE

[Ben Pfaff]

On Wed, Nov 01, 2017 at 08:43:20AM -0700, Greg Rose wrote:
> On 10/30/2017 06:13 PM, fukaige wrote:
> >From: Kaige Fu 
> >
> >IFF_NO_QUEUE is checked twice. This cause a situation
> >that HAVE_IFF_NO_QUEUE is defined no matter whether
> >the flag IFF_NO_QUEUE is defined in kernel or not.
> >
> >Signed-off-by: Kaige Fu 
> >---
> >  datapath/linux/compat/include/linux/netdevice.h | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> >diff --git a/datapath/linux/compat/include/linux/netdevice.h 
> >b/datapath/linux/compat/include/linux/netdevice.h
> >index a5d4ee8..3c3cf42 100644
> >--- a/datapath/linux/compat/include/linux/netdevice.h
> >+++ b/datapath/linux/compat/include/linux/netdevice.h
> >@@ -22,9 +22,6 @@ struct net;
> >  #define IFF_LIVE_ADDR_CHANGE 0
> >  #endif
> >-#ifndef IFF_NO_QUEUE
> >-#define IFF_NO_QUEUE0
> >-#endif
> >  #ifndef IFF_OPENVSWITCH
> >  #define IFF_OPENVSWITCH 0
> >  #endif
> >
> 
> Looks correct to me.
> 
> Reviewed-by: Greg Rose 

Thanks Kaige and Greg.  I applied this to master.  Let me know if it
should be backported.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCHv2 2/2] ovn-sbctl: Fix possible null pointer to qsort.

[Ben Pfaff]

On Tue, Oct 31, 2017 at 05:29:51PM -0700, William Tu wrote:
> Clang reports possible null pointer 'lflows' passed to qsort.
> This is due to the checker unable to make sure whether 'lflows'
> gets malloc or not in the previous loop.  Fix it by checking the
> 'n_flows' before calling qsort.
> 
> Signed-off-by: William Tu 

Thanks for the patches!  I applied both of these to master.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCHv2] acinclude: Fix SKB_GSO_UDP check.

[Ben Pfaff]

On Tue, Oct 31, 2017 at 05:03:41PM -0700, William Tu wrote:
> The HAVE_SKB_GSO_UDP checks whether skbuff.h defines SKB_GSO_UDP.
> However, it falsely returns yes because grep matches SKB_GSO_UDP_TUNNEL.
> Thus, add space character '[:space:]' before and after it.
> 
> Fixes: ad283644f0e4 ("acinclude: Check for SKB_GSO_UDP")
> Signed-off-by: William Tu 
> Cc: Greg Rose 
> ---
> v1->v2
>   remove using "grep -w" since -w is not POSIX

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

[ovs-dev] [PATCHv2] dpif-netlink-rtnl: Fix ovs_geneve probing after restart.

[William Tu]

When using the out-of-tree (openvswitch compat) geneve module,
the first time oot tunnel probing returns true (correct).
Without unloading the geneve module, if the userspace ovs-vswitchd
restarts, because the 'geneve_sys_6081' still exists, the probing
incorrectly returns false and loads the in-tree (upstream kernel)
geneve module.

The patch fixes it by querying the geneve device's kind when exists.
The out-of-tree modules uses kind string as 'ovs_geneve', while the
in-tree module uses 'geneve'.  To reproduce the issue, start the ovs
> /etc/init.d/openvswitch-switch start
> creat a bridge and attach a geneve port using out-of-tree geneve
> /etc/init.d/openvswitch-switch restart

Fixes: 921c370a9df5 ("dpif-netlink: Probe for out-of-tree tunnels, decides used 
interface")
Signed-off-by: William Tu 
Cc: Eric Garver 
Cc: Gurucharan Shetty 
---
v1->v2:
Add detection of existing module, instead of unconditionally
remote it and create.
---
 lib/dpif-netlink-rtnl.c | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
index 0c32e7d8ccb4..b19b862d308b 100644
--- a/lib/dpif-netlink-rtnl.c
+++ b/lib/dpif-netlink-rtnl.c
@@ -440,6 +440,7 @@ dpif_netlink_rtnl_probe_oot_tunnels(void)
 
 error = netdev_open("ovs-system-probe", "geneve", );
 if (!error) {
+struct ofpbuf *reply;
 const struct netdev_tunnel_config *tnl_cfg;
 
 tnl_cfg = netdev_get_tunnel_config(netdev);
@@ -448,6 +449,36 @@ dpif_netlink_rtnl_probe_oot_tunnels(void)
 }
 
 name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);
+
+/* The geneve module exists when ovs-vswitchd crashes
+ * and restarts, handle the case here.
+ */
+error = dpif_netlink_rtnl_getlink(name, );
+if (!error) {
+
+struct nlattr *linkinfo[ARRAY_SIZE(linkinfo_policy)];
+struct nlattr *rtlink[ARRAY_SIZE(rtlink_policy)];
+const char *kind;
+
+nl_policy_parse(reply, NLMSG_HDRLEN + sizeof(struct ifinfomsg),
+rtlink_policy, rtlink,
+ARRAY_SIZE(rtlink_policy));
+nl_parse_nested(rtlink[IFLA_LINKINFO], linkinfo_policy,
+linkinfo, ARRAY_SIZE(linkinfo_policy));
+kind = nl_attr_get_string(linkinfo[IFLA_INFO_KIND]);
+
+if (!strcmp(kind, "ovs_geneve")) {
+out_of_tree = true;
+goto exit;
+} else if (!strcmp(kind, "geneve")) {
+out_of_tree = false;
+goto exit;
+} else {
+VLOG_ABORT("Geneve tunnel device %s with kind %s"
+   " not supported", name, kind);
+}
+}
+
 error = dpif_netlink_rtnl_create(tnl_cfg, name, OVS_VPORT_TYPE_GENEVE,
  "ovs_geneve",
  (NLM_F_REQUEST | NLM_F_ACK
@@ -458,6 +489,10 @@ dpif_netlink_rtnl_probe_oot_tunnels(void)
 }
 out_of_tree = true;
 }
+}
+
+exit:
+if (!error) {
 netdev_close(netdev);
 }
 
-- 
2.7.4

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

[ovs-dev] [RFC PATCH 2/2] Adding configuration option to whitelist DPDK physical ports.

[Sugesh Chandran]

Adding a OVS configuration option to whitelist DPDK physical ports. By default
running multiple instances of DPDK on a single platform cannot use physical
ports at the same time even though they are distinct.

The eal init scans all the ports that are bound to DPDK and initialize the
drivers accordingly. This happens for every DPDK process init.
On a multi instance deployment usecase, it causes issues for using physical
NIC ports.
Consider a two DPDK process that are running on a single platform,
the second DPDK primary process will try to initialize the drivers for all the
physical ports even though it may be used in first DPDK process.

To avoid this situation user can whitelist the ports for each DPDK application.
Whitelisting of ports/PCI-ID in a DPDK process will limit the eal-init only on
those ports.

To whitelist two physical ports ":06:00.0" and ":06:00.1", the
configuration option in OVS would be
  ovs-vsctl  set Open_vSwitch . 
other_config:dpdk-whitelist-pci-ids=":06:00.0,:06:00.1"

To update the whitelist ports, OVS daemon has to be restarted.

Signed-off-by: Sugesh Chandran 
---
 lib/dpdk.c   | 29 +
 vswitchd/vswitch.xml | 21 +
 2 files changed, 50 insertions(+)

diff --git a/lib/dpdk.c b/lib/dpdk.c
index cbbf28d..ea7400f 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -336,6 +336,34 @@ dpdk_isolate_rte_mem_config(const struct smap 
*ovs_other_config,
 }
 
 static void
+dpdk_whitelist_pci_ids(const struct smap *ovs_other_config, char ***argv,
+   int *argc)
+{
+const char *pci_ids;
+char *pci_dev;
+int len;
+int i;
+pci_ids = smap_get(ovs_other_config, "dpdk-whitelist-pci-ids");
+if (!pci_ids) {
+return;
+}
+len = strlen(pci_ids);
+do {
+i = strcspn(pci_ids, ",");
+pci_dev = xmemdup0(pci_ids, i);
+if (!strlen(pci_dev)) {
+ break;
+}
+*argv = grow_argv(argv, *argc, 2);
+(*argv)[(*argc)++] = xstrdup("-w");
+(*argv)[(*argc)++] = pci_dev;
+i++;
+pci_ids += i;
+len -= i;
+} while (pci_ids && len > 0);
+}
+
+static void
 dpdk_init__(const struct smap *ovs_other_config)
 {
 char **argv = NULL, **argv_to_release = NULL;
@@ -422,6 +450,7 @@ dpdk_init__(const struct smap *ovs_other_config)
 }
 
 dpdk_isolate_rte_mem_config(ovs_other_config, , );
+dpdk_whitelist_pci_ids(ovs_other_config, , );
 argv = grow_argv(, argc, 1);
 argv[argc] = NULL;
 
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 3a507ea..04baf97 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -442,6 +442,27 @@
 
   
 
+  
+
+  Specifies list of pci-ids separated by , for whitelisting available
+  physical NIC ports in OVS. The option valid only when DPDK is enabled
+  in OVS and the ports are already bound to DPDK userspace driver.
+
+
+  By default, all the DPDK bound ports are initialized at the time of
+  vswitchd start. Whitelisting a list of pci-ids is used to limit the
+  initialization only to specific ports. Changing this value requires
+  restarting the daemon.
+
+
+ This option allow user to run multiple instance of DPDK including
+ vswitchd simultaneously by exclusively allocating the physical NIC
+ ports between them. Its impossible to use the NIC ports in
+ multiple primary DPDK processes without whitelisting them
+ even the application is using distinct ports.
+
+  
+
 
 
 
-- 
2.7.4

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

[ovs-dev] [RFC PATCH 1/2] Adding DPDK configuration option to isolate rte-mempool allocation.

[Sugesh Chandran]

DPDK allocate memory regions at the time of vswitchd init. To run multiple
primary instance of DPDK including OVS on a single platform, the memory map
regions in the filesystem should be distinct.

The new configuration option let user to enable the memory isolation in need.
By default, OVS uses default dpdk memory regions.

To isolate the memory regions, DPDK prefix the memory map files with user
input string. This implementation uses the pid of vswitchd process as a memory
map prefix, because its unique in the platform.

For eg: a vswitchd process with pid '12345' create memory map regions with
prefix 'ovs-12345' in the filesystem.

The following configuration option is used to enable the feature and changing
this value requires restarting the daemon.

 ovs-vsctl --no-wait set Open_vSwitch . other_config:dpdk-isolate-mem=true

Signed-off-by: Sugesh Chandran 
---
 lib/daemon.c |  9 +
 lib/daemon.h |  1 +
 lib/dpdk.c   | 34 ++
 vswitchd/vswitch.xml | 23 +++
 4 files changed, 67 insertions(+)

diff --git a/lib/daemon.c b/lib/daemon.c
index 3249c5a..3aa61e1 100644
--- a/lib/daemon.c
+++ b/lib/daemon.c
@@ -66,6 +66,15 @@ set_pidfile(const char *name)
 pidfile = make_pidfile_name(name);
 }
 
+/*
+ * Return the name of pidfile, NULL when its none.
+ */
+const char *
+get_pidfile(void)
+{
+return pidfile;
+}
+
 /* Disables self confinement. */
 void
 daemon_disable_self_confinement(void)
diff --git a/lib/daemon.h b/lib/daemon.h
index bfa0640..1df156e 100644
--- a/lib/daemon.h
+++ b/lib/daemon.h
@@ -157,5 +157,6 @@ void service_stop(void);
 bool should_service_stop(void);
 void set_pidfile(const char *name);
 void close_standard_fds(void);
+const char *get_pidfile(void);
 
 #endif /* daemon.h */
diff --git a/lib/dpdk.c b/lib/dpdk.c
index 8da6c32..cbbf28d 100644
--- a/lib/dpdk.c
+++ b/lib/dpdk.c
@@ -35,6 +35,7 @@
 #include "openvswitch/dynamic-string.h"
 #include "openvswitch/vlog.h"
 #include "smap.h"
+#include "daemon.h"
 
 VLOG_DEFINE_THIS_MODULE(dpdk);
 
@@ -302,6 +303,38 @@ static cookie_io_functions_t dpdk_log_func = {
 .write = dpdk_log_write,
 };
 
+/*
+ * Isolate the dpdk rte_memory pool for the vswitch process.
+ * The isolation is achieved by using a specific prefix for rte memory maps.
+ * the prefix is created from the pid of the vswitchd. This allows to run
+ * multiple ovs-dpdk instance on same platform in need.
+ */
+static void
+dpdk_isolate_rte_mem_config(const struct smap *ovs_other_config,
+char ***argv, int *argc)
+{
+if (smap_get_bool(ovs_other_config, "dpdk-isolate-mem", false)) {
+/*
+ * pid file is present only on Linux distribution, so DPDK?
+ */
+pid_t pid;
+const char *pidfile;
+pidfile = get_pidfile();
+if (!pidfile) {
+VLOG_INFO("Error : Cannot find ovs-vswitchd pidfile");
+return;
+}
+pid = read_pidfile(pidfile);
+if (pid < 0) {
+VLOG_INFO("Error : Invalid pid in the ovs-vswitchd pidfile");
+return;
+}
+*argv = grow_argv(argv, *argc, 2);
+(*argv)[(*argc)++] = xstrdup("--file-prefix");
+(*argv)[(*argc)++] = xasprintf("%s-%X", "ovs", (int)pid);
+}
+}
+
 static void
 dpdk_init__(const struct smap *ovs_other_config)
 {
@@ -388,6 +421,7 @@ dpdk_init__(const struct smap *ovs_other_config)
 }
 }
 
+dpdk_isolate_rte_mem_config(ovs_other_config, , );
 argv = grow_argv(, argc, 1);
 argv[argc] = NULL;
 
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index d7f6839..3a507ea 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -419,6 +419,29 @@
   VLAN.
 
   
+
+  
+
+  Set this value to true to isolate DPDK rte mempool at
+  the time of allocation. The option is valid only when DPDK is enabled
+  in OVS.
+
+
+  The default value is false. Changing this value requires
+  restarting the daemon.
+
+
+  The rte_mempool map is created at the time of vswitchd init and its
+  been used by all DPDK processes including vswitchd in the platform.
+  dpdk-isolate-mem option let vswitchd to use a separate
+  memory region than other processes for isolation. The separate
+  memory region map are prefixed by ovs-pid
+  in the filesystem. pid is the process id of vswitchd
+  process.
+
+  
+
 
 
 
-- 
2.7.4

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

[ovs-dev] [RFC PATCH 0/2] Adding ovs configuration options to run multiple DPDK instances on a single platform.

[Sugesh Chandran]

The current OVS-DPDK implementation doesnt allow users to run another DPDK
application simultaneously on the same platform.

To avoid this limitation, the patch series offers two configuration options
to isolate the OVS-DPDK process completely on a platform.

1) The configuration option is added to isolate the DPDK memory allocation in
need. This option allocate and use a seperate DPDK memory pool region for OVS,
instead of standard DPDK memory pool.

2) Configuration option to whitelist the ports that are going to use in OVS.
This option causes DPDK to init only specific ports and its drivers at the time
of vswitchd init.

Sugesh Chandran (2):
  Adding DPDK configuration option to isolate rte-mempool allocation.
  Adding configuration option to whitelist DPDK physical ports.

 lib/daemon.c |  9 
 lib/daemon.h |  1 +
 lib/dpdk.c   | 63 
 vswitchd/vswitch.xml | 44 
 4 files changed, 117 insertions(+)

-- 
2.7.4

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

Re: [ovs-dev] [PATCH 2/2] lib/netlink: Use correct netlink max message size

[Greg Rose]

On 11/01/2017 09:38 AM, Ben Pfaff wrote:

On Wed, Nov 01, 2017 at 08:28:48AM -0700, Greg Rose wrote:
> On 10/31/2017 12:53 PM, Ben Pfaff wrote:
>> On Fri, Sep 22, 2017 at 07:44:53AM -0700, Greg Rose wrote:
>>> The maximum message size for recent Linux kernels is 32Kb and in older
>>> kernels it is 16KB.
>>>
>>> See http://www.spinics.net/lists/netdev/msg431592.html
>>>
>>> Adjust the size checked and update a comment.
>>>
>>> Signed-off-by: Greg Rose 
>>
>> ...
>>
>>> diff --git a/lib/netlink.c b/lib/netlink.c
>>> index de3ebcd..04310ff 100644
>>> --- a/lib/netlink.c
>>> +++ b/lib/netlink.c
>>> @@ -570,7 +570,7 @@ nl_msg_next(struct ofpbuf *buffer, struct ofpbuf *msg)
>>>   bool
>>>   nl_attr_oversized(size_t payload_size)
>>>   {
>>> -return payload_size > UINT16_MAX - NLA_HDRLEN;
>>> +return payload_size > INT16_MAX - NLA_HDRLEN;
>>>   }
>>
>> Thanks for the patch!
>>
>> I am confused by a difference between the commit message and the code.
>> Before this patch, nl_attr_oversized() considered an attribute of about
>> 64 kB to be oversize; after this patch, about 32 kB.  Shouldn't the new
>> value be about 16 kB?
>>
>> Thanks,
>>
>> Ben.
>>
>
> IIRC this is in user space so we prepare for whatever the maximum size we 
might
> get from a kernel, which is 32KB.
>
> We could have just left it at 64KB but we don't ever need that much.

This response implies that nl_attr_oversized() has something to do with
Netlink attributes for messages that userspace receives from the kernel.
This is not the case.  OVS userspace is prepared to handle any length
attribute in Netlink messages that it receives.  It won't apply
nl_attr_oversized() to received attributes, it will just go ahead and
process them.

On the contrary, userspace uses nl_attr_oversized() to limit the maximum
size of a Netlink attribute it *sends to* the kernel.  If the kernel
only supports a maximum 16 kB or 32 kB attribute, according to its
version, then the function needs to apply this limit.  The current code
assumes that the kernel can process anything that can actually be
formulated into Netlink.

What actual limit does Linux apply to Netlink messages, for sending and
receiving?  I am beginning to believe that this function is a red
herring and does not need to change.


So you're right, I didn't recall correctly... that's what I get for quick 
answers.

In net/netlink/af_netlink.c the core netlink handler will attempt  to copy the
maximum length of the message and if it is not able to then it will set the
MSG_TRUNC flag and attempt to process whatever it can.  The netlink
skb's are processed as datagrams.

Generally the large size messages come from the kernel, not to the kernel
so it is not much of a concern for any applications I'm aware of.

Does OVS send large messages to the kernel?

- Greg

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

[ovs-dev] [PATCH 4/4] ofp-actions: Add compare to offsetof need for MSVC 2015/17

[Alin Gabriel Serdean]

Unfortunately starting from VS 2015, the "C" definition for `offsetof`
has been changed. Please see:
https://stackoverflow.com/questions/42725929/using-offsetof-with-enum-does-not-compile-in-visual-studio-2015/42726424

Several people reported the bug for 2015 and also 2017 (i.e. :
https://developercommunity.visualstudio.com/content/problem/22196/static-assert-cannot-compile-constexprs-method-tha.html
), but we don't have a fix yet.

This patch adds an explicit compare, although we could redefine the macro
for the same effect.

Signed-off-by: Alin Gabriel Serdean 
---
 include/openvswitch/ofp-actions.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/openvswitch/ofp-actions.h 
b/include/openvswitch/ofp-actions.h
index 03c1d11..25f61ef 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -1128,7 +1128,7 @@ void *ofpact_finish(struct ofpbuf *, struct ofpact *);
 BUILD_ASSERT_DECL(offsetof(struct STRUCT, ofpact) == 0);\
 \
 enum { OFPACT_##ENUM##_SIZE \
-   = (offsetof(struct STRUCT, MEMBER)   \
+   = (offsetof(struct STRUCT, MEMBER) != 0  \
   ? offsetof(struct STRUCT, MEMBER) \
   : OFPACT_ALIGN(sizeof(struct STRUCT))) }; \
 \
-- 
2.10.2.windows.1

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

[ovs-dev] [PATCH 2/4] windows: Add interlocked function definitions for VS 2015

[Alin Gabriel Serdean]

For some unclear and accidental reasons, the Windows 10 SDK
renamed _Interlocked* functions to _InlineInterlocked* (although the
documentation still points to the old form:
https://msdn.microsoft.com/en-us/library/191ca0sk.aspx).

This patch adds mappings for used functions.

Signed-off-by: Alin Gabriel Serdean 
---
 lib/ovs-atomic-msvc.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/lib/ovs-atomic-msvc.h b/lib/ovs-atomic-msvc.h
index 0b041c6..81f7682 100644
--- a/lib/ovs-atomic-msvc.h
+++ b/lib/ovs-atomic-msvc.h
@@ -41,6 +41,13 @@ typedef enum {
 memory_order_seq_cst
 } memory_order;
 
+#if _MSC_VER > 1800 && defined(_M_IX86)
+/* From WDK 10 _InlineInterlocked* functions are renamed to
+ * _InlineInterlocked* although the documentation does not specify it */
+#define _InterlockedExchangeAdd64 _InlineInterlockedExchangeAdd64
+#define _InterlockedExchange64 _InlineInterlockedExchange64
+#endif
+
 #define ATOMIC_BOOL_LOCK_FREE 2
 #define ATOMIC_CHAR_LOCK_FREE 2
 #define ATOMIC_SHORT_LOCK_FREE 2
-- 
2.10.2.windows.1

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

[ovs-dev] [PATCH 0/4] windows: Add support for compiling using 2015/2017

[Alin Gabriel Serdean]

This series adds support for userspace compiling on VS 2015/2017.

I would mark "ofp-actions: Add compare to offsetof need for MSVC 2015/17"
as RFC since it is not the cleanest solution.

Alin Gabriel Serdean (4):
  windows: _set_output_format is no longer required from VS2015
  windows: Add interlocked function definitions for VS 2015
  build-windows: Add check for struct timespec
  ofp-actions: Add compare to offsetof need for MSVC 2015/17

 include/openvswitch/ofp-actions.h | 2 +-
 lib/ovs-atomic-msvc.h | 7 +++
 lib/util.c| 3 +++
 m4/openvswitch.m4 | 1 +
 4 files changed, 12 insertions(+), 1 deletion(-)

-- 
2.10.2.windows.1

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

[ovs-dev] [PATCH 3/4] build-windows: Add check for struct timespec

[Alin Gabriel Serdean]

Starting from WDK 10 the structure `timespec` is defined in .

This patch adds a check for the structure to make  aware of it, so
it doesn't try to redefine the structure.

Signed-off-by: Alin Gabriel Serdean 
---
 m4/openvswitch.m4 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
index 59e1352..01d2269 100644
--- a/m4/openvswitch.m4
+++ b/m4/openvswitch.m4
@@ -143,6 +143,7 @@ AC_DEFUN([OVS_CHECK_WIN32],
   )
 
   AC_DEFINE([WIN32], [1], [Define to 1 if building on WIN32.])
+  AC_CHECK_TYPES([struct timespec], [], [], [[#include ]])
   AH_BOTTOM([#ifdef WIN32
 #include "include/windows/windefs.h"
 #endif])
-- 
2.10.2.windows.1

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

[ovs-dev] [PATCH 1/4] windows: _set_output_format is no longer required from VS2015

[Alin Gabriel Serdean]

_set_output_format is deprecated ang no longer required
starting from MSC_VER 1900 (VS 2015):
https://msdn.microsoft.com/en-us/library/bb531344(v=vs.140).aspx .

Signed-off-by: Alin Gabriel Serdean 
---
 lib/util.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/util.c b/lib/util.c
index a26cd51..9e6edd2 100644
--- a/lib/util.c
+++ b/lib/util.c
@@ -490,7 +490,10 @@ ovs_set_program_name(const char *argv0, const char 
*version)
 size_t max_len = strlen(argv0) + 1;
 
 SetErrorMode(GetErrorMode() | SEM_NOGPFAULTERRORBOX);
+#if _MSC_VER < 1900
+ /* This function is deprecated from 1900 (Visual Studio 2015) */
 _set_output_format(_TWO_DIGIT_EXPONENT);
+#endif
 
 basename = xmalloc(max_len);
 _splitpath_s(argv0, NULL, 0, NULL, 0, basename, max_len, NULL, 0);
-- 
2.10.2.windows.1

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

Re: [ovs-dev] [RFC PATCH v2 08/10] vswitch.xml: Detail vxlanipsec user interface.

[Stokes, Ian]

> On Fri, Aug 25, 2017 at 05:40:30PM +0100, Ian Stokes wrote:
> > This commit adds details to the vswitch xml regarding the use of the
> > vxlanipsec interface type. This patch is not intended for upstreaming
> > and simply seeks to solicit feedback on the user interface design of
> > the vxlanipsec port type as described in the vswitch.xml.
> >
> > This modifies the vswitch.xml with a proposed vxlanipsec interface.
> > It also provides details for the proposed interface options such as
> > SPD creation, SA creation and modification, Policy entries for the SPD
> > as well as traffic selector options for the policy.
> >
> > Signed-off-by: Ian Stokes 
> 
> Thanks for adding documentation.
> 
> Would you mind adding the documentation in the same commit that adds the
> documented feature?  We find that this makes what's going on a little
> easier to understand.

No problem, the feature being added is kind of a bloated patch and needs to be 
spit out anyhow in the next RFC, I'll combine the doc patch at that stage to 
make reviewing a little easier.

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

Re: [ovs-dev] [RFC PATCH v2 06/10] vxlanipsec: Add userspace support for vxlan ipsec.

[Stokes, Ian]

> On Fri, Aug 25, 2017 at 05:40:28PM +0100, Ian Stokes wrote:
> > This patch introduces a new tunnel port type 'vxlanipsec'. This port
> > combines vxlan tunnelling with IPsec operating in transport mode.
> >
> > Ciphering and authentication actions ares provided by a DPDK cryptodev.
> > The cryptodev operates as a vdev and is associated with the vxlan
> > tunnel port. Upon tunnel encapsulation packets are encrypted and a
> > hash digest attached to the packet as per RFC4303. Upon decapsulation
> > a packet is first verified via the hash and then decrypted.
> >
> > The cipher algorithm used is 128 AES-CBC and the authentication
> > algorithm is HMAC-SHA1-96. Note this work is in progress and is not
> > meant for upstream. It's purpose is to solicit feedback on the
> > approach and known issues flagged in the accompanying cover letter to
> the patch series.
> >
> > Signed-off-by: Ian Stokes 
> 
> Thanks a lot for working on this feature!
> 
> When I compile without dpdk enabled, I now get:
> 
> ../lib/netdev-vport.c:31:10: fatal error: 'rte_config.h' file not
> found
> ../lib/netdev-native-tnl.c:35:10: fatal error: 'rte_config.h' file not
> found "sparse" complains:
> 
> ../lib/netdev-vport.h:40:22: warning: symbol 'spi_map' was not declared.
> Should it be static?

Hi Ben, thanks for looking at this, I flagged that compilation fails without 
DPDK enabled in the cover letter (I know, a big no no, I didn't expect this 
code to be upstreamed in its current form so I thought flagging it as known in 
the cover and keeping it as RFC would be ok. 

For the purpose of this RFC my aim was to give people something to functionally 
test with, and hopefully gather opinions on issues such as the acinclude build 
steps, dependency on external libraries etc. as well as the overall design.

Any feedback you have as regards design or changes is more than welcome as I 
expect a few more RFC revisions before nailing something concrete down.

Ian
> 
> There is obviously a lot of code here to review, but I have not started on
> that yet.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/2] lib/netlink: Use correct netlink max message size

[Ben Pfaff]

On Wed, Nov 01, 2017 at 08:28:48AM -0700, Greg Rose wrote:
> On 10/31/2017 12:53 PM, Ben Pfaff wrote:
> >On Fri, Sep 22, 2017 at 07:44:53AM -0700, Greg Rose wrote:
> >>The maximum message size for recent Linux kernels is 32Kb and in older
> >>kernels it is 16KB.
> >>
> >>See http://www.spinics.net/lists/netdev/msg431592.html
> >>
> >>Adjust the size checked and update a comment.
> >>
> >>Signed-off-by: Greg Rose 
> >
> >...
> >
> >>diff --git a/lib/netlink.c b/lib/netlink.c
> >>index de3ebcd..04310ff 100644
> >>--- a/lib/netlink.c
> >>+++ b/lib/netlink.c
> >>@@ -570,7 +570,7 @@ nl_msg_next(struct ofpbuf *buffer, struct ofpbuf *msg)
> >>  bool
> >>  nl_attr_oversized(size_t payload_size)
> >>  {
> >>-return payload_size > UINT16_MAX - NLA_HDRLEN;
> >>+return payload_size > INT16_MAX - NLA_HDRLEN;
> >>  }
> >
> >Thanks for the patch!
> >
> >I am confused by a difference between the commit message and the code.
> >Before this patch, nl_attr_oversized() considered an attribute of about
> >64 kB to be oversize; after this patch, about 32 kB.  Shouldn't the new
> >value be about 16 kB?
> >
> >Thanks,
> >
> >Ben.
> >
> 
> IIRC this is in user space so we prepare for whatever the maximum size we 
> might
> get from a kernel, which is 32KB.
> 
> We could have just left it at 64KB but we don't ever need that much.

This response implies that nl_attr_oversized() has something to do with
Netlink attributes for messages that userspace receives from the kernel.
This is not the case.  OVS userspace is prepared to handle any length
attribute in Netlink messages that it receives.  It won't apply
nl_attr_oversized() to received attributes, it will just go ahead and
process them.

On the contrary, userspace uses nl_attr_oversized() to limit the maximum
size of a Netlink attribute it *sends to* the kernel.  If the kernel
only supports a maximum 16 kB or 32 kB attribute, according to its
version, then the function needs to apply this limit.  The current code
assumes that the kernel can process anything that can actually be
formulated into Netlink.

What actual limit does Linux apply to Netlink messages, for sending and
receiving?  I am beginning to believe that this function is a red
herring and does not need to change.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH v2 04/10] flow: Add ESP spi value to flow struct.

[Stokes, Ian]

> On Tue, Oct 31, 2017 at 02:46:15PM -0700, Ben Pfaff wrote:
> > On Fri, Aug 25, 2017 at 05:40:26PM +0100, Ian Stokes wrote:
> > > This patch adds a field to the flow struct to represent the ESP
> > > security parameter index of a packet.
> > >
> > > Signed-off-by: Ian Stokes 
> > > ---
> > >  include/openvswitch/flow.h |5 +
> > >  1 files changed, 5 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h
> > > index a658a58..d986929 100644
> > > --- a/include/openvswitch/flow.h
> > > +++ b/include/openvswitch/flow.h
> > > @@ -156,6 +156,11 @@ struct flow {
> > >  ovs_be32 igmp_group_ip4;/* IGMP group IPv4 address.
> > >   * Keep last for BUILD_ASSERT_DECL
> below. */
> > >  ovs_be32 pad3;  /* Pad to 64 bits. */
> > > +
> > > +/* SPI for ESP (64-bit aligned) */
> > > +/* XXX TO DO: move this to the l3 layer */
> > > +ovs_be32 spi;
> > > +ovs_be32 pad4;
> > >  };
> >
> > I'd like to see this moved to the L3 layer (as your comment says).
> 
> Actually, I think I take that back.  Isn't this logically part of L4?
> (If so, then 'spi' can replace 'pad3' instead of being a doubleword of its
> own.)

No you're right, technically speaking IPsec is an L3 protocol.

I may have caused confusion here and in the EMC extract patch I submitted as 
part of this, in the EMC extract ESP is treated along with other L4 protocols, 
the reason there was from a code execution point of view I felt it wasnt a 
typical case and I didn't want to introduce a redundant check for the EMC in 
each call looking for ESP at layer 3 as chances are it won't be an ESP header.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH v2 02/10] openvswitch.h: add vport to ovs_action_push_tnl.

[Ben Pfaff]

On Wed, Nov 01, 2017 at 04:14:23PM +, Stokes, Ian wrote:
> > On Fri, Aug 25, 2017 at 05:40:24PM +0100, Ian Stokes wrote:
> > > Upon callback for building/pushing a packet header when encapsulating
> > > for tunneling a reference to the vport in question is required to
> > > access associated devices such as cryptodevs. This patch adds this
> > > pointer and will enable future work with cryptodevs that are
> > > associated with a vport.
> > >
> > > Signed-off-by: Ian Stokes 
> > > ---
> > >  datapath/linux/compat/include/linux/openvswitch.h |3 +++
> > >  1 files changed, 3 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/datapath/linux/compat/include/linux/openvswitch.h
> > > b/datapath/linux/compat/include/linux/openvswitch.h
> > > index bc6c94b..afa7faf 100644
> > > --- a/datapath/linux/compat/include/linux/openvswitch.h
> > > +++ b/datapath/linux/compat/include/linux/openvswitch.h
> > > @@ -723,6 +723,8 @@ struct ovs_action_hash {
> > >   * @tnl_port: To identify tunnel port to pass header info.
> > >   * @out_port: Physical port to send encapsulated packet.
> > >   * @header_len: Length of the header to be pushed.
> > > + * @dev: Pointer to vport so that the cryptodev parameters associated
> > > + with the
> > > + * vport can be accessed at the callback function.
> > >   * @tnl_type: This is only required to format this header.  Otherwise
> > >   * ODP layer can not parse %header.
> > >   * @header: Partial header for the tunnel. Tunnel push action can use
> > > @@ -732,6 +734,7 @@ struct ovs_action_push_tnl {
> > >   odp_port_t tnl_port;
> > >   odp_port_t out_port;
> > >   uint32_t header_len;
> > > +struct netdev_vport *dev;
> > >   uint32_t tnl_type; /* For logging. */
> > >   uint32_t header[TNL_PUSH_HEADER_SIZE / 4];  };
> > 
> > Maybe this is safe for some reason, but I worry that there's the
> > possibility of a use-after-free error.  Is 'dev' supposed to hold a
> > reference to the netdev (with netdev_ref())?  If so, it would be good to
> > document that in the comment.
> 
> Sure, the purpose of dev here is to allow access to the ipsec options such as 
> security association info (cipher/authentication type, keys etc.) associated 
> with the vport to be referenced during the build_header phase of processing. 
> For example this info would be required to know the length of the 
> initialization vector for the packet header. I'm open to a better way of this 
> info being shared to the build header stage for the vport but the only 
> reference passed was a pointer to ovs_action_push_tnl struct I just want to 
> get an opinion on using it this way.
> 
> This was a quick way to enable this for RFC purposes, so there could be a 
> use-after-free error (I didn't hit it in testing myself) but I'll look at it 
> again.

OK.  We need to carefully review this in a later version of the patch
set, then.  It would be good to call that out in a comment or the commit
message.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH v2 01/10] acinclude.m4: Support compilation of libIPsec.

[Ben Pfaff]

On Wed, Nov 01, 2017 at 04:01:10PM +, Stokes, Ian wrote:
> > On Fri, Aug 25, 2017 at 05:40:23PM +0100, Ian Stokes wrote:
> > > LibIpsecMB is required to enable the use of vdev cryptodev devices in
> > > DPDK. This patch adds a condition to check for the library when it is
> > > detected that ONFIG_RTE_LIBRTE_PMD_AESNI_MB=y is enabled in the DPDK
> > > config.
> > >
> > > Signed-off-by: Ian Stokes 
> > > ---
> > >  acinclude.m4 |   13 +
> > >  1 files changed, 13 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/acinclude.m4 b/acinclude.m4 index aeb594a..8c14367 100644
> > > --- a/acinclude.m4
> > > +++ b/acinclude.m4
> > > @@ -271,6 +271,19 @@ AC_DEFUN([OVS_CHECK_DPDK], [
> > > ], [],
> > > [AC_DEFINE([DPDK_PDUMP], [1], [DPDK pdump enabled in OVS.])])
> > >   ])
> > > +AC_COMPILE_IFELSE([
> > > +  AC_LANG_PROGRAM(
> > > +[
> > > +  #include 
> > > +#if RTE_LIBRTE_PMD_AESNI_MB
> > > +#error
> > > +#endif
> > > +], [])
> > > +  ], [],
> > > +
> > [AC_SEARCH_LIBS([init_mb_mgr_sse],[IPSec_MB],[],[AC_MSG_ERROR([unable to
> > find lib_IPSec_MB in ${LDFLAGS}, install the dependency package])])
> > > +   DPDK_EXTRA_LIB="-lIPSec_MB"
> > > +   AC_DEFINE([PMD_AESNI_MB], [1], [PMD_AESNI_MB support detected
> > > +in DPDK.])])
> > > +
> > 
> > It is a little unusual to make a test fail when a feature
> > (RTE_LIBRTE_PMD_AESNI_MB in this case) is detected.  This makes the
> > feature prone to being detected if something unrelated fails in the
> > toolchain.  Usually, one would either use the opposite approach--that is,
> > fail if RTE_LIBRTE_PMD_AESNI_MB is not declared--or something based on
> > AC_CHECK_DECL or AC_LINK_IFELSE using some symbol that
> > RTE_LIBRTE_PMD_AESNI_MB makes available.
> 
> Thanks Ben, Looking at this your right, this was my first foray into 
> modifying the acinclude to include an external library, but I take you point, 
> I'll clean this up in the RFC v3.
> 
> On a side note, are you ok with a separate library being required to enable a 
> feature?
> Part of this patch was to gather feedback on this, that a user would have to 
> download compile and populate the IPsec library for DPDK.
> The knock on effect being that the crypto dev functionality can then be used 
> in OVS DPDK can also use that functionality then.

I guess it's not reasonable for OVS to add this functionality in-tree,
so a separate library is probably the only choice.

To my mind, the bigger question is, should the library be optional or
required?  It is confusing to end users if they have to be aware of all
of the list of features that are built or not built into their binary,
so it's nice if a library can be required.  On the other hand, sometimes
that requirement puts an unreasonable burden on developers or packagers,
so it's necessary to have some balance.

> > Also, I recommend adding an explanation of this dependency to the
> > documentation, otherwise this will be confusing to users.
> 
> I think the dependency for IPsec MB is added to the DPDK install section in 
> this patchset, if not I'll definitely add it.

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

Re: [ovs-dev] [RFC PATCH v2 05/10] flow: Modify minflow extract to handle SPI.

[Stokes, Ian]

> On Fri, Aug 25, 2017 at 05:40:27PM +0100, Ian Stokes wrote:
> > The patch modifies the miniflow extract function to hande the case
> > where the network protocol for a packet is ESP. In this case the SPI
> > value in the ESP header is extracted and set in the minflow map.
> >
> > Signed-off-by: Ian Stokes 
> > ---
> >  lib/flow.c |   11 ++-
> >  1 files changed, 10 insertions(+), 1 deletions(-)
> >
> > diff --git a/lib/flow.c b/lib/flow.c
> > index b2b10aa..428ff7a 100644
> > --- a/lib/flow.c
> > +++ b/lib/flow.c
> > @@ -643,6 +643,7 @@ miniflow_extract(struct dp_packet *packet, struct
> miniflow *dst)
> >  uint8_t nw_frag, nw_tos, nw_ttl, nw_proto;
> >  uint8_t *ct_nw_proto_p = NULL;
> >  ovs_be16 ct_tp_src = 0, ct_tp_dst = 0;
> > +ovs_be32 esp_spi = 0;
> >
> >  /* Metadata. */
> >  if (flow_tnl_dst_is_set(>tunnel)) { @@ -920,7 +921,15 @@
> > miniflow_extract(struct dp_packet *packet, struct miniflow *dst)
> >  miniflow_push_be16(mf, ct_tp_src, ct_tp_src);
> >  miniflow_push_be16(mf, ct_tp_dst, ct_tp_dst);
> >  }
> > -} else if (OVS_LIKELY(nw_proto == IPPROTO_SCTP)) {
> > +} else if (OVS_LIKELY(nw_proto == IPPROTO_ESP)) {
> > +if (OVS_LIKELY(size >= ESP_HEADER_LEN)) {
> > +const struct esp_header *esp = data;
> > +
> > +esp_spi = esp->spi;
> > +miniflow_push_be32(mf, spi, esp_spi);
> > +miniflow_push_be32(mf, pad4, 0); /* Pad for ESP */
> > +}
> > +}else if (OVS_LIKELY(nw_proto == IPPROTO_SCTP)) {
> >  if (OVS_LIKELY(size >= SCTP_HEADER_LEN)) {
> >  const struct sctp_header *sctp = data;
> 
> This removes a space from the SCTP 'if' statement, it would be better
> without that change.
> 

Thanks will fix for rfc v3.
> Thanks,
> 
> Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [RFC PATCH v2 03/10] packets: Add ESP header and trailer.

[Stokes, Ian]

> -Original Message-
> From: Ben Pfaff [mailto:b...@ovn.org]
> Sent: Tuesday, October 31, 2017 9:44 PM
> To: Stokes, Ian 
> Cc: d...@openvswitch.org
> Subject: Re: [ovs-dev] [RFC PATCH v2 03/10] packets: Add ESP header and
> trailer.
> 
> On Fri, Aug 25, 2017 at 05:40:25PM +0100, Ian Stokes wrote:
> > This patch introduces structs for both ESP headers and ESP trailers
> > along with expected size assertions.
> >
> > Signed-off-by: Ian Stokes 
> 
> Applied to master, thanks!

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

Re: [ovs-dev] [RFC PATCH v2 02/10] openvswitch.h: add vport to ovs_action_push_tnl.

[Stokes, Ian]

> On Fri, Aug 25, 2017 at 05:40:24PM +0100, Ian Stokes wrote:
> > Upon callback for building/pushing a packet header when encapsulating
> > for tunneling a reference to the vport in question is required to
> > access associated devices such as cryptodevs. This patch adds this
> > pointer and will enable future work with cryptodevs that are
> > associated with a vport.
> >
> > Signed-off-by: Ian Stokes 
> > ---
> >  datapath/linux/compat/include/linux/openvswitch.h |3 +++
> >  1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/datapath/linux/compat/include/linux/openvswitch.h
> > b/datapath/linux/compat/include/linux/openvswitch.h
> > index bc6c94b..afa7faf 100644
> > --- a/datapath/linux/compat/include/linux/openvswitch.h
> > +++ b/datapath/linux/compat/include/linux/openvswitch.h
> > @@ -723,6 +723,8 @@ struct ovs_action_hash {
> >   * @tnl_port: To identify tunnel port to pass header info.
> >   * @out_port: Physical port to send encapsulated packet.
> >   * @header_len: Length of the header to be pushed.
> > + * @dev: Pointer to vport so that the cryptodev parameters associated
> > + with the
> > + * vport can be accessed at the callback function.
> >   * @tnl_type: This is only required to format this header.  Otherwise
> >   * ODP layer can not parse %header.
> >   * @header: Partial header for the tunnel. Tunnel push action can use
> > @@ -732,6 +734,7 @@ struct ovs_action_push_tnl {
> > odp_port_t tnl_port;
> > odp_port_t out_port;
> > uint32_t header_len;
> > +struct netdev_vport *dev;
> > uint32_t tnl_type; /* For logging. */
> > uint32_t header[TNL_PUSH_HEADER_SIZE / 4];  };
> 
> Maybe this is safe for some reason, but I worry that there's the
> possibility of a use-after-free error.  Is 'dev' supposed to hold a
> reference to the netdev (with netdev_ref())?  If so, it would be good to
> document that in the comment.

Sure, the purpose of dev here is to allow access to the ipsec options such as 
security association info (cipher/authentication type, keys etc.) associated 
with the vport to be referenced during the build_header phase of processing. 
For example this info would be required to know the length of the 
initialization vector for the packet header. I'm open to a better way of this 
info being shared to the build header stage for the vport but the only 
reference passed was a pointer to ovs_action_push_tnl struct I just want to get 
an opinion on using it this way.

This was a quick way to enable this for RFC purposes, so there could be a 
use-after-free error (I didn't hit it in testing myself) but I'll look at it 
again.

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

Re: [ovs-dev] [PATCH] rhel.rst: Add python-sphinx as a dependency.

[Guru Shetty]

On 31 October 2017 at 13:06, Aaron Conole  wrote:

> Ben Pfaff  writes:
>
> > On Tue, Oct 31, 2017 at 03:47:35PM -0400, Aaron Conole wrote:
> >> Ben Pfaff  writes:
> >>
> >> > On Fri, Oct 20, 2017 at 12:39:10AM -0700, Gurucharan Shetty wrote:
> >> >> Signed-off-by: Gurucharan Shetty 
> >> >> ---
> >> >>  Documentation/intro/install/rhel.rst | 2 +-
> >> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/Documentation/intro/install/rhel.rst
> b/Documentation/intro/install/rhel.rst
> >> >> index 86c5cf3..aff6ccf 100644
> >> >> --- a/Documentation/intro/install/rhel.rst
> >> >> +++ b/Documentation/intro/install/rhel.rst
> >> >> @@ -76,7 +76,7 @@ the below command::
> >> >>
> >> >>  $ yum install gcc make python-devel openssl-devel kernel-devel
> graphviz \
> >> >>  kernel-debug-devel autoconf automake rpm-build
> redhat-rpm-config \
> >> >> -libtool checkpolicy selinux-policy-devel
> >> >> +libtool checkpolicy selinux-policy-devel python-sphinx
> >> >
> >> > For Debian, we just recommend installing the build-dependencies listed
> >> > in debian/control.  That has the advantage that it can't get out of
> >> > date.  It has the disadvantage, though, that it's not easy to cut and
> >> > paste (although "apt-get build-dep openvswitch" usually does the
> trick).
> >> > Maybe "yum" has some mode that installs dependencies from a spec file?
> >>
> >> For 'yum' distributions:
> >>
> >>   yum-builddep
> >>
> >> For 'dnf' distributions (newer Fedora, and future RHEL versions):
> >>
> >>   dnf builddep
> >
> > Would it be reasonable to change rhel.rst to recommend using one of
> > those tools, to ease future maintenance?
>
> Sure.  Something like below?  I don't know about the wordsmithing, so
> I'll defer that to Guru.
>
> ---
>
> diff --git a/Documentation/intro/install/rhel.rst
> b/Documentation/intro/install/
> rhel.rst
> index 86c5cf3..36bb661 100644
> --- a/Documentation/intro/install/rhel.rst
> +++ b/Documentation/intro/install/rhel.rst
> @@ -72,11 +72,14 @@ Build Requirements
>
>  To compile the RPMs, you will need to install the packages described in
> the
>  :doc:`general` along with some additional packages. These can be
> installed with
> -the below command::
> +the below command for ``yum`` based distributions (but note that the
> +openvswitch source RPM must be available somewhere)::
>

Expecting source rpm while building the rpm, looks like is not going to
happen for majority of the cases. I took it that we will need it if
"yum-builddep openvswitch" needs to succeed?.





>
> -$ yum install gcc make python-devel openssl-devel kernel-devel
> graphviz \
> -kernel-debug-devel autoconf automake rpm-build redhat-rpm-config \
> -libtool checkpolicy selinux-policy-devel
> +$ yum-builddep openvswitch
> +
> +For ``dnf`` based distributions, use the following command::
> +
> +$ dnf builddep rhel/openvswitch-fedora.spec
>
>  .. _rhel-bootstrapping:
> --
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2] tunnel: ToS and TTL inheritance for MPLS tunneled traffic

[Balazs Nemeth]

From: Miklos Pelyva 

When a new outermost MPLS label is added to 'flow' the 'flow''s
Ethernet type is changed to 'mpls_eth_type'. After the new label is
set, the 'flow''s MPLS stack is updated, and the L3/4 fields are
cleared to mark them invalid. This results in loosing the values of
the 'nw_tos' and the 'nw_ttl' fields from the struct 'flow'.

Hence, it is impossible to use the ToS and TTL 'inherit' feature in
case of MPLS tunneled traffic, because currently the values to be
inherited are coming from the cleared (invalidated) variables of
struct 'flow'.

To support inheriting the ToS field to the outer tunnel header even
in the presence of an MPLS shim header, this patch introduces a new
variable in the context structure, called 'latest_nw_tos', which is
used for storing the up-to-date 'nw_tos' value during the flow
translation, and is referred in the 'tnl_port_send()' function.

Besides, for every MPLS packet the MPLS TTL field is copied to the
TTL field of the outer tunnel header, if inheritance is configured.

Two new unit tests are created for checking the ToS and TTL
inheritance for IP, and non-IP packets sent over MPLS over IP
tunnels. The ECN inheritance is not applied in that case, because
the related RFC 5129 does not describe an individual way, just
options for ECN in MPLS.

Signed-off-by: Miklos Pelyva 
Signed-off-by: Balazs Nemeth 
Signed-off-by: Jan Scheurich 
Co-authored-by: Jan Scheurich 
---
 lib/flow.h   |  5 +
 ofproto/ofproto-dpif-xlate.c | 24 
 ofproto/tunnel.c | 10 +-
 ofproto/tunnel.h |  2 +-
 tests/tunnel.at  | 36 +++-
 5 files changed, 70 insertions(+), 7 deletions(-)

diff --git a/lib/flow.h b/lib/flow.h
index b3128da..3b38298 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -1000,6 +1000,11 @@ static inline bool is_ip_any(const struct flow *flow)
 return dl_type_is_ip_any(get_dl_type(flow));
 }

+static inline bool is_mpls_any(const struct flow *flow)
+{
+return eth_type_mpls(flow->dl_type);
+}
+
 static inline bool is_ip_proto(const struct flow *flow, uint8_t ip_proto,
struct flow_wildcards *wc)
 {
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index d0b45d2..429bf99 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -372,6 +372,11 @@ struct xlate_ctx {
  * the MPLS label stack that was originally present. */
 bool was_mpls;

+/* Latest IP ToS, an always up to date representation of nw_tos during the
+ * lifetime of the ctx. Every time nw_tos is modified, it should be updated
+ * for possible inheritance configurations. UINT16_MAX means undefined. */
+uint16_t latest_nw_tos;
+
 /* True if conntrack has been performed on this packet during processing
  * on the current bridge. This is used to determine whether conntrack
  * state from the datapath should be honored after thawing. */
@@ -3716,6 +3721,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t 
ofp_port,
 wc->masks.nw_tos |= IP_DSCP_MASK;
 flow->nw_tos &= ~IP_DSCP_MASK;
 flow->nw_tos |= dscp;
+ctx->latest_nw_tos = flow->nw_tos;
 }
 }

@@ -3726,7 +3732,8 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t 
ofp_port,
   * matches, while explicit set actions on tunnel metadata are.
   */
 flow_tnl = flow->tunnel;
-odp_port = tnl_port_send(xport->ofport, flow, ctx->wc);
+odp_port = tnl_port_send(xport->ofport, flow, ctx->latest_nw_tos,
+ ctx->wc);
 if (odp_port == ODPP_NONE) {
 xlate_report(ctx, OFT_WARN, "Tunneling decided against output");
 goto out; /* restore flow_nw_tos */
@@ -3823,10 +3830,10 @@ compose_output_action__(struct xlate_ctx *ctx, 
ofp_port_t ofp_port,
  xport->xbundle));
 }

- out:
+out:
 /* Restore flow */
 memcpy(flow->vlans, flow_vlans, sizeof flow->vlans);
-flow->nw_tos = flow_nw_tos;
+ctx->latest_nw_tos = flow->nw_tos = flow_nw_tos;
 flow->dl_dst = flow_dl_dst;
 flow->dl_src = flow_dl_src;
 flow->packet_type = flow_packet_type;
@@ -5230,7 +5237,7 @@ xlate_sample_action(struct xlate_ctx *ctx,

 if (xport && xport->is_tunnel) {
 struct flow *flow = >xin->flow;
-tnl_port_send(xport->ofport, flow, ctx->wc);
+tnl_port_send(xport->ofport, flow, ctx->latest_nw_tos, ctx->wc);
 if (!ovs_native_tunneling_is_on(ctx->xbridge->ofproto)) {
 struct flow_tnl flow_tnl = flow->tunnel;

@@ -6305,6 +6312,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t 
ofpacts_len,
 wc->masks.nw_tos |= 

Re: [ovs-dev] [PATCH] travis: Fix OSX build on travis

[Ben Pfaff]

On Tue, Oct 31, 2017 at 05:16:59PM -0700, Yi-Hung Wei wrote:
> On Mon, Oct 23, 2017 at 12:40 PM, Guru Shetty  wrote:
> > On 23 October 2017 at 09:39, William Tu  wrote:
> >
> >> Run "brew update" before any installs.
> >> This yields a clean build:
> >> https://travis-ci.org/williamtu/ovs-travis/builds/291616874
> >>
> >> Signed-off-by: William Tu 
> >> Cc: Yi-Hung Wei 
> >>
> >
> > Applied to master, thanks!
> 
> Hi Guru,
> 
> Can you help to cherry pick this fix to older branchs? We have similar
> errors in MAC OSX build on other older branches.
> 
> ex: https://travis-ci.org/openvswitch/ovs/jobs/295462097 (branch 2.7)
> https://travis-ci.org/openvswitch/ovs/builds/295482886 (branch 2.8)

It was easy, so I backported this as far as branch-2.6.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v3 0/9] OVS-DPDK flow offload with rte_flow

[Ben Pfaff]

On Wed, Nov 01, 2017 at 11:24:30PM +0800, Yuanhan Liu wrote:
> On Tue, Oct 31, 2017 at 02:19:20PM -0700, Ben Pfaff wrote:
> > I see that this series appears to have stalled.  Should we expect a new
> > version sometime soon?  Do you need more reviews on the current version?
> 
> My bad. As I have stated in another email, I meant to send v4 earlier, but
> I found a bug. Meanwhile, I was heavily occupied by something else, that I
> don't have time to shape it.
> 
> Likely, I could be able to start working on v4 at mid of this month.

There's no hurry from my perspective, my main goal was to make sure that
I/we hadn't dropped the ball.  I'll look forward to the next version.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpif-netlink-rtnl: Fix ovs_geneve probing after restart

[Eric Garver]

On Mon, Oct 30, 2017 at 10:36:29AM -0700, William Tu wrote:
> Thanks for the review.
> 
> Guru and I had some offline discussion. We have concern about possible
> packet lost when unconditionally remove the interface and bring it
> again. Currently if the userspace ovs-vswitchd dies, since the kernel
> datapath module still remains, packets can still go through.
> If restarting ovs-vswitch restarts the geneve device, then we might
> see more packets lost. So I'm thinking about another way to fix it
> without deleting the device.

You can use dpif_netlink_rtnl_getlink() if the device already exists.
Then check IFLA_INFO_KIND == "ovs_geneve".

> 
> Thanks
> William
> 
> On Thu, Oct 26, 2017 at 1:20 PM, Eric Garver  wrote:
> > On Thu, Oct 26, 2017 at 12:31:03PM -0700, William Tu wrote:
> >> When using the out-of-tree (openvswitch compat) geneve module,
> >> the first time oot tunnel probing returns true (correct).
> >> Without unloading the geneve module, if the userspace ovs-vswitchd
> >> restarts, because the 'geneve_sys_6081' still exists, the probing
> >> incorrectly returns false and loads the in-tree (upstream kernel)
> >> geneve module.  The issue also exists the other way around, where
> >> existing geneve module is in-tree and ovs-vswitchd restarts.
> >>
> >> The patch fixes it by unconditionally removing the tunnel device
> >> before the probing.  To reproduce the issue, start the ovs
> >> > /etc/init.d/openvswitch-switch start
> >> > creat a bridge and attach a geneve port using out-of-tree geneve
> >> > /etc/init.d/openvswitch-switch restart
> >>
> >> Fixes: 921c370a9df5 ("dpif-netlink: Probe for out-of-tree tunnels, decides 
> >> used interface")
> >> Signed-off-by: William Tu 
> >> Cc: Eric Garver 
> >> Cc: Gurucharan Shetty 
> >> ---
> >>  lib/dpif-netlink-rtnl.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c
> >> index 0c32e7d8ccb4..148ce5ff3a3d 100644
> >> --- a/lib/dpif-netlink-rtnl.c
> >> +++ b/lib/dpif-netlink-rtnl.c
> >> @@ -448,6 +448,8 @@ dpif_netlink_rtnl_probe_oot_tunnels(void)
> >>  }
> >>
> >>  name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof 
> >> namebuf);
> >> +dpif_netlink_rtnl_destroy(name);
> >> +
> >>  error = dpif_netlink_rtnl_create(tnl_cfg, name, 
> >> OVS_VPORT_TYPE_GENEVE,
> >>   "ovs_geneve",
> >>   (NLM_F_REQUEST | NLM_F_ACK
> >> --
> >> 2.7.4
> >
> > Thanks for the fix!
> >
> > Acked-by: Eric Garver 
> ___
> 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 2/2] lib/netlink: Use correct netlink max message size

[Greg Rose]

On 10/31/2017 12:53 PM, Ben Pfaff wrote:

On Fri, Sep 22, 2017 at 07:44:53AM -0700, Greg Rose wrote:

The maximum message size for recent Linux kernels is 32Kb and in older
kernels it is 16KB.

See http://www.spinics.net/lists/netdev/msg431592.html

Adjust the size checked and update a comment.

Signed-off-by: Greg Rose 


...


diff --git a/lib/netlink.c b/lib/netlink.c
index de3ebcd..04310ff 100644
--- a/lib/netlink.c
+++ b/lib/netlink.c
@@ -570,7 +570,7 @@ nl_msg_next(struct ofpbuf *buffer, struct ofpbuf *msg)
  bool
  nl_attr_oversized(size_t payload_size)
  {
-return payload_size > UINT16_MAX - NLA_HDRLEN;
+return payload_size > INT16_MAX - NLA_HDRLEN;
  }


Thanks for the patch!

I am confused by a difference between the commit message and the code.
Before this patch, nl_attr_oversized() considered an attribute of about
64 kB to be oversize; after this patch, about 32 kB.  Shouldn't the new
value be about 16 kB?

Thanks,

Ben.



IIRC this is in user space so we prepare for whatever the maximum size we might
get from a kernel, which is 32KB.

We could have just left it at 64KB but we don't ever need that much.

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

Re: [ovs-dev] [PATCH 2/2] ipfix: Update Timestamp when flow updated

[Greg Rose]

On 10/31/2017 10:37 AM, Ben Pfaff wrote:

On Tue, Oct 31, 2017 at 10:08:08AM -0700, Greg Rose wrote:

On 10/24/2017 03:19 PM, Ben Pfaff wrote:

On Tue, Jun 06, 2017 at 01:42:08PM -0700, Greg Rose wrote:

On 06/06/2017 08:22 AM, Ben Pfaff wrote:

On Mon, Jun 05, 2017 at 08:34:32PM -0700, Greg Rose wrote:

On 06/05/2017 06:04 PM, Ben Pfaff wrote:

From: Greg Rose 

Reported-by: Felix Konstantin Maurer 
Signed-off-by: Greg Rose 
[b...@ovn.org changed this to use ipfix_now()]
Signed-off-by: Ben Pfaff 
---
   ofproto/ofproto-dpif-ipfix.c | 1 +
   1 file changed, 1 insertion(+)

diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c
index 5589b0ea05e1..bc63b7b0294b 100644
--- a/ofproto/ofproto-dpif-ipfix.c
+++ b/ofproto/ofproto-dpif-ipfix.c
@@ -1643,6 +1643,7 @@ ipfix_cache_update(struct dpif_ipfix_exporter *exporter,
   ipfix_cache_aggregate_entries(entry, old_entry);
   free(entry);
   ipfix_update_stats(exporter, false, current_flows, sampled_pkt_type);
+old_entry->flow_end_timestamp_usec = ipfix_now();
   }
   }



Looks good, thanks Ben!


Thanks for the review!

If I recall correctly, Felix reported that your original patch didn't
help, though, so probably this one doesn't either.  We should track that
down before we go farther, I guess.


Yes, I'm am following up.  Having all sorts of problems getting a working ipfix 
flow collector to work though.  The ManageEngine netflow collector claims to 
work with ipfix but SFAICT it does not so I'm not going to waste more time on 
it.  Once I can start
collecting and analyzing the work should proceed quickly.


I never applied this patch (from June) because we were continuing to
look deeper.  I don't know whether that ever bore fruit.  Should I apply
this patch now?

Thanks,

Ben.



I think this one got lost.  I never followed up on it and got busy with other 
more pressing issues.


Should it get revived?


Well, when I think about it I have been using ipfix for quite a while
and I haven't seen any timestamp issues.  So unless we see a reported
bug I'd hold off.

Thanks,

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

Re: [ovs-dev] [PATCH v3 0/9] OVS-DPDK flow offload with rte_flow

[Yuanhan Liu]

On Tue, Oct 31, 2017 at 02:19:20PM -0700, Ben Pfaff wrote:
> I see that this series appears to have stalled.  Should we expect a new
> version sometime soon?  Do you need more reviews on the current version?

My bad. As I have stated in another email, I meant to send v4 earlier, but
I found a bug. Meanwhile, I was heavily occupied by something else, that I
don't have time to shape it.

Likely, I could be able to start working on v4 at mid of this month.

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

Re: [ovs-dev] [PATCH net-next v15] openvswitch: enable NSH support

[Eric Garver]

On Wed, Nov 01, 2017 at 12:03:01PM +0800, Yi Yang wrote:
> v14->v15
>  - Check size in nsh_hdr_from_nlattr
>  - Fixed four small issues pointed out By Jiri and Eric

Thanks Yi.

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

[ovs-dev] [PATCH v3] tunnel: Fix deletion of datapath tunnel ports in case of reconfiguration

[Balazs Nemeth]

There is an issue in OVS with tunnel deletion during the
reconfiguration of OF tunnels. If the dst_port value is changed, the
old tunnel map entry will not be deleted, because the tp_port
argument of tnl_port_map_delete() has the new dst_port setting, hence
the tunnel cannot be found in the list of tnl_port structures.

The patch corrects this mechanism by adding a new argument,
'old_odp_port' to tnl_port_reconfigure(). This value is used to
identify the datapath tunnel port which is being reconfigured. In
connection with this fix, to unify the tunnel port map handling,
odp_port value is used to search the proper port to insert and delete
tunnel map entries as well. This variable can be used instead of
tp_port, as it is unique for all datapath tunnel ports, and there is
no need to reach dst_port from netdev_tunnel_config structure.

This patch also adds a printout to check the reference counter of
a tnl_port structure in tnl-port.c. Extending OVS unit test cases to
have ref_cnt values in the expected dump. Adding new test cases to
check if packet receiving is still working in the case of OF tunnel
port deletion. Adding new test cases to check the reference counter
in case of OF tunnel deletion or reconfiguration.

Signed-off-by: Balazs Nemeth 
Signed-off-by: Jan Scheurich 
Co-authored-by: Jan Scheurich 
---
 lib/tnl-ports.c   |  9 +
 lib/tnl-ports.h   |  4 ++--
 ofproto/ofproto-dpif.c|  8 +---
 ofproto/tunnel.c  | 37 -
 ofproto/tunnel.h  |  7 ---
 tests/tunnel-push-pop-ipv6.at | 33 ++---
 tests/tunnel-push-pop.at  | 35 ---
 7 files changed, 98 insertions(+), 35 deletions(-)

diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
index 777ed4d..04d2b3f 100644
--- a/lib/tnl-ports.c
+++ b/lib/tnl-ports.c
@@ -195,7 +195,7 @@ tnl_port_map_insert(odp_port_t port, ovs_be16 tp_port,

 ovs_mutex_lock();
 LIST_FOR_EACH(p, node, _list) {
-if (tp_port == p->tp_port && p->nw_proto == nw_proto) {
+if (p->port == port && p->nw_proto == nw_proto) {
 ovs_refcount_ref(>ref_cnt);
 goto out;
 }
@@ -255,7 +255,7 @@ ipdev_map_delete(struct ip_device *ip_dev, ovs_be16 
tp_port, uint8_t nw_proto)
 }

 void
-tnl_port_map_delete(ovs_be16 tp_port, const char type[])
+tnl_port_map_delete(odp_port_t port, const char type[])
 {
 struct tnl_port *p, *next;
 struct ip_device *ip_dev;
@@ -265,7 +265,7 @@ tnl_port_map_delete(ovs_be16 tp_port, const char type[])

 ovs_mutex_lock();
 LIST_FOR_EACH_SAFE(p, next, node, _list) {
-if (p->tp_port == tp_port && p->nw_proto == nw_proto &&
+if (p->port == port && p->nw_proto == nw_proto &&
 ovs_refcount_unref_relaxed(>ref_cnt) == 1) {
 ovs_list_remove(>node);
 LIST_FOR_EACH(ip_dev, node, _list) {
@@ -348,7 +348,8 @@ tnl_port_show(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
 }

 LIST_FOR_EACH(p, node, _list) {
-ds_put_format(, "%s (%"PRIu32")\n", p->dev_name, p->port);
+ds_put_format(, "%s (%"PRIu32") ref_cnt=%u\n", p->dev_name, p->port,
+  ovs_refcount_read(>ref_cnt));
 }

 out:
diff --git a/lib/tnl-ports.h b/lib/tnl-ports.h
index 58b048a..61ca0f8 100644
--- a/lib/tnl-ports.h
+++ b/lib/tnl-ports.h
@@ -26,10 +26,10 @@

 odp_port_t tnl_port_map_lookup(struct flow *flow, struct flow_wildcards *wc);

-void tnl_port_map_insert(odp_port_t port, ovs_be16 udp_port,
+void tnl_port_map_insert(odp_port_t, ovs_be16 tp_port,
  const char dev_name[], const char type[]);

-void tnl_port_map_delete(ovs_be16 udp_port, const char type[]);
+void tnl_port_map_delete(odp_port_t, const char type[]);
 void tnl_port_map_insert_ipdev(const char dev[]);
 void tnl_port_map_delete_ipdev(const char dev[]);
 void tnl_port_map_run(void);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 43d670a..c2c2b8a 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -378,6 +378,7 @@ type_run(const char *type)
 HMAP_FOR_EACH (iter, up.hmap_node, >up.ports) {
 char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
 const char *dp_port;
+odp_port_t old_odp_port;

 if (!iter->is_tunnel) {
 continue;
@@ -385,6 +386,7 @@ type_run(const char *type)

 dp_port = netdev_vport_get_dpif_port(iter->up.netdev,
  namebuf, sizeof namebuf);
+old_odp_port = iter->odp_port;
 node = simap_find(_backers, dp_port);
 if (node) {
 simap_put(>tnl_backers, dp_port, node->data);
@@ -406,7 +408,7 @@ type_run(const char *type)

 iter->odp_port = node ? 

Re: [ovs-dev] [PKG-Openstack-devel] Bug#878249: Bug#878249: recent OVS upload

[Thomas Goirand]

On 11/01/2017 02:58 AM, Thomas Goirand wrote:
> Ben, could you investigate? I've uploaded to Unstable a version with the
> patch above, because it's the last blocker to have the last OpenStack
> dependency approved in the NEW queue. Let me know if you think we need
> to work more on the package.
> 
> Cheers,
> 
> Thomas Goirand (zigo)

Hi again,

After that upload, it built on all platforms but mips and m68k. Would
you know how to fix?

Cheers,

Thomas Goirand (zigo)
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v5 0/7] Output packet batching.

[Eelco Chaudron]

I've reviewed the full patchset and I think the changes are ok.
Did add some small notes, but nothing major.

I replied to the ones missing my acked-by.

Cheers,

Eelco

On 27/10/17 18:19, Ilya Maximets wrote:

This patch-set inspired by [1] from Bhanuprakash Bodireddy.
Implementation of [1] looks very complex and introduces many pitfalls [2]
for later code modifications like possible packet stucks.

This version targeted to make simple and flexible output packet batching on
higher level without introducing and even simplifying netdev layer.

Basic testing of 'PVP with OVS bonding on phy ports' scenario shows
significant performance improvement.

Test results for time-based batching for v3:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-September/338247.html

Test results for v4:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-October/339624.html

[1] [PATCH v4 0/5] netdev-dpdk: Use intermediate queue during packet 
transmission.
 https://mail.openvswitch.org/pipermail/ovs-dev/2017-August/337019.html

[2] For example:
 https://mail.openvswitch.org/pipermail/ovs-dev/2017-August/337133.html

Version 5:
* pmd_thread_ctx_time_update() calls moved to different places to
  call them only from dp_netdev_process_rxq_port() and main
  polling functions:
pmd_thread_main, dpif_netdev_run and dpif_netdev_execute.
  All other functions should use cached time from pmd->ctx.now.
  It's guaranteed to be updated at least once per polling cycle.
* 'may_steal' patch returned to version from v3 because
  'may_steal' in qos is a completely different variable. This
  patch only removes 'may_steal' from netdev API.
* 2 more usec functions added to timeval to have complete public API.
* Checking of 'output_cnt' turned to assertion.

Version 4:
* Rebased on current master.
* Rebased on top of "Keep latest measured time for PMD thread."
  (Jan Scheurich)
* Microsecond resolution related patches integrated.
* Time-based batching without RFC tag.
* 'output_time' renamed to 'flush_time'. (Jan Scheurich)
* 'flush_time' update moved to 'dp_netdev_pmd_flush_output_on_port'.
  (Jan Scheurich)
* 'output-max-latency' renamed to 'tx-flush-interval'.
* Added patch for output batching statistics.

Version 3:

* Rebased on current master.
* Time based RFC: fixed assert on n_output_batches <= 0.

Version 2:

* Rebased on current master.
* Added time based batching RFC patch.
* Fixed mixing packets with different sources in same batch.


Ilya Maximets (7):
   dpif-netdev: Keep latest measured time for PMD thread.
   dpif-netdev: Output packet batching.
   netdev: Remove unused may_steal.
   netdev: Remove useless cutlen.
   timeval: Add functions with microsecond granularity.
   dpif-netdev: Time based output batching.
   dpif-netdev: Count sent packets and batches.

  lib/dpif-netdev.c | 335 +-
  lib/netdev-bsd.c  |   6 +-
  lib/netdev-dpdk.c |  30 ++---
  lib/netdev-dummy.c|   6 +-
  lib/netdev-linux.c|   8 +-
  lib/netdev-provider.h |   7 +-
  lib/netdev.c  |  12 +-
  lib/netdev.h  |   2 +-
  lib/timeval.c |  35 ++
  lib/timeval.h |   4 +
  vswitchd/vswitch.xml  |  16 +++
  11 files changed, 336 insertions(+), 125 deletions(-)



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

Re: [ovs-dev] [PATCH v5 6/7] dpif-netdev: Time based output batching.

[Eelco Chaudron]

On 27/10/17 18:19, Ilya Maximets wrote:

This allows to collect packets from more than one RX burst
and send them together with a configurable intervals.

'other_config:tx-flush-interval' can be used to configure
time that a packet can wait in output batch for sending.

dpif-netdev turned to microsecond resolution for time
measuring to ensure desired resolution of 'tx-flush-interval'.

Signed-off-by: Ilya Maximets 

This change looks fine to me.

However I did like the previous non assert() v4 implementation of
dp_netdev_pmd_flush_output_on_port better, but this is just a personal
preference. I guess you made this based on a comment from Bhannu.

Acked-by: Eelco Chaudron 

---8<---

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

Re: [ovs-dev] [PATCH net-next v15] openvswitch: enable NSH support

[Jiri Benc]

On Wed,  1 Nov 2017 12:03:01 +0800, Yi Yang wrote:
> v14->v15
>  - Check size in nsh_hdr_from_nlattr
>  - Fixed four small issues pointed out By Jiri and Eric

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

Re: [ovs-dev] [PATCH v4 1/7] dpif-netdev: Keep latest measured time for PMD thread.

[Eelco Chaudron]

On 27/10/17 13:11, Ilya Maximets wrote:
...
...

I feel that this patch makes the code more confusing. It's not clear when
I should just use pmd->ctx.now, or call pmd_thread_ctx_time_update() first.
Maybe just say we update it in pmd_thread_main() before every call to
dp_netdev_process_rxq_port(). If you need more accurate time, get it directly,
and obsolete the pmd_thread_ctx_time_update() call?

I understand your confusion, but make a syscall for each empty (no rx packets)
port may cause serious performance drop in case we're receiving intensive 
traffic
only on one port. So, I'm still insist on calling update inside 
*process_rxq_port()
only if we have some packets to process.

One more thing is that we also have main and revalidator threads which are able
to send packets, and they need at least nearly accurate time.

We have, actually, 2 functions which needs accurate time:
dp_netdev_input() and dp_netdev_pmd_flush_output_packets().

So, what if we'll keep only 5 calls to update functions when all patches 
applied?
Like this:

1. dp_netdev_configure_pmd() - initialization
2. dp_netdev_process_rxq_port() - time update before packet processing start
3. pmd_thread_main() - if (need_to_flush) - there was no time updates in this
processing cycle. Fix that before calling flush 
function.
4. dpif_netdev_run() - same, but for non-pmd thread.
5. dpif_netdev_execute() - because it's the one more packet source beside the
direct receiving from port.

Above schema will ensure that we have at least one time update per polling 
cycle.
And we will not need to have any other calls to pmd_thread_ctx_time_update().
It'll be safe for all other functions to just use pmd->ctx.now without thinking
when it was updated last time.

Hi Ilya,

I'll like this approach, it's more clean, and I'll ack it as part of 
your v5 submission.


//Eelco

Note:
As you can see above, update call will be removed from 
dp_netdev_pmd_try_optimize()
in this case. But it still will be in the first patch to be sure that time was
updated at least once before optimization. But I think, we can move it out of
the function to pmd_thread_main() just before calling 
dp_netdev_pmd_try_optimize().
The same about time update before XPS revalidation in dpif_netdev_run().

I'm going to implement above schema because, I think, it's really better than
currently implemented one. IMHO, it's simpler to understand and maintain.
Any thoughts?

Best regards, Ilya Maximets.


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