Re: [ovs-dev] [PATCH v5 1/9] datapath: add transport ports in route lookup for geneve

[William Tu]

On Thu, Aug 9, 2018 at 4:13 PM, Qiuyu Xiao  wrote:

> I have one question. In "datapath/linux/compat/include/net/geneve.h", 
> USE_UPSTREAM_TUNNEL
> decides whether to use Linux upstream kernel function or OVS kernel
> function to transmit Geneve packet. Currently, it chooses Linux upstream
> kernel function. How to set USE_UPSTREAM_TUNNEL to use OVS kernel function?
>

you can't set USE_UPSTREAM_TUNNEL, it is detected by acinclude by looking
at the kernel header files.


> Otherwise, even though this patch is applied, IPsec won't work for Geneve
> tunnel without Linux upstream also being patched?
>

another way is to test on different kernel version, for example, 4.8 kernel
the USE_UPSTREAM_TUNNEL should be no.

--William


>
> Thanks,
> Qiuyu
>
> On Thu, Aug 9, 2018 at 3:41 PM, William Tu  wrote:
>
>>
>>
>> On Thu, Aug 9, 2018 at 3:28 PM, Qiuyu Xiao 
>> wrote:
>>
>>> Hi William,
>>>
>>> ip_route_output_key() calls xfrm_lookup(). xfrm_lookup() needs L4 ports
>>> so that the packet can match IPsec's security policy based on L4 ports.
>>> IPsec security policy for Geneve selects udp packets with dst port
>>> 6081. If no port information, the IPsec stack won't know the packet is
>>> a Geneve packet and the packet won't be encrypted.
>>>
>>> Different dport and sport affect `struct xfrm_state` in the `struct 
>>> dst_entry`.
>>> But this structure only matters to the xfrm module. The Linux upstream
>>> VXLAN module already included L4 ports for VXLAN route look up.
>>>
>>>
>> I see, thanks!
>>
>> --William
>>
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Flavio Leitner]

On Thu, Aug 09, 2018 at 11:50:38AM +0200, Timothy Redaelli wrote:
> On Wed, 8 Aug 2018 13:49:12 -0300
> Flavio Leitner  wrote:
> 
> > Actually, we also have dependencies in the systemd service
> > to the network service:
> > 
> > [Unit]  
> > 
> > Description=Open vSwitch Forwarding Unit
> > After=ovsdb-server.service network-pre.target
> > systemd-udev-settle.service Before=network.target network.service
> >   ^^^ 
> > 
> > Looks like that would be gone, right?
> 
> I checked and "network.service" is an auto-generated service
> (systemd-sysv-generator - Unit generator for SysV init scripts)
> from /etc/rc.d/init.d/network that brings Up and Down the interfaces
> (if you don't use Network Manager).
> 
> The explicit dependency is needed in order to avoid having it to run
> before openvswitch / ovsdb-server.
> Moreover it's also harmless to have it when network.service doesn't
> exists since, in this case, systemd will only ignore the dependency.
>
> For example, NetworkManager.service has "Before=network.target
> network.service" on Fedora Rawhide too, since Fedora Rawhide still have
> the support for the legacy network scripts (/etc/rc.d/init.d/network, in
> network-scripts package).

Alright, thanks for looking into this.

Acked-by: Flavio Leitner 

Thanks Timothy.
-- 
Flavio

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

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

[Vasiliy Tolstov]

ср, 8 авг. 2018 г. в 12:28, Vasiliy Tolstov :


> My question is - how can i do connectivity if vm attached to br-int
> have external ip address for example 1.2.3.100 (provided by ovn dhcp),
> and gateway server that have uplink to internet have ip 1.2.3.254.
> As i understand logical router needs to be created to route traffic
> from diffrent networks...


Sorry i can't find in google something like this. In google i only
find what i need to do route from different networks or attach port to
br-ex bridge.


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

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

[Yifeng Sun]

Hi Greg,

Thanks for clarifying this bug, you did a much better job than I did. Sorry
I didn't describe it clearly.

I will created another patch, as you suggested.


William, below is the crash dump you requested.

[  645.832991] general protection fault:  [#1] SMP PTI
[  645.833033] Modules linked in: veth vport_vxlan(OE) vport_stt(OE)
vport_lisp(OE) vport_geneve(OE) openvswitch(OE) tunnel6 nf_conntrack_ipv6
nf_defrag_ipv6 nf_nat_ipv6 nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4
nf_nat nf_conntrack udp_tunnel netconsole rpcsec_gss_krb5 auth_rpcgss nfsv4
nfs vmw_vsock_vmci_transport vsock lockd grace fscache vmw_balloon sb_edac
joydev input_leds serio_raw coretemp vmw_vmci shpchp i2c_piix4 mac_hid
ib_iser rdma_cm iw_cm ib_cm ib_core sunrpc configfs iscsi_tcp libiscsi_tcp
libiscsi scsi_transport_iscsi autofs4 btrfs zstd_decompress zstd_compress
xxhash raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor
async_tx xor raid6_pq libcrc32c raid1 raid0 multipath linear hid_generic
usbhid hid crct10dif_pclmul crc32_pclmul ghash_clmulni_intel vmwgfx pcbc
ttm drm_kms_helper
[  645.836096]  syscopyarea sysfillrect sysimgblt fb_sys_fops drm psmouse
aesni_intel aes_x86_64 crypto_simd glue_helper cryptd mptspi mptscsih
mptbase vmxnet3 ahci libahci scsi_transport_spi pata_acpi
[  645.837697] CPU: 0 PID: 17724 Comm: handler2 Tainted: G   OE
 4.15.18 #1
[  645.838250] Hardware name: VMware, Inc. VMware Virtual Platform/440BX
Desktop Reference Platform, BIOS 6.00 09/21/2015
[  645.839419] RIP: 0010:ip6erspan_tunnel_xmit+0x1da/0x6f0 [openvswitch]
[  645.839996] RSP: 0018:b36d008177c0 EFLAGS: 00010286
[  645.840562] RAX: ffea RBX: 9be571e84000 RCX:
0040
[  645.841126] RDX: 003e RSI: 71cf7e00 RDI:
9be571cf7600
[  645.841697] RBP: b36d00817880 R08: 0018 R09:

[  645.842268] R10: b36d00817a90 R11: fffe R12:
9be5610e4400
[  645.842838] R13:  R14: 0001 R15:

[  645.843422] FS:  7eff6e146700() GS:9be5bfc0()
knlGS:
[  645.844015] CS:  0010 DS:  ES:  CR0: 80050033
[  645.844607] CR2: 7febc8f859d0 CR3: 5de8a002 CR4:
001606f0
[  645.845250] Call Trace:
[  645.846070]  do_execute_actions+0x1505/0x1900 [openvswitch]
[  645.846686]  ? reserve_sfa_size+0x28/0x100 [openvswitch]
[  645.847319]  ? __ovs_nla_copy_actions+0x15c/0x6d0 [openvswitch]
[  645.847958]  ? __kmalloc_reserve.isra.39+0x2e/0x80
[  645.848581]  ? _cond_resched+0x16/0x40
[  645.849196]  ? __kmalloc+0x16d/0x1f0
[  645.849791]  ? ovs_execute_actions+0x48/0x120 [openvswitch]
[  645.850401]  ovs_execute_actions+0x48/0x120 [openvswitch]
[  645.851012]  ovs_packet_cmd_execute+0x267/0x2b0 [openvswitch]
[  645.851624]  genl_family_rcv_msg+0x1e9/0x380
[  645.852231]  genl_rcv_msg+0x47/0x90
[  645.852820]  ? genl_family_rcv_msg+0x380/0x380
[  645.853399]  netlink_rcv_skb+0xde/0x110
[  645.853962]  genl_rcv+0x24/0x40
[  645.854505]  netlink_unicast+0x183/0x240
[  645.855040]  netlink_sendmsg+0x2c2/0x3b0
[  645.855568]  sock_sendmsg+0x36/0x40
[  645.856074]  ___sys_sendmsg+0x2bc/0x2d0
[  645.856571]  ? ep_item_poll.isra.10+0x3b/0xc0
[  645.857045]  ? ep_send_events_proc+0x87/0x1a0
[  645.857501]  ? ep_read_events_proc+0xd0/0xd0
[  645.857940]  ? ep_scan_ready_list+0x1f3/0x200
[  645.858371]  ? ep_poll+0x1fb/0x3b0
[  645.858788]  ? __sys_sendmsg+0x51/0x90
[  645.859189]  __sys_sendmsg+0x51/0x90
[  645.859585]  do_syscall_64+0x6e/0x120
[  645.859964]  entry_SYSCALL_64_after_hwframe+0x3d/0xa2
[  645.860472] RIP: 0033:0x7eff6ec36a6d
[  645.860843] RSP: 002b:7eff6e0e81a0 EFLAGS: 0293 ORIG_RAX:
002e
[  645.861226] RAX: ffda RBX: 0002 RCX:
7eff6ec36a6d
[  645.861622] RDX:  RSI: 7eff6e0e8200 RDI:
0011
[  645.862009] RBP: 7eff6e0e8fe0 R08:  R09:
7eff6e0e9a60
[  645.862396] R10: 00a5d340 R11: 0293 R12:
009f7690
[  645.862775] R13: 7eff6e0e8200 R14: 0002 R15:
7eff6e0e86a0
[  645.863189] Code: 0f 85 a6 fe ff ff 45 31 c9 b8 ea ff ff ff 66 45 89 4c
24 3c 66 81 a3 1a 09 00 00 ff fb 49 8b 74 24 38 48 85 f6 0f 84 a2 fe ff ff
<0f> b6 96 f1 00 00 00 b8 ea ff ff ff f6 c2 01 0f 84 8d fe ff ff
[  645.864513] RIP: ip6erspan_tunnel_xmit+0x1da/0x6f0 [openvswitch] RSP:
b36d008177c0
[  645.865043] ---[ end trace 25bab82318677b6f ]---
[  645.865452] Kernel panic - not syncing: Fatal exception in interrupt
[  645.865923] Kernel Offset: 0x3d00 from 0x8100
(relocation range: 0x8000-0xbfff)
[  645.866752] ---[ end Kernel panic - not syncing: Fatal exception in
interrupt


On Thu, Aug 9, 2018 at 3:59 PM, Gregory Rose  wrote:

> On 8/8/2018 11:32 AM, Yifeng Sun wrote:
>
>> In compatable gre module, skb->cb is used as ovs_gso_cb.
>> This bug clears the 16-23 bit in the address of 

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

[Gregory Rose]

On 8/8/2018 11:32 AM, Yifeng Sun wrote:

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

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

diff --git a/datapath/linux/compat/ip6_gre.c b/datapath/linux/compat/ip6_gre.c
index 54a76ab..16c1f72 100644
--- a/datapath/linux/compat/ip6_gre.c
+++ b/datapath/linux/compat/ip6_gre.c
@@ -1146,7 +1146,6 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff 
*skb,
goto tx_err;
  
  	t->parms.o_flags &= ~TUNNEL_KEY;

-   IPCB(skb)->flags = 0;
  
  	tun_info = ovs_skb_tunnel_info(skb);

if (unlikely(!tun_info ||


Yifeng,

First, I'm sorry but you actually did identify the problem correctly at 
first, I just didn't see it.  My bad.


Second, I'm no longer worried about removing the code.  It shouldn't be 
used in the case where
we are not using USE_UPSTREAM_TUNNEL.  If USE_UPSTREAM_TUNNEL is defined 
then this entire
module does not compile anyway.  So I think removing the IPCB macro that 
sets the flags to zero is fine.


The same needs to be done in _gre6_xmit().

Go ahead and resubmit the patch with the _gre6_xmit() case handled as 
well and I think that should be fine.


Thanks for all your work!!!

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

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

[Ben Pfaff]

On Thu, Aug 09, 2018 at 04:51:44PM +0300, Ilya Maximets wrote:
> On 09.08.2018 02:05, Ben Pfaff wrote:
> > On Wed, Aug 08, 2018 at 03:35:31PM +0300, Ilya Maximets wrote:
> >> Each run of the testsuite produces millions lines in a system
> >> log. This is completely unnecessary and makes it difficult to
> >> use system logs on test / build servers.
> >>
> >> This series is aimed to disable most of the syslog messages.
> >> There are still few logs that requires significant changes in
> >> tests or code to disable. They will be removed separately if
> >> needed.
> > 
> > What do you think of the following approach?
> > https://patchwork.ozlabs.org/patch/955273/
> 
> That's cool. I forget about ability to change syslog method.
> And we don't need to change any tests with this approach.
> I'll reply with my comments to the patch.

Thanks.

I really wanted a solution that didn't require a lot of maintenance in
the future.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Gregory Rose]

On 8/9/2018 3:33 PM, William Tu wrote:



On Thu, Aug 9, 2018 at 2:37 PM, Gregory Rose > wrote:


On 8/9/2018 11:36 AM, Yifeng Sun wrote:

Yes, I agree. It should be the way to go.

Thanks,
Yifeng



If I'm reading the code correctly the OVS packet cmd will set the
skb->cb using the OVS_CB macro and then
pass that packet down to the ip6erspan_tunnel_xmit function.  The
ip6erspan_tunnel_xmit function will
then use the IPCB macro to access the skb->cb area to set the flag
to zero.  That works fine for packets
generated by the system that have used the IPCB macro accessor to
set the skb->cb data.  However, it
is catastrophic when an skb was prepared with the OVS_CB macro
because, as you might imagine, it
is not pointing to an inet_skb_parm structure but an ovs_skb_cb
structure instead.

on thing I don't understand is that:
when packet/skb is sent to ip6erspan_tunnel_xmit, it is already out of 
OVS code.
ip6erspan_tunnel_xmit clears the inner skb->cb, encap skb with outer 
erspan+gre header,
then lookup the next netdev using outer ip address. The skb again 
arrives at another netdev.
Until this point, there is no OVS code involves and so OVS_CB macro 
shouldn't cause issue.


Yifeng, can you share the crash dump?
-- William


OK, I did not read the code correctly.  It's an ovs_gso_cb structure and 
now I get where you were talking
about bits 16-23 being over written - that being the tun_dst pointer in 
our case since USE_UPSTREAM_TUNNEL

is not defined.  That wasn't really clear to me before.

Here's a before and after dump of the skb->cb data area:
[17182.462235] Before -
[17182.463006] 00 9c 6f 8b 21 9d ff ff 00 00 4c 00 00 00 00 00
[17182.463010]00 f2 22 b6 21 9d ff ff 00 00 00 00 00 00 00 00
[17182.463777] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[17182.464519] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[17182.465277] -
[17182.466749] After -
[17182.467462] 00 9c 6f 8b 21 9d ff ff 00 00 4c 00 00 00 00 00
[17182.467465] 00 f2 22 b6 00 00 ff ff 00 00 00 00 00 00 00 00
[17182.468221] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[17182.468968] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[17182.469734] -

Note the bits 16-23 of the tun_dst address being overwritten...  :O

Guh - keeping track of all the  CB accessors is a real tangled mess.

- Greg



This code was *never* correct.  Somehow or other it didn't cause a
problem until 4.15.  We'll have to
figure out some way to check if the packet is ours or is system
generated to know if we should be using
the IPCB accessor.

In instances of packets generated by the system or any other
entity transmitting ipv6 packets over the
tunnel we'll need to keep the code that clears the flags.

Thanks,

- Greg



On Thu, Aug 9, 2018 at 11:17 AM, Gregory Rose
mailto:gvrose8...@gmail.com>> wrote:

On 8/9/2018 11:03 AM, Yifeng Sun wrote:

There is a difference regarding how skb_tunnel_info is
stored in skb
between ovs and upstream kernel. In ovs's compatible gre module,
skb_tunnel_info is stored in skb->cb while in upstream
kernel, it is
referenced by skb->_skb_refdst.

The upstream netdev code should be okay.

To fix this issue, my guess is that either we comply to the
kernel's way
by using skb->_skb_refdst to store skb_tunnel_info, or we
don't use
IPCB at all.


Ah, I see...

We must comply with the kernel method for any given kernel. 
We can't be sure that we'll only handle
or own packets.

Can't this be fixed by a compatibility layer #define in
acinclude.m4 so that kernels that store it in
skb->cb vs. kernels that store it in skb->__skb_refdst will
both work?

Thanks,

- Greg




Thanks,
Yifeng



On Thu, Aug 9, 2018 at 10:28 AM, Gregory Rose
mailto:gvrose8...@gmail.com>> wrote:


On 8/8/2018 6:50 PM, William Tu wrote:

thanks for the fix.

On Wed, Aug 8, 2018 at 11:32 AM, Yifeng Sun
mailto:pkusunyif...@gmail.com>> wrote:

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

can you explain more about ovs_gso_cb?

Signed-off-by: Yifeng Sun
mailto:pkusunyif...@gmail.com>>
---
datapath/linux/compat/ip6_gre.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/datapath/linux/compat/ip6_gre.c
b/datapath/linux/compat/ip6_
gre.c
index 54a76ab..16c1f72 100644
  

Re: [ovs-dev] [PATCH v5 1/9] datapath: add transport ports in route lookup for geneve

[William Tu]

On Thu, Aug 9, 2018 at 3:28 PM, Qiuyu Xiao  wrote:

> Hi William,
>
> ip_route_output_key() calls xfrm_lookup(). xfrm_lookup() needs L4 ports
> so that the packet can match IPsec's security policy based on L4 ports.
> IPsec security policy for Geneve selects udp packets with dst port 6081.
> If no port information, the IPsec stack won't know the packet is a Geneve
> packet and the packet won't be encrypted.
>
> Different dport and sport affect `struct xfrm_state` in the `struct 
> dst_entry`.
> But this structure only matters to the xfrm module. The Linux upstream
> VXLAN module already included L4 ports for VXLAN route look up.
>
>
I see, thanks!

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

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

[Ben Pfaff]

Thanks.  I folded that in and applied this to master.

Maybe some of your series is cleanups or improvements.  If so, please
feel free to re-send those parts of it.

On Thu, Aug 09, 2018 at 05:02:29PM +0300, Ilya Maximets wrote:
> The patch is good.
> 
> The following incremental needed to make it complete:
> 
> diff --git a/python/ovs/vlog.py b/python/ovs/vlog.py
> index d7a84d8..7d416c3 100644
> --- a/python/ovs/vlog.py
> +++ b/python/ovs/vlog.py
> @@ -305,9 +305,11 @@ class Vlog(object):
>  return
>  
>  logger = logging.getLogger('syslog')
> -# If there is no infrastructure to support python syslog, disable
> -# the logger to avoid repeated errors.
> -if not os.path.exists("/dev/log"):
> +# Disable the logger if there is no infrastructure to support python
> +# syslog (to avoid repeated errors) or if the "null" syslog method
> +# requested by environment.
> +if not os.path.exists("/dev/log") \
> +   or os.environ.get('OVS_SYSLOG_METHOD') == "null":
>  logger.disabled = True
>  return
> 
> 
> With above change, tests leaves the crystal clear syslog.
> 
> Beside that,
> Acked-by: Ilya Maximets 
> 
> On 09.08.2018 02:04, Ben Pfaff wrote:
> > Until now, "make check" generated a huge amount of output to syslog.  This
> > commit suppresses it.
> > 
> > CC: Ilya Maximets 
> > Signed-off-by: Ben Pfaff 
> > ---
> >  NEWS  |  2 ++
> >  lib/automake.mk   |  2 ++
> >  lib/syslog-null.c | 60 
> > +++
> >  lib/syslog-null.h | 22 
> >  lib/vlog.c| 12 +--
> >  lib/vlog.man  |  7 ++-
> >  lib/vlog.xml  | 11 +-
> >  tests/atlocal.in  |  4 
> >  8 files changed, 116 insertions(+), 4 deletions(-)
> >  create mode 100644 lib/syslog-null.c
> >  create mode 100644 lib/syslog-null.h
> > 
> > diff --git a/NEWS b/NEWS
> > index 7875f6673e34..ae21340e9046 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -1,5 +1,7 @@
> >  Post-v2.10.0
> >  -
> > +   - The environment variable OVS_SYSLOG_METHOD, if set, is now used
> > + as the default syslog method.
> >  
> >  
> >  v2.10.0 - xx xxx 
> > diff --git a/lib/automake.mk b/lib/automake.mk
> > index fb43aa1413b2..63e9d72ac18a 100644
> > --- a/lib/automake.mk
> > +++ b/lib/automake.mk
> > @@ -280,6 +280,8 @@ lib_libopenvswitch_la_SOURCES = \
> > lib/syslog-direct.h \
> > lib/syslog-libc.c \
> > lib/syslog-libc.h \
> > +   lib/syslog-null.c \
> > +   lib/syslog-null.h \
> > lib/syslog-provider.h \
> > lib/table.c \
> > lib/table.h \
> > diff --git a/lib/syslog-null.c b/lib/syslog-null.c
> > new file mode 100644
> > index ..9dbd13911c21
> > --- /dev/null
> > +++ b/lib/syslog-null.c
> > @@ -0,0 +1,60 @@
> > +/*
> > + * Copyright (c) 2015, 2016 Nicira, Inc.
> > + *
> > + * Licensed under the Apache License, Version 2.0 (the "License");
> > + * you may not use this file except in compliance with the License.
> > + * You may obtain a copy of the License at:
> > + *
> > + * http://www.apache.org/licenses/LICENSE-2.0
> > + *
> > + * Unless required by applicable law or agreed to in writing, software
> > + * distributed under the License is distributed on an "AS IS" BASIS,
> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> > + * See the License for the specific language governing permissions and
> > + * limitations under the License.
> > + */
> > +#include "syslog-null.h"
> > +
> > +#include 
> > +
> > +#include "compiler.h"
> > +#include "syslog-provider.h"
> > +#include "util.h"
> > +
> > +static void syslog_null_open(struct syslogger *this, int facility);
> > +static void syslog_null_log(struct syslogger *this, int pri, const char 
> > *msg);
> > +
> > +static struct syslog_class syslog_null_class = {
> > +syslog_null_open,
> > +syslog_null_log,
> > +};
> > +
> > +struct syslog_null {
> > +struct syslogger parent;
> > +};
> > +
> > +/* This function  creates object that delegate all logging to null's
> > + * syslog implementation. */
> > +struct syslogger *
> > +syslog_null_create(void)
> > +{
> > +struct syslog_null *this = xmalloc(sizeof *this);
> > +
> > +this->parent.class = _null_class;
> > +this->parent.prefix = "";
> > +
> > +return >parent;
> > +}
> > +
> > +static void
> > +syslog_null_open(struct syslogger *this OVS_UNUSED, int facility 
> > OVS_UNUSED)
> > +{
> > +/* Nothing to do. */
> > +}
> > +
> > +static void
> > +syslog_null_log(struct syslogger *this OVS_UNUSED, int pri OVS_UNUSED,
> > +const char *msg OVS_UNUSED)
> > +{
> > +/* Nothing to do. */
> > +}
> > diff --git a/lib/syslog-null.h b/lib/syslog-null.h
> > new file mode 100644
> > index ..0f7731dc4dcc
> > --- /dev/null
> > +++ b/lib/syslog-null.h
> > @@ -0,0 +1,22 @@
> > +/*
> > + * Copyright (c) 2015 Nicira, Inc.
> > + *
> 

Re: [ovs-dev] [PATCH v5 3/6] debian and rhel: Create IPsec package.

[Ben Pfaff]

On Thu, Aug 09, 2018 at 06:31:31PM -0400, Aaron Conole wrote:
> Ben Pfaff  writes:
> 
> > On Thu, Aug 09, 2018 at 12:40:39PM -0700, Ansis Atteka wrote:
> >> On Tue, 7 Aug 2018 at 09:43, Qiuyu Xiao  wrote:
> >> >
> >> > Added rules and files to create debian and rpm ovs-ipsec packages.
> >> >
> >> > Signed-off-by: Qiuyu Xiao 
> >> > Signed-off-by: Ansis Atteka 
> >> > Co-authored-by: Ansis Atteka 
> >> 
> >> Did you test this patch on Fedora with SElinux enabled?
> >> ovs-monitor-ipsec daemon fails to start. You need to create SElinux
> >> policy too:
> >
> > Is that something you can help with?  I doubt that Qiuyu has much
> > experience with SELinux (and I don't either).
> 
> I'll throw something together tomorrow, if Ansis isn't able to do so.

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

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

[William Tu]

On Thu, Aug 9, 2018 at 2:37 PM, Gregory Rose  wrote:

> On 8/9/2018 11:36 AM, Yifeng Sun wrote:
>
> Yes, I agree. It should be the way to go.
>
> Thanks,
> Yifeng
>
>
> If I'm reading the code correctly the OVS packet cmd will set the skb->cb
> using the OVS_CB macro and then
> pass that packet down to the ip6erspan_tunnel_xmit function.  The
> ip6erspan_tunnel_xmit function will
> then use the IPCB macro to access the skb->cb area to set the flag to
> zero.  That works fine for packets
> generated by the system that have used the IPCB macro accessor to set the
> skb->cb data.  However, it
> is catastrophic when an skb was prepared with the OVS_CB macro because, as
> you might imagine, it
> is not pointing to an inet_skb_parm structure but an ovs_skb_cb structure
> instead.
>
> on thing I don't understand is that:
when packet/skb is sent to ip6erspan_tunnel_xmit, it is already out of OVS
code.
ip6erspan_tunnel_xmit clears the inner skb->cb, encap skb with outer
erspan+gre header,
then lookup the next netdev using outer ip address. The skb again arrives
at another netdev.
Until this point, there is no OVS code involves and so OVS_CB macro
shouldn't cause issue.

Yifeng, can you share the crash dump?
-- William

This code was *never* correct.  Somehow or other it didn't cause a problem
> until 4.15.  We'll have to
> figure out some way to check if the packet is ours or is system generated
> to know if we should be using
> the IPCB accessor.
>
> In instances of packets generated by the system or any other entity
> transmitting ipv6 packets over the
> tunnel we'll need to keep the code that clears the flags.
>
> Thanks,
>
> - Greg
>
>
> On Thu, Aug 9, 2018 at 11:17 AM, Gregory Rose 
> wrote:
>
>> On 8/9/2018 11:03 AM, Yifeng Sun wrote:
>>
>> There is a difference regarding how skb_tunnel_info is stored in skb
>> between ovs and upstream kernel. In ovs's compatible gre module,
>> skb_tunnel_info is stored in skb->cb while in upstream kernel, it is
>> referenced by skb->_skb_refdst.
>>
>> The upstream netdev code should be okay.
>>
>> To fix this issue, my guess is that either we comply to the kernel's way
>> by using skb->_skb_refdst to store skb_tunnel_info, or we don't use
>> IPCB at all.
>>
>>
>> Ah, I see...
>>
>> We must comply with the kernel method for any given kernel.  We can't be
>> sure that we'll only handle
>> or own packets.
>>
>> Can't this be fixed by a compatibility layer #define in acinclude.m4 so
>> that kernels that store it in
>> skb->cb vs. kernels that store it in skb->__skb_refdst will both work?
>>
>> Thanks,
>>
>> - Greg
>>
>>
>>
>> Thanks,
>> Yifeng
>>
>>
>>
>> On Thu, Aug 9, 2018 at 10:28 AM, Gregory Rose 
>> wrote:
>>
>>>
>>> On 8/8/2018 6:50 PM, William Tu wrote:
>>>
 thanks for the fix.

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

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

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

>>>
>>> That would be the normal work flow.
>>>
>>> Yifeng,
>>>
>>> Can you please post a patch with this fix to netdev?  Taking William's
>>> comments into account as well.
>>>
>>> Good catch and thanks for the fix!
>>>
>>> - Greg
>>>
>>>
 Thanks,
 William
 ___
 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 v5 3/6] debian and rhel: Create IPsec package.

[Aaron Conole]

Ben Pfaff  writes:

> On Thu, Aug 09, 2018 at 12:40:39PM -0700, Ansis Atteka wrote:
>> On Tue, 7 Aug 2018 at 09:43, Qiuyu Xiao  wrote:
>> >
>> > Added rules and files to create debian and rpm ovs-ipsec packages.
>> >
>> > Signed-off-by: Qiuyu Xiao 
>> > Signed-off-by: Ansis Atteka 
>> > Co-authored-by: Ansis Atteka 
>> 
>> Did you test this patch on Fedora with SElinux enabled?
>> ovs-monitor-ipsec daemon fails to start. You need to create SElinux
>> policy too:
>
> Is that something you can help with?  I doubt that Qiuyu has much
> experience with SELinux (and I don't either).

I'll throw something together tomorrow, if Ansis isn't able to do so.

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

Re: [ovs-dev] [PATCH v5 1/9] datapath: add transport ports in route lookup for geneve

[Qiuyu Xiao]

Hi William,

ip_route_output_key() calls xfrm_lookup(). xfrm_lookup() needs L4 ports so
that the packet can match IPsec's security policy based on L4 ports. IPsec
security policy for Geneve selects udp packets with dst port 6081. If no
port information, the IPsec stack won't know the packet is a Geneve packet
and the packet won't be encrypted.

Different dport and sport affect `struct xfrm_state` in the `struct dst_entry`.
But this structure only matters to the xfrm module. The Linux upstream VXLAN
module already included L4 ports for VXLAN route look up.

Thanks,
Qiuyu



On Thu, Aug 9, 2018 at 2:13 PM, William Tu  wrote:

> Hi Qiuyu,
>
> Can you explain a little more about why you need L4 ports for route lookup?
> Does Linux kernel support different route for different L4 ports?
> I thought it only uses L3 address for route lookup.
>
> On Mon, Aug 6, 2018 at 11:04 AM, Qiuyu Xiao 
> wrote:
>
>> This patch adds transport ports information for route lookup so that
>> IPsec can select geneve tunnel traffic to do encryption.
>>
>> Signed-off-by: Qiuyu Xiao 
>> Reviewed-by: Greg Rose 
>> Tested-by: Greg Rose 
>> ---
>>  datapath/linux/compat/geneve.c | 29 +++--
>>  1 file changed, 19 insertions(+), 10 deletions(-)
>>
>> diff --git a/datapath/linux/compat/geneve.c
>> b/datapath/linux/compat/geneve.c
>> index 435a23fb7..95a665ddd 100644
>> --- a/datapath/linux/compat/geneve.c
>> +++ b/datapath/linux/compat/geneve.c
>> @@ -836,7 +836,8 @@ free_dst:
>>  static struct rtable *geneve_get_v4_rt(struct sk_buff *skb,
>>struct net_device *dev,
>>struct flowi4 *fl4,
>> -  struct ip_tunnel_info *info)
>> +  struct ip_tunnel_info *info,
>> +  __be16 dport, __be16 sport)
>>  {
>> bool use_cache = ip_tunnel_dst_cache_usable(skb, info);
>> struct geneve_dev *geneve = netdev_priv(dev);
>> @@ -850,6 +851,8 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff
>> *skb,
>> memset(fl4, 0, sizeof(*fl4));
>> fl4->flowi4_mark = skb->mark;
>> fl4->flowi4_proto = IPPROTO_UDP;
>> +   fl4->fl4_dport = dport;
>> +   fl4->fl4_sport = sport;
>
>
> the route entry (rt) is from
>   rt = ip_route_output_key(geneve->net, fl4)
>
> Does different dport and sport return different route lookup entry?
>
> Thanks
> William
>
>
>>
> if (info) {
>> fl4->daddr = info->key.u.ipv4.dst;
>> @@ -895,7 +898,8 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff
>> *skb,
>>  static struct dst_entry *geneve_get_v6_dst(struct sk_buff *skb,
>>struct net_device *dev,
>>struct flowi6 *fl6,
>> -  struct ip_tunnel_info *info)
>> +  struct ip_tunnel_info *info,
>> + __be16 dport, __be16 sport)
>>  {
>> bool use_cache = ip_tunnel_dst_cache_usable(skb, info);
>> struct geneve_dev *geneve = netdev_priv(dev);
>> @@ -911,6 +915,8 @@ static struct dst_entry *geneve_get_v6_dst(struct
>> sk_buff *skb,
>> memset(fl6, 0, sizeof(*fl6));
>> fl6->flowi6_mark = skb->mark;
>> fl6->flowi6_proto = IPPROTO_UDP;
>> +   fl6->fl6_dport = dport;
>> +   fl6->fl6_sport = sport;
>>
>> if (info) {
>> fl6->daddr = info->key.u.ipv6.dst;
>> @@ -1005,13 +1011,13 @@ static netdev_tx_t geneve_xmit_skb(struct sk_buff
>> *skb, struct net_device *dev,
>> goto tx_error;
>> }
>>
>> -   rt = geneve_get_v4_rt(skb, dev, , info);
>> +   sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
>> +   rt = geneve_get_v4_rt(skb, dev, , info, geneve->dst_port,
>> sport);
>> if (IS_ERR(rt)) {
>> err = PTR_ERR(rt);
>> goto tx_error;
>> }
>>
>> -   sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
>> skb_reset_mac_header(skb);
>>
>> iip = ip_hdr(skb);
>> @@ -1097,13 +1103,13 @@ static netdev_tx_t geneve6_xmit_skb(struct
>> sk_buff *skb, struct net_device *dev,
>> }
>> }
>>
>> -   dst = geneve_get_v6_dst(skb, dev, , info);
>> +   sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
>> +   dst = geneve_get_v6_dst(skb, dev, , info, geneve->dst_port,
>> sport);
>> if (IS_ERR(dst)) {
>> err = PTR_ERR(dst);
>> goto tx_error;
>> }
>>
>> -   sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
>> skb_reset_mac_header(skb);
>>
>> iip = ip_hdr(skb);
>> @@ -1232,13 +1238,17 @@ int ovs_geneve_fill_metadata_dst(struct
>> net_device *dev, struct sk_buff *skb)
>> struct geneve_dev *geneve = 

Re: [ovs-dev] [PATCH v5 3/6] debian and rhel: Create IPsec package.

[Ben Pfaff]

On Thu, Aug 09, 2018 at 12:40:39PM -0700, Ansis Atteka wrote:
> On Tue, 7 Aug 2018 at 09:43, Qiuyu Xiao  wrote:
> >
> > Added rules and files to create debian and rpm ovs-ipsec packages.
> >
> > Signed-off-by: Qiuyu Xiao 
> > Signed-off-by: Ansis Atteka 
> > Co-authored-by: Ansis Atteka 
> 
> Did you test this patch on Fedora with SElinux enabled?
> ovs-monitor-ipsec daemon fails to start. You need to create SElinux
> policy too:

Is that something you can help with?  I doubt that Qiuyu has much
experience with SELinux (and I don't either).
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[William Tu]

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

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

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

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

[William Tu]

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

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

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

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

[William Tu]

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

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

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

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

[Gregory Rose]

On 8/9/2018 11:36 AM, Yifeng Sun wrote:

Yes, I agree. It should be the way to go.

Thanks,
Yifeng



If I'm reading the code correctly the OVS packet cmd will set the 
skb->cb using the OVS_CB macro and then
pass that packet down to the ip6erspan_tunnel_xmit function.  The 
ip6erspan_tunnel_xmit function will
then use the IPCB macro to access the skb->cb area to set the flag to 
zero.  That works fine for packets
generated by the system that have used the IPCB macro accessor to set 
the skb->cb data.  However, it
is catastrophic when an skb was prepared with the OVS_CB macro because, 
as you might imagine, it
is not pointing to an inet_skb_parm structure but an ovs_skb_cb 
structure instead.


This code was *never* correct.  Somehow or other it didn't cause a 
problem until 4.15.  We'll have to
figure out some way to check if the packet is ours or is system 
generated to know if we should be using

the IPCB accessor.

In instances of packets generated by the system or any other entity 
transmitting ipv6 packets over the

tunnel we'll need to keep the code that clears the flags.

Thanks,

- Greg

On Thu, Aug 9, 2018 at 11:17 AM, Gregory Rose > wrote:


On 8/9/2018 11:03 AM, Yifeng Sun wrote:

There is a difference regarding how skb_tunnel_info is stored in skb
between ovs and upstream kernel. In ovs's compatible gre module,
skb_tunnel_info is stored in skb->cb while in upstream kernel, it is
referenced by skb->_skb_refdst.

The upstream netdev code should be okay.

To fix this issue, my guess is that either we comply to the
kernel's way
by using skb->_skb_refdst to store skb_tunnel_info, or we don't use
IPCB at all.


Ah, I see...

We must comply with the kernel method for any given kernel.  We
can't be sure that we'll only handle
or own packets.

Can't this be fixed by a compatibility layer #define in
acinclude.m4 so that kernels that store it in
skb->cb vs. kernels that store it in skb->__skb_refdst will both work?

Thanks,

- Greg




Thanks,
Yifeng



On Thu, Aug 9, 2018 at 10:28 AM, Gregory Rose
mailto:gvrose8...@gmail.com>> wrote:


On 8/8/2018 6:50 PM, William Tu wrote:

thanks for the fix.

On Wed, Aug 8, 2018 at 11:32 AM, Yifeng Sun
mailto:pkusunyif...@gmail.com>>
wrote:

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

can you explain more about ovs_gso_cb?

Signed-off-by: Yifeng Sun mailto:pkusunyif...@gmail.com>>
---
  datapath/linux/compat/ip6_gre.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/datapath/linux/compat/ip6_gre.c
b/datapath/linux/compat/ip6_
gre.c
index 54a76ab..16c1f72 100644
--- a/datapath/linux/compat/ip6_gre.c
+++ b/datapath/linux/compat/ip6_gre.c
@@ -1146,7 +1146,6 @@ static netdev_tx_t
ip6erspan_tunnel_xmit(struct
sk_buff *skb,
                 goto tx_err;

         t->parms.o_flags &= ~TUNNEL_KEY;
-       IPCB(skb)->flags = 0;

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


That would be the normal work flow.

Yifeng,

Can you please post a patch with this fix to netdev?  Taking
William's comments into account as well.

Good catch and thanks for the fix!

- Greg


Thanks,
William
___
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 v5 1/9] datapath: add transport ports in route lookup for geneve

[William Tu]

Hi Qiuyu,

Can you explain a little more about why you need L4 ports for route lookup?
Does Linux kernel support different route for different L4 ports?
I thought it only uses L3 address for route lookup.

On Mon, Aug 6, 2018 at 11:04 AM, Qiuyu Xiao 
wrote:

> This patch adds transport ports information for route lookup so that
> IPsec can select geneve tunnel traffic to do encryption.
>
> Signed-off-by: Qiuyu Xiao 
> Reviewed-by: Greg Rose 
> Tested-by: Greg Rose 
> ---
>  datapath/linux/compat/geneve.c | 29 +++--
>  1 file changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/datapath/linux/compat/geneve.c b/datapath/linux/compat/
> geneve.c
> index 435a23fb7..95a665ddd 100644
> --- a/datapath/linux/compat/geneve.c
> +++ b/datapath/linux/compat/geneve.c
> @@ -836,7 +836,8 @@ free_dst:
>  static struct rtable *geneve_get_v4_rt(struct sk_buff *skb,
>struct net_device *dev,
>struct flowi4 *fl4,
> -  struct ip_tunnel_info *info)
> +  struct ip_tunnel_info *info,
> +  __be16 dport, __be16 sport)
>  {
> bool use_cache = ip_tunnel_dst_cache_usable(skb, info);
> struct geneve_dev *geneve = netdev_priv(dev);
> @@ -850,6 +851,8 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff
> *skb,
> memset(fl4, 0, sizeof(*fl4));
> fl4->flowi4_mark = skb->mark;
> fl4->flowi4_proto = IPPROTO_UDP;
> +   fl4->fl4_dport = dport;
> +   fl4->fl4_sport = sport;


the route entry (rt) is from
  rt = ip_route_output_key(geneve->net, fl4)

Does different dport and sport return different route lookup entry?

Thanks
William


>
if (info) {
> fl4->daddr = info->key.u.ipv4.dst;
> @@ -895,7 +898,8 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff
> *skb,
>  static struct dst_entry *geneve_get_v6_dst(struct sk_buff *skb,
>struct net_device *dev,
>struct flowi6 *fl6,
> -  struct ip_tunnel_info *info)
> +  struct ip_tunnel_info *info,
> + __be16 dport, __be16 sport)
>  {
> bool use_cache = ip_tunnel_dst_cache_usable(skb, info);
> struct geneve_dev *geneve = netdev_priv(dev);
> @@ -911,6 +915,8 @@ static struct dst_entry *geneve_get_v6_dst(struct
> sk_buff *skb,
> memset(fl6, 0, sizeof(*fl6));
> fl6->flowi6_mark = skb->mark;
> fl6->flowi6_proto = IPPROTO_UDP;
> +   fl6->fl6_dport = dport;
> +   fl6->fl6_sport = sport;
>
> if (info) {
> fl6->daddr = info->key.u.ipv6.dst;
> @@ -1005,13 +1011,13 @@ static netdev_tx_t geneve_xmit_skb(struct sk_buff
> *skb, struct net_device *dev,
> goto tx_error;
> }
>
> -   rt = geneve_get_v4_rt(skb, dev, , info);
> +   sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
> +   rt = geneve_get_v4_rt(skb, dev, , info, geneve->dst_port,
> sport);
> if (IS_ERR(rt)) {
> err = PTR_ERR(rt);
> goto tx_error;
> }
>
> -   sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
> skb_reset_mac_header(skb);
>
> iip = ip_hdr(skb);
> @@ -1097,13 +1103,13 @@ static netdev_tx_t geneve6_xmit_skb(struct sk_buff
> *skb, struct net_device *dev,
> }
> }
>
> -   dst = geneve_get_v6_dst(skb, dev, , info);
> +   sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
> +   dst = geneve_get_v6_dst(skb, dev, , info, geneve->dst_port,
> sport);
> if (IS_ERR(dst)) {
> err = PTR_ERR(dst);
> goto tx_error;
> }
>
> -   sport = udp_flow_src_port(geneve->net, skb, 1, USHRT_MAX, true);
> skb_reset_mac_header(skb);
>
> iip = ip_hdr(skb);
> @@ -1232,13 +1238,17 @@ int ovs_geneve_fill_metadata_dst(struct
> net_device *dev, struct sk_buff *skb)
> struct geneve_dev *geneve = netdev_priv(dev);
> struct rtable *rt;
> struct flowi4 fl4;
> +   __be16 sport;
>  #if IS_ENABLED(CONFIG_IPV6)
> struct dst_entry *dst;
> struct flowi6 fl6;
>  #endif
>
> +   sport = udp_flow_src_port(geneve->net, skb,
> +1, USHRT_MAX, true);
> +
> if (ip_tunnel_info_af(info) == AF_INET) {
> -   rt = geneve_get_v4_rt(skb, dev, , info);
> +   rt = geneve_get_v4_rt(skb, dev, , info,
> geneve->dst_port, sport);
> if (IS_ERR(rt))
> return PTR_ERR(rt);
>
> @@ -1246,7 +1256,7 @@ int ovs_geneve_fill_metadata_dst(struct net_device
> *dev, struct sk_buff *skb)
> info->key.u.ipv4.src = fl4.saddr;
>  

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

[Yifeng Sun]

Yes, exactly.

Thanks,
Yifeng

On Thu, Aug 9, 2018 at 1:13 PM, William Tu  wrote:

>
>
> On Thu, Aug 9, 2018 at 12:52 PM, Yifeng Sun 
> wrote:
>
>> The above packet hex is supposed to be generated by linux native tunnel.
>> But since native tunnel conflicts with openvswitch kernel module, so we
>> don't add native tunnel but just send simulated packets to OVS.
>>
>> The ERSPAN implementation of OVS is still tested on br0 and br-underlay.
>> In this test, br0 actually generate ERSPAN packets, which are captured on
>> p0
>> through tcpdump.
>>
>>
> I see, thanks!
> So the hex packet is actually an incoming ERSPAN packet to br-underlay.
> As a result, it triggers OVS's ERSPAN tunnel decap as well as encap,
> when we see it in tcpdump.
>
> Regards,
> William
>
>
>
>>
>> On Thu, Aug 9, 2018 at 12:09 PM, William Tu  wrote:
>>
>>>
>>>
>>> On Wed, Aug 8, 2018 at 11:32 AM, Yifeng Sun 
>>> wrote:
>>>
 Introduce a new test that doesn't setup native erspan v2 tunnels but
 sends
 simulated raw packets.
 This test is supposed to only run for kernel version from 4.4.x to
 4.15.x.

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

 diff --git a/tests/system-traffic.at b/tests/system-traffic.at
 index 44669f8..64b37df 100644
 --- a/tests/system-traffic.at
 +++ b/tests/system-traffic.at
 @@ -670,6 +670,50 @@ AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP
 172.31.1.100 > 172.31.1.1: GRE
  OVS_TRAFFIC_VSWITCHD_STOP
  AT_CLEANUP

 +AT_SETUP([datapath - ping over erspan v2 tunnel by simulated packets])
 +OVS_CHECK_KERNEL(4, 4, 15)
 +
 +OVS_TRAFFIC_VSWITCHD_START()
 +AT_CHECK([ovs-vsctl -- set bridge br0 other-config:hwaddr=\"f2:ff:00
 :00:00:01\"])
 +ADD_BR([br-underlay], [set bridge br-underlay
 other-config:hwaddr=\"f2:ff:00:00:00:02\"])
 +
 +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
 +AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
 +
 +ADD_NAMESPACES(at_ns0)
 +
 +dnl Set up underlay link from host into the namespace using veth pair.
 +ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24", f2:ff:00:00:00:03)
 +AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
 +AT_CHECK([ip link set dev br-underlay up])
 +
 +dnl Set up tunnel endpoints on OVS outside the namespace and simulate
 a native
 +dnl linux device inside the namespace.
 +ADD_OVS_TUNNEL([erspan], [br0], [at_erspan0], [172.31.1.1], [
 10.1.1.100/24], [options:key=1 options:erspan_ver=2
 options:erspan_dir=1 options:erspan_hwid=0x7])
 +
 +ip netns exec at_ns0 tcpdump -U -i p0 -w p0.pcap &
 +sleep 1
 +
 +dnl First, check the underlay
 +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 |
 FORMAT_PING], [0], [dnl
 +3 packets transmitted, 3 received, 0% packet loss, time 0ms
 +])
 +
 +dnl Okay, send raw arp request and icmp echo request.
 +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1
 packet=f2ff0002f2ff000308004552373d4000402fa89ca
 c1f0101ac1f0164100088be000620016f54b4178078f
 2ff000408060001080006040001f2ff00040a0101010a010164
 actions=normal"
 +
 +sleep 1
 +AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "0806 0001 0800 0604
 0002 f2ff " 2>&1 1>/dev/null])
 +AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "0a01 0164 f2ff 
 0004 0a01 0101" 2>&1 1>/dev/null])
 +
 +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1
 packet=f2ff0002f2ff00030800459287e14000402f57b8a
 c1f0101ac1f0164100088be00052001144cd5a48078f2ff0
 001f2ff00040800455c38d640004001eb640a0101010a010
 16408005e57585f0001df6c6b5b45bc0500101112131
 415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f
 actions=normal"

>>>
>>> The above packet hex is already encapsulated with ERSPAN
>>> (https://www.gasmi.net/hpd/ can help decode)
>>> So this packet does not exercise the ERSPAN implementation in OVS.
>>>
>>> Can we generate packet from br0 instead of br-underlay?
>>>
>>> Thanks
>>> William
>>>
>>>
 +
 +sleep 1
 +AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP 172.31.1.100 >
 172.31.1.1: GREv0, .* length 126" 2>&1 1>/dev/null])
 +
 +OVS_TRAFFIC_VSWITCHD_STOP
 +AT_CLEANUP
 +
  AT_SETUP([datapath - clone action])
  OVS_TRAFFIC_VSWITCHD_START()

 --
 2.7.4

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

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

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

[William Tu]

On Thu, Aug 9, 2018 at 12:52 PM, Yifeng Sun  wrote:

> The above packet hex is supposed to be generated by linux native tunnel.
> But since native tunnel conflicts with openvswitch kernel module, so we
> don't add native tunnel but just send simulated packets to OVS.
>
> The ERSPAN implementation of OVS is still tested on br0 and br-underlay.
> In this test, br0 actually generate ERSPAN packets, which are captured on
> p0
> through tcpdump.
>
>
I see, thanks!
So the hex packet is actually an incoming ERSPAN packet to br-underlay.
As a result, it triggers OVS's ERSPAN tunnel decap as well as encap,
when we see it in tcpdump.

Regards,
William



>
> On Thu, Aug 9, 2018 at 12:09 PM, William Tu  wrote:
>
>>
>>
>> On Wed, Aug 8, 2018 at 11:32 AM, Yifeng Sun 
>> wrote:
>>
>>> Introduce a new test that doesn't setup native erspan v2 tunnels but
>>> sends
>>> simulated raw packets.
>>> This test is supposed to only run for kernel version from 4.4.x to
>>> 4.15.x.
>>>
>>> Signed-off-by: Yifeng Sun 
>>> ---
>>>  tests/system-traffic.at | 44 ++
>>> ++
>>>  1 file changed, 44 insertions(+)
>>>
>>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>>> index 44669f8..64b37df 100644
>>> --- a/tests/system-traffic.at
>>> +++ b/tests/system-traffic.at
>>> @@ -670,6 +670,50 @@ AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP
>>> 172.31.1.100 > 172.31.1.1: GRE
>>>  OVS_TRAFFIC_VSWITCHD_STOP
>>>  AT_CLEANUP
>>>
>>> +AT_SETUP([datapath - ping over erspan v2 tunnel by simulated packets])
>>> +OVS_CHECK_KERNEL(4, 4, 15)
>>> +
>>> +OVS_TRAFFIC_VSWITCHD_START()
>>> +AT_CHECK([ovs-vsctl -- set bridge br0 other-config:hwaddr=\"f2:ff:00
>>> :00:00:01\"])
>>> +ADD_BR([br-underlay], [set bridge br-underlay
>>> other-config:hwaddr=\"f2:ff:00:00:00:02\"])
>>> +
>>> +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
>>> +AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
>>> +
>>> +ADD_NAMESPACES(at_ns0)
>>> +
>>> +dnl Set up underlay link from host into the namespace using veth pair.
>>> +ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24", f2:ff:00:00:00:03)
>>> +AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
>>> +AT_CHECK([ip link set dev br-underlay up])
>>> +
>>> +dnl Set up tunnel endpoints on OVS outside the namespace and simulate a
>>> native
>>> +dnl linux device inside the namespace.
>>> +ADD_OVS_TUNNEL([erspan], [br0], [at_erspan0], [172.31.1.1], [
>>> 10.1.1.100/24], [options:key=1 options:erspan_ver=2
>>> options:erspan_dir=1 options:erspan_hwid=0x7])
>>> +
>>> +ip netns exec at_ns0 tcpdump -U -i p0 -w p0.pcap &
>>> +sleep 1
>>> +
>>> +dnl First, check the underlay
>>> +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 |
>>> FORMAT_PING], [0], [dnl
>>> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
>>> +])
>>> +
>>> +dnl Okay, send raw arp request and icmp echo request.
>>> +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1
>>> packet=f2ff0002f2ff000308004552373d4000402fa89ca
>>> c1f0101ac1f0164100088be000620016f54b4178078f
>>> 2ff000408060001080006040001f2ff00040a0101010a010164
>>> actions=normal"
>>> +
>>> +sleep 1
>>> +AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "0806 0001 0800 0604 0002
>>> f2ff " 2>&1 1>/dev/null])
>>> +AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "0a01 0164 f2ff  0004
>>> 0a01 0101" 2>&1 1>/dev/null])
>>> +
>>> +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1
>>> packet=f2ff0002f2ff00030800459287e14000402f57b8a
>>> c1f0101ac1f0164100088be00052001144cd5a48078f2ff0
>>> 001f2ff00040800455c38d640004001eb640a0101010a010
>>> 16408005e57585f0001df6c6b5b45bc0500101112131
>>> 415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f
>>> actions=normal"
>>>
>>
>> The above packet hex is already encapsulated with ERSPAN
>> (https://www.gasmi.net/hpd/ can help decode)
>> So this packet does not exercise the ERSPAN implementation in OVS.
>>
>> Can we generate packet from br0 instead of br-underlay?
>>
>> Thanks
>> William
>>
>>
>>> +
>>> +sleep 1
>>> +AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP 172.31.1.100 >
>>> 172.31.1.1: GREv0, .* length 126" 2>&1 1>/dev/null])
>>> +
>>> +OVS_TRAFFIC_VSWITCHD_STOP
>>> +AT_CLEANUP
>>> +
>>>  AT_SETUP([datapath - clone action])
>>>  OVS_TRAFFIC_VSWITCHD_START()
>>>
>>> --
>>> 2.7.4
>>>
>>> ___
>>> dev mailing list
>>> d...@openvswitch.org
>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>
>>
>>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[William Tu]

On Thu, Aug 9, 2018 at 12:46 PM, Yifeng Sun  wrote:

> Yes, in original test, ADD_NATIVE_TUNNEL will try to load upstream
> gre modules. But since openvswitch is running in compatible model,
> there is a conflict and the gre tests will always fail. So this new series
> of tests abandoned using ADD_NATIVE_TUNNEL and doesn't need
> native gre modules any more.
>
>
oh, my mistake, the line is actually a comment.
+dnl ADD_NATIVE_TUNNEL([gretap], [ns_gre0], [at_ns0], [172.31.1.100], [
10.1.1.1/24])
I thought it is active.

Thanks
William



> On Thu, Aug 9, 2018 at 11:55 AM, William Tu  wrote:
>
>>
>>
>> On Wed, Aug 8, 2018 at 11:32 AM, Yifeng Sun 
>> wrote:
>>
>>> Introduce a new test that doesn't setup native gre tunnels but sends
>>> simulated raw packets.
>>> This test is supposed to only run for kernel version from 4.4.x to
>>> 4.15.x.
>>>
>>> Signed-off-by: Yifeng Sun 
>>> ---
>>>  tests/system-traffic.at | 47 ++
>>> +
>>>  1 file changed, 47 insertions(+)
>>>
>>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>>> index cf53c10..dca2bc8 100644
>>> --- a/tests/system-traffic.at
>>> +++ b/tests/system-traffic.at
>>> @@ -575,6 +575,53 @@ NS_CHECK_EXEC([at_ns0], [ping -s 3200 -q -c 3 -i
>>> 0.3 -w 2 10.1.1.100 | FORMAT_PI
>>>  OVS_TRAFFIC_VSWITCHD_STOP
>>>  AT_CLEANUP
>>>
>>> +AT_SETUP([datapath - ping over gre tunnel by simulated packets])
>>> +OVS_CHECK_KERNEL(4, 4, 15)
>>> +
>>> +OVS_TRAFFIC_VSWITCHD_START()
>>> +
>>> +AT_CHECK([ovs-vsctl -- set bridge br0 other-config:hwaddr=\"f2:ff:00
>>> :00:00:01\"])
>>> +ADD_BR([br-underlay], [set bridge br-underlay
>>> other-config:hwaddr=\"f2:ff:00:00:00:02\"])
>>> +
>>> +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
>>> +AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
>>> +
>>> +ADD_NAMESPACES(at_ns0)
>>> +
>>> +dnl Set up underlay link from host into the namespace using veth pair.
>>> +ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24", f2:ff:00:00:00:03)
>>> +AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
>>> +AT_CHECK([ip link set dev br-underlay up])
>>> +
>>> +dnl Set up tunnel endpoints on OVS outside the namespace.
>>> +ADD_OVS_TUNNEL([gre], [br0], [at_gre0], [172.31.1.1], [10.1.1.100/24])
>>> +
>>
>> +ip netns exec at_ns0 tcpdump -U -i p0 -w p0.pcap &
>>> +sleep 1
>>> +
>>> +dnl First, check the underlay.
>>> +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 |
>>> FORMAT_PING], [0], [dnl
>>> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
>>> +])
>>> +
>>> +dnl We don't actually add gretap port as below, instead, we will
>>> +dnl emulate one that sends out packets. Suppose its mac address is
>>> f2:ff:00:00:00:04.
>>> +dnl ADD_NATIVE_TUNNEL([gretap], [ns_gre0], [at_ns0], [172.31.1.100], [
>>> 10.1.1.1/24])
>>>
>>
>> Doesn't ADD_NATIVE_TUNNEL setup native gre tunnels?
>> And this causes loading the upstream kernel's gre module.
>>
>> Thanks
>> William
>>
>>> +
>>> +dnl Now, check the overlay by sending out raw arp and icmp packets.
>>> +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1
>>> packet=f2ff0002f2ff000308004542ec2c4000402ff3bca
>>> c1f0101ac1f01646558f2ff00040806000108000
>>> 6040001f2ff00040a0101010a010164 actions=NORMAL"
>>> +
>>> +sleep 1
>>> +AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP 172.31.1.100 >
>>> 172.31.1.1: GREv0, length 46: ARP, Reply 10.1.1.100 is-at
>>> f2:ff:00:00:00:01 .* length 28" 2>&1 1>/dev/null])
>>> +
>>> +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1
>>> packet=f2ff0002f2ff00030800457aec8e4000402ff322a
>>> c1f0101ac1f01646558f2ff0001f2ff0004080045545
>>> 48f40004001cfb30a0101010a0101640800e6e829270003e1a3435b0
>>> 000ff1a0500101112131415161718191a1b1c1d1e1f202122232
>>> 425262728292a2b2c2d2e2f3031323334353637 actions=NORMAL"
>>> +
>>> +sleep 1
>>> +AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP 172.31.1.100 >
>>> 172.31.1.1: GREv0, length 102: IP 10.1.1.100 > 10.1.1.1: ICMP echo
>>> reply, .* length 64$" 2>&1 1>/dev/null])
>>> +
>>> +OVS_TRAFFIC_VSWITCHD_STOP
>>> +AT_CLEANUP
>>> +
>>>  AT_SETUP([datapath - clone action])
>>>  OVS_TRAFFIC_VSWITCHD_START()
>>>
>>> --
>>> 2.7.4
>>>
>>> ___
>>> dev mailing list
>>> d...@openvswitch.org
>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>
>>
>>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Yifeng Sun]

The above packet hex is supposed to be generated by linux native tunnel.
But since native tunnel conflicts with openvswitch kernel module, so we
don't add native tunnel but just send simulated packets to OVS.

The ERSPAN implementation of OVS is still tested on br0 and br-underlay.
In this test, br0 actually generate ERSPAN packets, which are captured on p0
through tcpdump.


On Thu, Aug 9, 2018 at 12:09 PM, William Tu  wrote:

>
>
> On Wed, Aug 8, 2018 at 11:32 AM, Yifeng Sun 
> wrote:
>
>> Introduce a new test that doesn't setup native erspan v2 tunnels but sends
>> simulated raw packets.
>> This test is supposed to only run for kernel version from 4.4.x to 4.15.x.
>>
>> Signed-off-by: Yifeng Sun 
>> ---
>>  tests/system-traffic.at | 44 ++
>> ++
>>  1 file changed, 44 insertions(+)
>>
>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>> index 44669f8..64b37df 100644
>> --- a/tests/system-traffic.at
>> +++ b/tests/system-traffic.at
>> @@ -670,6 +670,50 @@ AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP
>> 172.31.1.100 > 172.31.1.1: GRE
>>  OVS_TRAFFIC_VSWITCHD_STOP
>>  AT_CLEANUP
>>
>> +AT_SETUP([datapath - ping over erspan v2 tunnel by simulated packets])
>> +OVS_CHECK_KERNEL(4, 4, 15)
>> +
>> +OVS_TRAFFIC_VSWITCHD_START()
>> +AT_CHECK([ovs-vsctl -- set bridge br0 other-config:hwaddr=\"f2:ff:00
>> :00:00:01\"])
>> +ADD_BR([br-underlay], [set bridge br-underlay
>> other-config:hwaddr=\"f2:ff:00:00:00:02\"])
>> +
>> +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
>> +AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
>> +
>> +ADD_NAMESPACES(at_ns0)
>> +
>> +dnl Set up underlay link from host into the namespace using veth pair.
>> +ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24", f2:ff:00:00:00:03)
>> +AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
>> +AT_CHECK([ip link set dev br-underlay up])
>> +
>> +dnl Set up tunnel endpoints on OVS outside the namespace and simulate a
>> native
>> +dnl linux device inside the namespace.
>> +ADD_OVS_TUNNEL([erspan], [br0], [at_erspan0], [172.31.1.1], [
>> 10.1.1.100/24], [options:key=1 options:erspan_ver=2 options:erspan_dir=1
>> options:erspan_hwid=0x7])
>> +
>> +ip netns exec at_ns0 tcpdump -U -i p0 -w p0.pcap &
>> +sleep 1
>> +
>> +dnl First, check the underlay
>> +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 |
>> FORMAT_PING], [0], [dnl
>> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> +])
>> +
>> +dnl Okay, send raw arp request and icmp echo request.
>> +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1
>> packet=f2ff0002f2ff000308004552373d4000402fa89ca
>> c1f0101ac1f0164100088be000620016f54b4178078f
>> 2ff000408060001080006040001f2ff00040a0101010a010164
>> actions=normal"
>> +
>> +sleep 1
>> +AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "0806 0001 0800 0604 0002
>> f2ff " 2>&1 1>/dev/null])
>> +AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "0a01 0164 f2ff  0004
>> 0a01 0101" 2>&1 1>/dev/null])
>> +
>> +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1
>> packet=f2ff0002f2ff00030800459287e14000402f57b8a
>> c1f0101ac1f0164100088be00052001144cd5a48078f2ff0
>> 001f2ff00040800455c38d640004001eb640a0101010a010
>> 16408005e57585f0001df6c6b5b45bc0500101112131
>> 415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f
>> actions=normal"
>>
>
> The above packet hex is already encapsulated with ERSPAN
> (https://www.gasmi.net/hpd/ can help decode)
> So this packet does not exercise the ERSPAN implementation in OVS.
>
> Can we generate packet from br0 instead of br-underlay?
>
> Thanks
> William
>
>
>> +
>> +sleep 1
>> +AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP 172.31.1.100 >
>> 172.31.1.1: GREv0, .* length 126" 2>&1 1>/dev/null])
>> +
>> +OVS_TRAFFIC_VSWITCHD_STOP
>> +AT_CLEANUP
>> +
>>  AT_SETUP([datapath - clone action])
>>  OVS_TRAFFIC_VSWITCHD_START()
>>
>> --
>> 2.7.4
>>
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Yifeng Sun]

Good question. I tried on RHEL7x but somehow these tests failed.
For now, I have run these tests on 4.4, 4.15, 4.16 and they all passed.

On Thu, Aug 9, 2018 at 11:59 AM, William Tu  wrote:

>
>
> On Wed, Aug 8, 2018 at 11:32 AM, Yifeng Sun 
> wrote:
>
>> Skip gre, erspan and ip6erspan related tests on kernel version from
>> 4.4.x to 4.15.x because these tests will always fail.
>>
>> Signed-off-by: Yifeng Sun 
>> ---
>>  tests/system-traffic.at | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>> index cbd9542..cf53c10 100644
>> --- a/tests/system-traffic.at
>> +++ b/tests/system-traffic.at
>> @@ -300,6 +300,7 @@ OVS_TRAFFIC_VSWITCHD_STOP
>>  AT_CLEANUP
>>
>>  AT_SETUP([datapath - ping over gre tunnel])
>> +OVS_CHECK_KERNEL_EXCL(4, 4, 15)
>>
>
> How about RHEL7x? Which uses 3.10.xxx?
>
> William
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Yifeng Sun]

Yes, in original test, ADD_NATIVE_TUNNEL will try to load upstream
gre modules. But since openvswitch is running in compatible model,
there is a conflict and the gre tests will always fail. So this new series
of tests abandoned using ADD_NATIVE_TUNNEL and doesn't need
native gre modules any more.

On Thu, Aug 9, 2018 at 11:55 AM, William Tu  wrote:

>
>
> On Wed, Aug 8, 2018 at 11:32 AM, Yifeng Sun 
> wrote:
>
>> Introduce a new test that doesn't setup native gre tunnels but sends
>> simulated raw packets.
>> This test is supposed to only run for kernel version from 4.4.x to 4.15.x.
>>
>> Signed-off-by: Yifeng Sun 
>> ---
>>  tests/system-traffic.at | 47 ++
>> +
>>  1 file changed, 47 insertions(+)
>>
>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>> index cf53c10..dca2bc8 100644
>> --- a/tests/system-traffic.at
>> +++ b/tests/system-traffic.at
>> @@ -575,6 +575,53 @@ NS_CHECK_EXEC([at_ns0], [ping -s 3200 -q -c 3 -i 0.3
>> -w 2 10.1.1.100 | FORMAT_PI
>>  OVS_TRAFFIC_VSWITCHD_STOP
>>  AT_CLEANUP
>>
>> +AT_SETUP([datapath - ping over gre tunnel by simulated packets])
>> +OVS_CHECK_KERNEL(4, 4, 15)
>> +
>> +OVS_TRAFFIC_VSWITCHD_START()
>> +
>> +AT_CHECK([ovs-vsctl -- set bridge br0 other-config:hwaddr=\"f2:ff:00
>> :00:00:01\"])
>> +ADD_BR([br-underlay], [set bridge br-underlay
>> other-config:hwaddr=\"f2:ff:00:00:00:02\"])
>> +
>> +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
>> +AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
>> +
>> +ADD_NAMESPACES(at_ns0)
>> +
>> +dnl Set up underlay link from host into the namespace using veth pair.
>> +ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24", f2:ff:00:00:00:03)
>> +AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
>> +AT_CHECK([ip link set dev br-underlay up])
>> +
>> +dnl Set up tunnel endpoints on OVS outside the namespace.
>> +ADD_OVS_TUNNEL([gre], [br0], [at_gre0], [172.31.1.1], [10.1.1.100/24])
>> +
>
> +ip netns exec at_ns0 tcpdump -U -i p0 -w p0.pcap &
>> +sleep 1
>> +
>> +dnl First, check the underlay.
>> +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 |
>> FORMAT_PING], [0], [dnl
>> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> +])
>> +
>> +dnl We don't actually add gretap port as below, instead, we will
>> +dnl emulate one that sends out packets. Suppose its mac address is
>> f2:ff:00:00:00:04.
>> +dnl ADD_NATIVE_TUNNEL([gretap], [ns_gre0], [at_ns0], [172.31.1.100], [
>> 10.1.1.1/24])
>>
>
> Doesn't ADD_NATIVE_TUNNEL setup native gre tunnels?
> And this causes loading the upstream kernel's gre module.
>
> Thanks
> William
>
>> +
>> +dnl Now, check the overlay by sending out raw arp and icmp packets.
>> +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1
>> packet=f2ff0002f2ff000308004542ec2c4000402ff3bca
>> c1f0101ac1f01646558f2ff00040806000108000
>> 6040001f2ff00040a0101010a010164 actions=NORMAL"
>> +
>> +sleep 1
>> +AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP 172.31.1.100 >
>> 172.31.1.1: GREv0, length 46: ARP, Reply 10.1.1.100 is-at
>> f2:ff:00:00:00:01 .* length 28" 2>&1 1>/dev/null])
>> +
>> +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1
>> packet=f2ff0002f2ff00030800457aec8e4000402ff322a
>> c1f0101ac1f01646558f2ff0001f2ff0004080045545
>> 48f40004001cfb30a0101010a0101640800e6e829270003e1a3435b0
>> 000ff1a0500101112131415161718191a1b1c1d1e1f202122232
>> 425262728292a2b2c2d2e2f3031323334353637 actions=NORMAL"
>> +
>> +sleep 1
>> +AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP 172.31.1.100 >
>> 172.31.1.1: GREv0, length 102: IP 10.1.1.100 > 10.1.1.1: ICMP echo
>> reply, .* length 64$" 2>&1 1>/dev/null])
>> +
>> +OVS_TRAFFIC_VSWITCHD_STOP
>> +AT_CLEANUP
>> +
>>  AT_SETUP([datapath - clone action])
>>  OVS_TRAFFIC_VSWITCHD_START()
>>
>> --
>> 2.7.4
>>
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v5 3/6] debian and rhel: Create IPsec package.

[Ansis Atteka]

On Tue, 7 Aug 2018 at 09:43, Qiuyu Xiao  wrote:
>
> Added rules and files to create debian and rpm ovs-ipsec packages.
>
> Signed-off-by: Qiuyu Xiao 
> Signed-off-by: Ansis Atteka 
> Co-authored-by: Ansis Atteka 

Did you test this patch on Fedora with SElinux enabled?
ovs-monitor-ipsec daemon fails to start. You need to create SElinux
policy too:

[root@fedoraubuilder vagrant]# systemctl restart openvswitch-ipsec
[root@fedoraubuilder vagrant]# ps -Af | grep ipsec
root  1799   880  0 19:37 pts/000:00:00 grep --color=auto ipsec
[root@fedoraubuilder vagrant]# journalctl -xe| tail -n20
-- Unit openvswitch-ipsec.service has begun starting up.
Aug 09 19:37:16 fedoraubuilder.dev audit[1769]: AVC avc:  denied  {
execute } for  pid=1769 comm="python" name="ldconfig" dev="vda1"
ino=133192 scontext=system_u:system_r:openvswitch_t:s0
tcontext=system_u:object_r:ldconfig_exec_t:s0 tclass=file permissive=0
Aug 09 19:37:16 fedoraubuilder.dev audit[1776]: AVC avc:  denied  {
execute } for  pid=1776 comm="python" name="ldconfig" dev="vda1"
ino=133192 scontext=system_u:system_r:openvswitch_t:s0
tcontext=system_u:object_r:ldconfig_exec_t:s0 tclass=file permissive=0
Aug 09 19:37:16 fedoraubuilder.dev audit[1781]: AVC avc:  denied  {
execute } for  pid=1781 comm="python" name="ldconfig" dev="vda1"
ino=133192 scontext=system_u:system_r:openvswitch_t:s0
tcontext=system_u:object_r:ldconfig_exec_t:s0 tclass=file permissive=0
Aug 09 19:37:16 fedoraubuilder.dev audit[1788]: AVC avc:  denied  {
execute } for  pid=1788 comm="python" name="ipsec" dev="vda1"
ino=149908 scontext=system_u:system_r:openvswitch_t:s0
tcontext=system_u:object_r:ipsec_mgmt_exec_t:s0 tclass=file
permissive=0
Aug 09 19:37:16 fedoraubuilder.dev python[1768]: ovs|  0  |
ovs-monitor-ipsec | ERR | [Errno 13] Permission denied
Aug 09 19:37:16 fedoraubuilder.dev ovs-ctl[1760]: 2018-08-09T19:37:16Z
|  0  | ovs-monitor-ipsec | ERR | [Errno 13] Permission denied
Aug 09 19:37:16 fedoraubuilder.dev ovs-ctl[1789]:
2018-08-09T19:37:16Z|1|daemon_unix|WARN|/var/run/openvswitch/ovs-monitor-ipsec.pid:
open: No such file or directory
Aug 09 19:37:16 fedoraubuilder.dev ovs-appctl[1797]:
ovs|1|daemon_unix|WARN|/var/run/openvswitch/ovs-monitor-ipsec.pid:
open: No such file or directory
Aug 09 19:37:16 fedoraubuilder.dev ovs-ctl[1789]: ovs-appctl: cannot
read pidfile "/var/run/openvswitch/ovs-monitor-ipsec.pid" (No such
file or directory)
Aug 09 19:37:16 fedoraubuilder.dev audit[1]: SERVICE_START pid=1 uid=0
auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0
msg='unit=openvswitch-ipsec comm="systemd"
exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=?
res=success'
Aug 09 19:37:16 fedoraubuilder.dev audit[1]: SERVICE_STOP pid=1 uid=0
auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0
msg='unit=openvswitch-ipsec comm="systemd"
exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=?
res=success'
Aug 09 19:37:16 fedoraubuilder.dev systemd[1]: Started OVS IPsec daemon.
-- Subject: Unit openvswitch-ipsec.service has finished start-up
-- Defined-By: systemd
-- Support: https://lists.freedesktop.org/mailman/listinfo/systemd-devel
-- 
-- Unit openvswitch-ipsec.service has finished starting up.
-- 
-- The start-up result is done.



> ---
>  debian/automake.mk|   3 +
>  debian/control|  21 ++
>  debian/openvswitch-ipsec.dirs |   1 +
>  debian/openvswitch-ipsec.init | 181 ++
>  debian/openvswitch-ipsec.install  |   1 +
>  rhel/automake.mk  |   1 +
>  rhel/openvswitch-fedora.spec.in   |  19 +-
>  ...b_systemd_system_openvswitch-ipsec.service |  12 ++
>  utilities/ovs-ctl.in  |  18 ++
>  9 files changed, 256 insertions(+), 1 deletion(-)
>  create mode 100644 debian/openvswitch-ipsec.dirs
>  create mode 100644 debian/openvswitch-ipsec.init
>  create mode 100644 debian/openvswitch-ipsec.install
>  create mode 100644 rhel/usr_lib_systemd_system_openvswitch-ipsec.service
>
> diff --git a/debian/automake.mk b/debian/automake.mk
> index 4d8e204bb..8a8d43c9f 100644
> --- a/debian/automake.mk
> +++ b/debian/automake.mk
> @@ -20,6 +20,9 @@ EXTRA_DIST += \
> debian/openvswitch-datapath-source.copyright \
> debian/openvswitch-datapath-source.dirs \
> debian/openvswitch-datapath-source.install \
> +   debian/openvswitch-ipsec.dirs \
> +   debian/openvswitch-ipsec.init \
> +   debian/openvswitch-ipsec.install \
> debian/openvswitch-pki.dirs \
> debian/openvswitch-pki.postinst \
> debian/openvswitch-pki.postrm \
> diff --git a/debian/control b/debian/control
> index 9ae248f27..cde93f20e 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -322,3 +322,24 @@ Description: Open vSwitch development package
>   1000V.
>   .
>   This package provides openvswitch headers and libopenvswitch for developers.
> +
> 

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

[William Tu]

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

> Introduce a new test that doesn't setup native erspan v2 tunnels but sends
> simulated raw packets.
> This test is supposed to only run for kernel version from 4.4.x to 4.15.x.
>
> Signed-off-by: Yifeng Sun 
> ---
>  tests/system-traffic.at | 44 
>  1 file changed, 44 insertions(+)
>
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 44669f8..64b37df 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -670,6 +670,50 @@ AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP
> 172.31.1.100 > 172.31.1.1: GRE
>  OVS_TRAFFIC_VSWITCHD_STOP
>  AT_CLEANUP
>
> +AT_SETUP([datapath - ping over erspan v2 tunnel by simulated packets])
> +OVS_CHECK_KERNEL(4, 4, 15)
> +
> +OVS_TRAFFIC_VSWITCHD_START()
> +AT_CHECK([ovs-vsctl -- set bridge br0 other-config:hwaddr=\"f2:ff:
> 00:00:00:01\"])
> +ADD_BR([br-underlay], [set bridge br-underlay other-config:hwaddr=\"f2:ff:
> 00:00:00:02\"])
> +
> +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
> +AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
> +
> +ADD_NAMESPACES(at_ns0)
> +
> +dnl Set up underlay link from host into the namespace using veth pair.
> +ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24", f2:ff:00:00:00:03)
> +AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
> +AT_CHECK([ip link set dev br-underlay up])
> +
> +dnl Set up tunnel endpoints on OVS outside the namespace and simulate a
> native
> +dnl linux device inside the namespace.
> +ADD_OVS_TUNNEL([erspan], [br0], [at_erspan0], [172.31.1.1], [
> 10.1.1.100/24], [options:key=1 options:erspan_ver=2 options:erspan_dir=1
> options:erspan_hwid=0x7])
> +
> +ip netns exec at_ns0 tcpdump -U -i p0 -w p0.pcap &
> +sleep 1
> +
> +dnl First, check the underlay
> +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 172.31.1.100 |
> FORMAT_PING], [0], [dnl
> +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> +])
> +
> +dnl Okay, send raw arp request and icmp echo request.
> +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 packet=
> f2ff0002f2ff000308004552373d4000402fa89cac1f0101
> ac1f0164100088be000620016f54b4178078
> f2ff000408060001080006040001f2ff00040a0101010a010164
> actions=normal"
> +
> +sleep 1
> +AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "0806 0001 0800 0604 0002
> f2ff " 2>&1 1>/dev/null])
> +AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "0a01 0164 f2ff  0004
> 0a01 0101" 2>&1 1>/dev/null])
> +
> +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 packet=
> f2ff0002f2ff00030800459287e14000402f57b8ac1f0101
> ac1f0164100088be00052001144cd5a48078f2ff0001
> f2ff00040800455c38d640004001eb640a0101010a0101640800
> 5e57585f0001df6c6b5b45bc05001011121314151617
> 18191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f
> actions=normal"
>

The above packet hex is already encapsulated with ERSPAN
(https://www.gasmi.net/hpd/ can help decode)
So this packet does not exercise the ERSPAN implementation in OVS.

Can we generate packet from br0 instead of br-underlay?

Thanks
William


> +
> +sleep 1
> +AT_CHECK([tcpdump -xx -r p0.pcap 2>&1 | egrep "IP 172.31.1.100 >
> 172.31.1.1: GREv0, .* length 126" 2>&1 1>/dev/null])
> +
> +OVS_TRAFFIC_VSWITCHD_STOP
> +AT_CLEANUP
> +
>  AT_SETUP([datapath - clone action])
>  OVS_TRAFFIC_VSWITCHD_START()
>
> --
> 2.7.4
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[William Tu]

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

> Skip gre, erspan and ip6erspan related tests on kernel version from
> 4.4.x to 4.15.x because these tests will always fail.
>
> Signed-off-by: Yifeng Sun 
> ---
>  tests/system-traffic.at | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index cbd9542..cf53c10 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -300,6 +300,7 @@ OVS_TRAFFIC_VSWITCHD_STOP
>  AT_CLEANUP
>
>  AT_SETUP([datapath - ping over gre tunnel])
> +OVS_CHECK_KERNEL_EXCL(4, 4, 15)
>

How about RHEL7x? Which uses 3.10.xxx?

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

Re: [ovs-dev] Problem in using ovs in raspberry pi - regd.

[Ashish Varma]

I understand that you have few Rasberry Pi's as switches and some as hosts.
A picture/some more explanation of the topology would help.
How many ports does the Rasberry Pi have. How many are connected to the OVS
bridge?  I understand you use the wireless interface of Rasberry Pi to
connet to RYU.
What are the flows installed on the OVS instances? (Can you give the output
of ovs-ofctl dump-flows  as well as ovs-vsctl show)


On Thu, Aug 2, 2018 at 7:34 PM, Viswanath Muthukumaraswamy Sathananth <
vs527...@dal.ca> wrote:

> Hi,
>
> I am trying to implement a real test bed to check the interaction between
> switches and controllers. Raspberry Pi 3b+ act as switches with OVS version
> 2.5.5 running on them. Two such  Raspberry Pis connected to a RYU
> controller running on Windows 10 Machine wirelessly. One Raspberry Pi which
> is a simple host is attached to every switch. When I try to ping from one
> host to another I get a few problems.
>
> 1. The host raspberry pi is treated as LOCAL. And the flow entries have
> in_port or out_port as LOCAL.
> 2. RYU controller is not able to indetify any links.
> 3. Ping request doesn't echo back.
>
> What can be the possible errors in this case. Kindly suggest us a solution.
>
> Thanks,
>
> Viz
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Yifeng Sun]

Yes, I agree. It should be the way to go.

Thanks,
Yifeng

On Thu, Aug 9, 2018 at 11:17 AM, Gregory Rose  wrote:

> On 8/9/2018 11:03 AM, Yifeng Sun wrote:
>
> There is a difference regarding how skb_tunnel_info is stored in skb
> between ovs and upstream kernel. In ovs's compatible gre module,
> skb_tunnel_info is stored in skb->cb while in upstream kernel, it is
> referenced by skb->_skb_refdst.
>
> The upstream netdev code should be okay.
>
> To fix this issue, my guess is that either we comply to the kernel's way
> by using skb->_skb_refdst to store skb_tunnel_info, or we don't use
> IPCB at all.
>
>
> Ah, I see...
>
> We must comply with the kernel method for any given kernel.  We can't be
> sure that we'll only handle
> or own packets.
>
> Can't this be fixed by a compatibility layer #define in acinclude.m4 so
> that kernels that store it in
> skb->cb vs. kernels that store it in skb->__skb_refdst will both work?
>
> Thanks,
>
> - Greg
>
>
>
> Thanks,
> Yifeng
>
>
>
> On Thu, Aug 9, 2018 at 10:28 AM, Gregory Rose 
> wrote:
>
>>
>> On 8/8/2018 6:50 PM, William Tu wrote:
>>
>>> thanks for the fix.
>>>
>>> On Wed, Aug 8, 2018 at 11:32 AM, Yifeng Sun 
>>> wrote:
>>>
>>> In compatable gre module, skb->cb is used as ovs_gso_cb.
 This bug clears the 16-23 bit in the address of ovs_gso_cb.tun_dst.

 can you explain more about ovs_gso_cb?
>>>
>>> Signed-off-by: Yifeng Sun 
 ---
   datapath/linux/compat/ip6_gre.c | 1 -
   1 file changed, 1 deletion(-)

 diff --git a/datapath/linux/compat/ip6_gre.c
 b/datapath/linux/compat/ip6_
 gre.c
 index 54a76ab..16c1f72 100644
 --- a/datapath/linux/compat/ip6_gre.c
 +++ b/datapath/linux/compat/ip6_gre.c
 @@ -1146,7 +1146,6 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct
 sk_buff *skb,
  goto tx_err;

  t->parms.o_flags &= ~TUNNEL_KEY;
 -   IPCB(skb)->flags = 0;

 The upstream linux kernel has the above code.
>>> Do we need to fix the upstream kernel then backport?
>>>
>>
>> That would be the normal work flow.
>>
>> Yifeng,
>>
>> Can you please post a patch with this fix to netdev?  Taking William's
>> comments into account as well.
>>
>> Good catch and thanks for the fix!
>>
>> - Greg
>>
>>
>>> Thanks,
>>> William
>>> ___
>>> dev mailing list
>>> d...@openvswitch.org
>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>
>>
>>
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Gregory Rose]

On 8/9/2018 11:03 AM, Yifeng Sun wrote:

There is a difference regarding how skb_tunnel_info is stored in skb
between ovs and upstream kernel. In ovs's compatible gre module,
skb_tunnel_info is stored in skb->cb while in upstream kernel, it is
referenced by skb->_skb_refdst.

The upstream netdev code should be okay.

To fix this issue, my guess is that either we comply to the kernel's way
by using skb->_skb_refdst to store skb_tunnel_info, or we don't use
IPCB at all.


Ah, I see...

We must comply with the kernel method for any given kernel.  We can't be 
sure that we'll only handle

or own packets.

Can't this be fixed by a compatibility layer #define in acinclude.m4 so 
that kernels that store it in

skb->cb vs. kernels that store it in skb->__skb_refdst will both work?

Thanks,

- Greg



Thanks,
Yifeng



On Thu, Aug 9, 2018 at 10:28 AM, Gregory Rose > wrote:



On 8/8/2018 6:50 PM, William Tu wrote:

thanks for the fix.

On Wed, Aug 8, 2018 at 11:32 AM, Yifeng Sun
mailto:pkusunyif...@gmail.com>> wrote:

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

can you explain more about ovs_gso_cb?

Signed-off-by: Yifeng Sun mailto:pkusunyif...@gmail.com>>
---
  datapath/linux/compat/ip6_gre.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/datapath/linux/compat/ip6_gre.c
b/datapath/linux/compat/ip6_
gre.c
index 54a76ab..16c1f72 100644
--- a/datapath/linux/compat/ip6_gre.c
+++ b/datapath/linux/compat/ip6_gre.c
@@ -1146,7 +1146,6 @@ static netdev_tx_t
ip6erspan_tunnel_xmit(struct
sk_buff *skb,
                 goto tx_err;

         t->parms.o_flags &= ~TUNNEL_KEY;
-       IPCB(skb)->flags = 0;

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


That would be the normal work flow.

Yifeng,

Can you please post a patch with this fix to netdev?  Taking
William's comments into account as well.

Good catch and thanks for the fix!

- Greg


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






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

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

[Yifeng Sun]

There is a difference regarding how skb_tunnel_info is stored in skb
between ovs and upstream kernel. In ovs's compatible gre module,
skb_tunnel_info is stored in skb->cb while in upstream kernel, it is
referenced by skb->_skb_refdst.

The upstream netdev code should be okay.

To fix this issue, my guess is that either we comply to the kernel's way
by using skb->_skb_refdst to store skb_tunnel_info, or we don't use
IPCB at all.

Thanks,
Yifeng



On Thu, Aug 9, 2018 at 10:28 AM, Gregory Rose  wrote:

>
> On 8/8/2018 6:50 PM, William Tu wrote:
>
>> thanks for the fix.
>>
>> On Wed, Aug 8, 2018 at 11:32 AM, Yifeng Sun 
>> wrote:
>>
>> In compatable gre module, skb->cb is used as ovs_gso_cb.
>>> This bug clears the 16-23 bit in the address of ovs_gso_cb.tun_dst.
>>>
>>> can you explain more about ovs_gso_cb?
>>
>> Signed-off-by: Yifeng Sun 
>>> ---
>>>   datapath/linux/compat/ip6_gre.c | 1 -
>>>   1 file changed, 1 deletion(-)
>>>
>>> diff --git a/datapath/linux/compat/ip6_gre.c
>>> b/datapath/linux/compat/ip6_
>>> gre.c
>>> index 54a76ab..16c1f72 100644
>>> --- a/datapath/linux/compat/ip6_gre.c
>>> +++ b/datapath/linux/compat/ip6_gre.c
>>> @@ -1146,7 +1146,6 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct
>>> sk_buff *skb,
>>>  goto tx_err;
>>>
>>>  t->parms.o_flags &= ~TUNNEL_KEY;
>>> -   IPCB(skb)->flags = 0;
>>>
>>> The upstream linux kernel has the above code.
>> Do we need to fix the upstream kernel then backport?
>>
>
> That would be the normal work flow.
>
> Yifeng,
>
> Can you please post a patch with this fix to netdev?  Taking William's
> comments into account as well.
>
> Good catch and thanks for the fix!
>
> - Greg
>
>
>> Thanks,
>> William
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 3/3] ovs-sandbox: Generate the SSL keys using the default key length

[Timothy Redaelli]

This commit removes the explicit set of 1024-bit RSA keys when the RSA
keys are generated on "make sandbox" and so the default (2048-bit) is used.

Signed-off-by: Timothy Redaelli 
---
 tutorial/ovs-sandbox | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tutorial/ovs-sandbox b/tutorial/ovs-sandbox
index 62ec537e8..7a5ab5f75 100755
--- a/tutorial/ovs-sandbox
+++ b/tutorial/ovs-sandbox
@@ -390,11 +390,11 @@ if $ovn; then
 
 if [ "$HAVE_OPENSSL" = yes ]; then
 OVS_PKI="run ovs-pki --dir=$sandbox/pki --log=$sandbox/ovs-pki.log"
-$OVS_PKI -B 1024 init
-$OVS_PKI -B 1024 req+sign ovnsb switch
-$OVS_PKI -B 1024 req+sign ovnnb switch
+$OVS_PKI init
+$OVS_PKI req+sign ovnsb switch
+$OVS_PKI req+sign ovnnb switch
 for i in $(seq $n_controllers); do
-$OVS_PKI -B 1024 -u req+sign chassis-$i switch
+$OVS_PKI -u req+sign chassis-$i switch
 done
 fi
 fi
-- 
2.17.1

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

[ovs-dev] [PATCH 2/3] ovn-architecture: Use the default key length in examples

[Timothy Redaelli]

This commit removes the explicit set of 1024-bit RSA keys on
ovn-architecture examples and so the default (2048-bit) is used.

Signed-off-by: Timothy Redaelli 
---
 ovn/ovn-architecture.7.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ovn/ovn-architecture.7.xml b/ovn/ovn-architecture.7.xml
index ae5ca8e4a..6ed2cf132 100644
--- a/ovn/ovn-architecture.7.xml
+++ b/ovn/ovn-architecture.7.xml
@@ -1607,7 +1607,7 @@
   Creating SSL certificates for each chassis with the certificate CN field
   set to the chassis name (e.g. for a chassis with
   external-ids:system-id=chassis-1, via the command
-  "ovs-pki -B 1024 -u req+sign chassis-1 switch").
+  "ovs-pki -u req+sign chassis-1 switch").
 
 
   Configuring each ovn-controller to use SSL when connecting to the
-- 
2.17.1

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

[ovs-dev] [PATCH 1/3] tests: Use the default key length when generating RSA keys

[Timothy Redaelli]

This commit removes the explicit set of 1024-bit RSA keys when ovs-pki
is launched and so the default (2048-bit) is used.

Signed-off-by: Timothy Redaelli 
---
 tests/ovs-vsctl.at  | 4 ++--
 tests/ovsdb-rbac.at | 8 
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index f9e7f3bb1..6f37c0da7 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -1374,7 +1374,7 @@ AT_KEYWORDS([ovs-vsctl ssl])
 AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
 PKIDIR=`pwd`
 OVS_PKI="sh $abs_top_srcdir/utilities/ovs-pki.in --dir=$PKIDIR/pki 
--log=$PKIDIR/ovs-pki.log"
-AT_CHECK([$OVS_PKI -B 1024 init && $OVS_PKI -B 1024 req+sign vsctl switch && 
$OVS_PKI -B 1024 req ovsdbserver && $OVS_PKI self-sign ovsdbserver], [0], 
[ignore], [ignore])
+AT_CHECK([$OVS_PKI init && $OVS_PKI req+sign vsctl switch && $OVS_PKI req 
ovsdbserver && $OVS_PKI self-sign ovsdbserver], [0], [ignore], [ignore])
 
 dnl Create database.
 OVSDB_INIT([conf.db])
@@ -1401,7 +1401,7 @@ AT_KEYWORDS([ovs-vsctl ssl])
 AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
 PKIDIR=`pwd`
 OVS_PKI="sh $abs_top_srcdir/utilities/ovs-pki.in --dir=$PKIDIR/pki 
--log=$PKIDIR/ovs-pki.log"
-AT_CHECK([$OVS_PKI -B 1024 init && $OVS_PKI -B 1024 req+sign vsctl switch && 
$OVS_PKI -B 1024 req+sign ovsdbserver controller], [0], [ignore], [ignore])
+AT_CHECK([$OVS_PKI init && $OVS_PKI req+sign vsctl switch && $OVS_PKI req+sign 
ovsdbserver controller], [0], [ignore], [ignore])
 
 dnl Create database.
 OVSDB_INIT([conf.db])
diff --git a/tests/ovsdb-rbac.at b/tests/ovsdb-rbac.at
index adefee4df..7de3711fb 100644
--- a/tests/ovsdb-rbac.at
+++ b/tests/ovsdb-rbac.at
@@ -6,10 +6,10 @@ AT_SKIP_IF([test "$HAVE_OPENSSL" = no])
 
 RBAC_PKIDIR="$(pwd)"
 RBAC_PKI="sh $abs_top_srcdir/utilities/ovs-pki.in --dir=$RBAC_PKIDIR/pki 
--log=$RBAC_PKIDIR/rbac-pki.log"
-$RBAC_PKI -B 1024 init
-$RBAC_PKI -B 1024 req+sign ovsdb-server switch
-$RBAC_PKI -B 1024 -u req+sign client-1 switch
-$RBAC_PKI -B 1024 -u req+sign client-2 switch
+$RBAC_PKI init
+$RBAC_PKI req+sign ovsdb-server switch
+$RBAC_PKI -u req+sign client-1 switch
+$RBAC_PKI -u req+sign client-2 switch
 
 AT_DATA([schema],
   [[{"name": "mydb",
-- 
2.17.1

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

[ovs-dev] [PATCH 0/3] Use the default key length for RSA keys

[Timothy Redaelli]

Currently, 1024-bit RSA keys are generated for OVS tests, are suggested in
ovn-architecture manpage examples and are used to generate the RSA keys inside
the sandbox (make sandbox), but OpenSSL documentation suggests to use at least
2048-bit keys, since "fewer amount of bits is considered insecure or to be
insecure pretty soon" [1].

Moreover, it's not currently possible to use OVS with 1024-bit keys (and
some SSL-related tests fail for this reason) on Fedora 29 when the FUTURE
crypto policies are enabled [2]. FUTURE crypto policies will become the
DEFAULT soon on Fedora Rawhide.

[1] https://github.com/openssl/openssl/blob/master/doc/HOWTO/keys.txt
[2] https://fedoraproject.org/wiki/Changes/CryptoSettings

Timothy Redaelli (3):
  tests: Use the default key length when generating RSA keys
  ovn-architecture: Use the default key length in examples
  ovs-sandbox: Generate the SSL keys using the default key length

 ovn/ovn-architecture.7.xml | 2 +-
 tests/ovs-vsctl.at | 4 ++--
 tests/ovsdb-rbac.at| 8 
 tutorial/ovs-sandbox   | 8 
 4 files changed, 11 insertions(+), 11 deletions(-)

-- 
2.17.1

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

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

[Gregory Rose]

On 8/8/2018 6:50 PM, William Tu wrote:

thanks for the fix.

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


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


can you explain more about ovs_gso_cb?


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

diff --git a/datapath/linux/compat/ip6_gre.c b/datapath/linux/compat/ip6_
gre.c
index 54a76ab..16c1f72 100644
--- a/datapath/linux/compat/ip6_gre.c
+++ b/datapath/linux/compat/ip6_gre.c
@@ -1146,7 +1146,6 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct
sk_buff *skb,
 goto tx_err;

 t->parms.o_flags &= ~TUNNEL_KEY;
-   IPCB(skb)->flags = 0;


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


That would be the normal work flow.

Yifeng,

Can you please post a patch with this fix to netdev?  Taking William's 
comments into account as well.


Good catch and thanks for the fix!

- Greg



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


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

Re: [ovs-dev] [PATCH v7 00/13] Support multi-segment mbufs

[Lam, Tiago]

On 09/08/2018 12:44, Ilya Maximets wrote:
> On 09.08.2018 11:38, Lam, Tiago wrote:
>> Hi Ilya,
>>
>> On 09/08/2018 09:27, Ilya Maximets wrote:
>>> Hmm. I found that this series modifies only dpdk related components
>>> and doesn't pay any attention to others.
>>> dp_packet API was modified to respect the segmented packets, but
>>> there are many places, where code uses packet data directly using
>>> only dp_packet_data() pointer and dp_packet_size().
>>> For example, at least following functions will read the wrong memory
>>> and probably generate segfault:
>>>
>>
>> No, that's not correct. Actually, this has been explained back in June
>> [1], where Eelco raised the same concerns. There, I replied with:
>>
>> "This series takes the approach of not allocating new mbufs in
>> `DPBUF_DPDK` dp_packets. This is to avoid the case where a
>> dp_packet_put() call, for example, allocates two mbufs to create enough
>> space, and returns a pointer to the data which isn't contiguous in
>> memory (because it is spread across two mbufs). Most of the code in OvS
>> that uses the dp_packet API relies on the assumption that memory
>> returned is contiguous in memory."
>>
>> But please do refer to the link for a full explanation. In fact, if that
>> was the case most of the tests in system-traffic would fail, and that
>> would show a flaw in the design.
>>
>> Now, obviously this doesn't mean that in the odd case something may have
>> slip through the cracks, but all the tests that have been run so far
>> have given enough confidence.
>>
>> Tiago.
>>
>> [1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/348502.html
> 
> I don't understand what you're talking about.
> Just look at the following case:
> 

The broad sentence above kind of threw me off to think this was about
the allocation of mbufs where data becomes uncontiguous, but the example
below shows well your point. I've similar tests, but they are missing
the "big file" part. Thanks for the detailed example.

I'll work to improve on this.

Tiago.

> 1. Applying following patch, just to be sure:
> ---
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 0c42268..bc995bc 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -1433,6 +1433,11 @@ netdev_linux_tap_batch_send(struct netdev *netdev_,
>  ssize_t retval;
>  int error;
>  
> +if (packet->mbuf.nb_segs > 1) {
> +VLOG_ERR_RL(,
> +"Sending multiseg mbuf as a plain data. nb_segs: %d",
> +packet->mbuf.nb_segs);
> +}
>  do {
>  retval = write(netdev->tap_fd, dp_packet_data(packet), size);
>  error = retval < 0 ? errno : 0;
> ---
> 
> 2. Configure OVS:
> ./bin/ovs-vsctl set Open_vSwitch . other_config:dpdk-multi-seg-mbufs=true
> 
> 3. Create one bridge with datapath_type "netdev", add one vhostuserclient
>port "vhost1" like this:
> 
> # ovs-vsctl show
> Bridge "ovsdpdk_br0"
> Port "vhost1"
> Interface "vhost1"
> type: dpdkvhostuserclient
> options: {vhost-server-path="/vhost1.sock"}
> Port "ovsdpdk_br0"
> Interface "ovsdpdk_br0"
> type: internal
> 
> 4. Start VM.
> 
> 5. Set up on host:
> 
> # ovs-vsctl set interface vhost1 mtu_request=9000
> # ovs-vsctl set interface ovsdpdk_br0 mtu_request=9000
> # ifconfig ovsdpdk_br0 192.168.114.20 netmask 255.255.255.0
> 
> 6. Set up inside VM:
> 
> # ifconfig eth0 192.168.114.10 netmask 255.255.255.0 mtu 9000
> 
> 7. Try to scp large file from VM to Host:
> 
> # scp file root@192.168.114.20:./
> file 13% 2112KB 434.8KB/s - *stalled*
> 
> At the same time in tcpdump on the host side:
> 
> 14:17:51.162168 IP (tos 0x8, ttl 64, id 30567, offset 0, flags [DF], proto 
> TCP (6), length 9000)
> 192.168.114.10.43648 > 192.168.114.20.ssh: Flags [.], cksum 0xc831 
> (incorrect -> 0x1be9), seq 480:9428, ack 761, win 256, options [nop,nop,TS 
> val 216598 ecr 1685821578], length 8948
> 14:17:51.582171 IP (tos 0x8, ttl 64, id 30568, offset 0, flags [DF], proto 
> TCP (6), length 9000)
> 192.168.114.10.43648 > 192.168.114.20.ssh: Flags [.], cksum 0xc807 
> (incorrect -> 0x980f), seq 480:9428, ack 761, win 256, options [nop,nop,TS 
> val 216640 ecr 1685821578], length 8948
> 14:17:52.422175 IP (tos 0x8, ttl 64, id 30569, offset 0, flags [DF], proto 
> TCP (6), length 9000)
> 192.168.114.10.43648 > 192.168.114.20.ssh: Flags [.], cksum 0xc7b3 
> (incorrect -> 0x1b6b), seq 480:9428, ack 761, win 256, options [nop,nop,TS 
> val 216724 ecr 1685821578], length 8948
> 14:17:54.115128 IP (tos 0x8, ttl 64, id 30570, offset 0, flags [DF], proto 
> TCP (6), length 9000)
> 192.168.114.10.43648 > 192.168.114.20.ssh: Flags [.], cksum 0xc70b 
> (incorrect -> 0x9713), seq 480:9428, ack 761, win 256, options [nop,nop,TS 
> val 216892 ecr 1685821578], length 8948
> 
> And in OVS log:
> 
> 

Re: [ovs-dev] [PATCH] vswitch.xml: Update dpdk-init documentation.

[Aaron Conole]

Kevin Traynor  writes:

> dpdk-init is now a string. Add description of 'true' and 'try'.
>
> Fixes: 3e52fa5644cd ("dpdk: reflect status and version in the database")
> Cc: acon...@redhat.com
> Signed-off-by: Kevin Traynor 
> ---

Acked-by: Aaron Conole 

D'oh!  Thanks, Kevin.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH] vswitch.xml: Update dpdk-init documentation.

[Kevin Traynor]

dpdk-init is now a string. Add description of 'true' and 'try'.

Fixes: 3e52fa5644cd ("dpdk: reflect status and version in the database")
Cc: acon...@redhat.com
Signed-off-by: Kevin Traynor 
---
 vswitchd/vswitch.xml | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 6342949..0cd8520 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -219,9 +219,15 @@
 
   
+  type='{"type": "string"}'>
 
-  Set this value to true to enable runtime support for
-  DPDK ports. The vswitch must have compile-time support for DPDK as
-  well.
+  Set this value to true or try to enable
+  runtime support for DPDK ports. The vswitch must have compile-time
+  support for DPDK as well.
+
+
+  A value of true will cause the ovs-vswitchd process to
+  abort if DPDK cannot be initialized. A value of try
+  will allow the ovs-vswitchd process to continue running even if DPDK
+  cannot be initialized.
 
 
-- 
1.8.3.1

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

Re: [ovs-dev] [PATCH v7 11/13] dpdk-tests: Add uni-tests for multi-seg mbufs.

[Stokes, Ian]

> In order to create a minimal environment that allows the tests to get
> mbufs from an existing mempool, the following approach is taken:
> - EAL is initialised (by using the main dpdk_init()) and a (very) small
>   mempool is instantiated (mimicking the logic in dpdk_mp_create()).
>   This mempool instance is global and used by all the tests;
> - Packets are then allocated from the instantiated mempool, and tested
>   on, by running some operations on them and manipulating data.
> 
> The tests introduced focus on testing DPDK dp_packets (where
> source=DPBUF_DPDK), linked with a single or multiple mbufs, across several
> operations, such as:
> - dp_packet_put();
> - dp_packet_shift();
> - dp_packet_reserve();
> - dp_packet_push_uninit();
> - dp_packet_clear();
> - And as a consequence of some of these, dp_packet_put_uninit() and
>   dp_packet_resize__().
> 
> Finally, this has also been integrated with the new DPDK testsuite.
> Thus, when running `$sudo make check-dpdk` one will also be running these
> tests.

Hi Tiago,

A few compilation errors flagged by sparse below.

> 
> Signed-off-by: Tiago Lam 
> Acked-by: Eelco Chaudron 
> ---
>  tests/automake.mk  |  10 +-
>  tests/dpdk-packet-mbufs.at |   7 +
>  tests/system-dpdk-testsuite.at |   1 +
>  tests/test-dpdk-mbufs.c| 513
> +
>  4 files changed, 530 insertions(+), 1 deletion(-)  create mode 100644
> tests/dpdk-packet-mbufs.at  create mode 100644 tests/test-dpdk-mbufs.c
> 
> diff --git a/tests/automake.mk b/tests/automake.mk index 8224e5a..5fe98bd
> 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -134,7 +134,8 @@ SYSTEM_DPDK_TESTSUITE_AT = \
>   tests/system-common-macros.at \
>   tests/system-dpdk-macros.at \
>   tests/system-dpdk-testsuite.at \
> - tests/system-dpdk.at
> + tests/system-dpdk.at \
> + tests/dpdk-packet-mbufs.at
> 
>  check_SCRIPTS += tests/atlocal
> 
> @@ -391,6 +392,10 @@ tests_ovstest_SOURCES = \
>   tests/test-vconn.c \
>   tests/test-aa.c \
>   tests/test-stopwatch.c
> +if DPDK_NETDEV
> +tests_ovstest_SOURCES += \
> + tests/test-dpdk-mbufs.c
> +endif
> 
>  if !WIN32
>  tests_ovstest_SOURCES += \
> @@ -403,6 +408,9 @@ tests_ovstest_SOURCES += \  endif
> 
>  tests_ovstest_LDADD = lib/libopenvswitch.la ovn/lib/libovn.la
> +if DPDK_NETDEV
> +tests_ovstest_LDFLAGS = $(AM_LDFLAGS) $(DPDK_vswitchd_LDFLAGS) endif
> 
>  noinst_PROGRAMS += tests/test-strtok_r
>  tests_test_strtok_r_SOURCES = tests/test-strtok_r.c diff --git
> a/tests/dpdk-packet-mbufs.at b/tests/dpdk-packet-mbufs.at new file mode
> 100644 index 000..f28e4fc
> --- /dev/null
> +++ b/tests/dpdk-packet-mbufs.at
> @@ -0,0 +1,7 @@
> +AT_BANNER([OVS-DPDK dp_packet unit tests])
> +
> +AT_SETUP([OVS-DPDK dp_packet - mbufs allocation])
> +AT_KEYWORDS([dp_packet, multi-seg, mbufs]) AT_CHECK(ovstest
> +test-dpdk-packet, [], [ignore], [ignore])
> +
> +AT_CLEANUP
> diff --git a/tests/system-dpdk-testsuite.at b/tests/system-dpdk-
> testsuite.at index 382f09e..f5edf58 100644
> --- a/tests/system-dpdk-testsuite.at
> +++ b/tests/system-dpdk-testsuite.at
> @@ -23,3 +23,4 @@ m4_include([tests/system-common-macros.at])
>  m4_include([tests/system-dpdk-macros.at])
> 
>  m4_include([tests/system-dpdk.at])
> +m4_include([tests/dpdk-packet-mbufs.at])
> diff --git a/tests/test-dpdk-mbufs.c b/tests/test-dpdk-mbufs.c new file
> mode 100644 index 000..8168cae
> --- /dev/null
> +++ b/tests/test-dpdk-mbufs.c
> @@ -0,0 +1,513 @@
> +/*
> + * Copyright (c) 2018 Intel Corporation
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "dp-packet.h"
> +#include "ovstest.h"
> +#include "dpdk.h"
> +#include "smap.h"
> +
> +#define N_MBUFS 1024
> +#define MBUF_DATA_LEN 2048
> +
> +int num_tests = 0;

Sparse compilation error: symbol 'num_tests' was not declared. Should it be 
static?

> +
> +/* Global var to hold a mempool instance, "test-mp", used in all of the
> +tests
> + * below. This instance is instantiated in dpdk_setup_eal_with_mp(). */
> +static struct rte_mempool *mp;
> +
> +/* Test data used to fill the packets with data. Note that this isn't a
> +string
> + * that repsents a valid packet, by any means. The pattern is generated
> +in set_
> + * testing_pattern_str() and the sole purpose is to verify the data
> +remains 

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

[Stokes, Ian]

> In its current implementation dp_packet_shift() is also unaware of multi-
> seg mbufs (that holds data in memory non-contiguously) and assumes that
> data exists contiguously in memory, memmove'ing data to perform the shift.
> 
> To add support for multi-seg mbuds a new set of functions was introduced,
> dp_packet_mbuf_shift() and dp_packet_mbuf_write(). These functions are
> used by dp_packet_shift(), when handling multi-seg mbufs, to shift and
> write data within a chain of mbufs.
> 
> Signed-off-by: Tiago Lam 
> Acked-by: Eelco Chaudron 
> ---
>  lib/dp-packet.c | 97
> +
>  lib/dp-packet.h | 10 ++
>  2 files changed, 107 insertions(+)
> 
> diff --git a/lib/dp-packet.c b/lib/dp-packet.c index 2aaeaae..d6e19eb
> 100644
> --- a/lib/dp-packet.c
> +++ b/lib/dp-packet.c
> @@ -294,6 +294,97 @@ dp_packet_prealloc_headroom(struct dp_packet *b,
> size_t size)
>  }
>  }
> 
> +#ifdef DPDK_NETDEV
> +/* Write len data bytes in a mbuf at specified offset.
> + *
> + * 'mbuf', pointer to the destination mbuf where 'ofs' is, and the mbuf
> +where
> + * the data will first be written.
> + * 'ofs', the offset within the provided 'mbuf' where 'data' is to be
> written.
> + * 'len', the size of the to be written 'data'.
> + * 'data', pointer to the to be written bytes.
> + *
> + * XXX: This function is the counterpart of the `rte_pktmbuf_read()`
> +function
> + * available with DPDK, in the rte_mbuf.h */ void
> +dp_packet_mbuf_write(struct rte_mbuf *mbuf, int16_t ofs, uint32_t len,
> + const void *data)
> +{
> +char *dst_addr;
> +uint16_t data_len;
> +int len_copy;
> +while (mbuf) {
> +if (len == 0) {
> +break;
> +}
> +
> +dst_addr = rte_pktmbuf_mtod_offset(mbuf, char *, ofs);
> +data_len = MBUF_BUF_END(mbuf->buf_addr, mbuf->buf_len) -
> + dst_addr;
> +
> +len_copy = MIN(len, data_len);
> +/* We don't know if 'data' is the result of a rte_pktmbuf_read()
> call,
> + * in which case we may end up writing to the same region of
> memory we
> + * are reading from and overlapping. Hence the use of memmove()
> here */
> +memmove(dst_addr, data, len_copy);
> +
> +data = ((char *) data) + len_copy;
> +len -= len_copy;
> +ofs = 0;
> +
> +mbuf = mbuf->next;
> +}
> +}
> +
> +static void
> +dp_packet_mbuf_shift_(struct rte_mbuf *dbuf, int16_t dst_ofs,
> +  const struct rte_mbuf *sbuf, uint16_t src_ofs,
> +int len) {
> +char rd[len];

Above will break compilation with sparse.

lib/dp-packet.c:341:13: error: Variable length array is used.

Regards
Ian

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

Re: [ovs-dev] [PATCH v7 00/13] Support multi-segment mbufs

[Ian Stokes]

On 8/9/2018 12:44 PM, Ilya Maximets wrote:

On 09.08.2018 11:38, Lam, Tiago wrote:

Hi Ilya,

On 09/08/2018 09:27, Ilya Maximets wrote:

Hmm. I found that this series modifies only dpdk related components
and doesn't pay any attention to others.
dp_packet API was modified to respect the segmented packets, but
there are many places, where code uses packet data directly using
only dp_packet_data() pointer and dp_packet_size().
For example, at least following functions will read the wrong memory
and probably generate segfault:



No, that's not correct. Actually, this has been explained back in June
[1], where Eelco raised the same concerns. There, I replied with:

"This series takes the approach of not allocating new mbufs in
`DPBUF_DPDK` dp_packets. This is to avoid the case where a
dp_packet_put() call, for example, allocates two mbufs to create enough
space, and returns a pointer to the data which isn't contiguous in
memory (because it is spread across two mbufs). Most of the code in OvS
that uses the dp_packet API relies on the assumption that memory
returned is contiguous in memory."

But please do refer to the link for a full explanation. In fact, if that
was the case most of the tests in system-traffic would fail, and that
would show a flaw in the design.

Now, obviously this doesn't mean that in the odd case something may have
slip through the cracks, but all the tests that have been run so far
have given enough confidence.

Tiago.

[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/348502.html


I don't understand what you're talking about.
Just look at the following case:

1. Applying following patch, just to be sure:
---
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 0c42268..bc995bc 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1433,6 +1433,11 @@ netdev_linux_tap_batch_send(struct netdev *netdev_,
  ssize_t retval;
  int error;
  
+if (packet->mbuf.nb_segs > 1) {

+VLOG_ERR_RL(,
+"Sending multiseg mbuf as a plain data. nb_segs: %d",
+packet->mbuf.nb_segs);
+}
  do {
  retval = write(netdev->tap_fd, dp_packet_data(packet), size);
  error = retval < 0 ? errno : 0;
---

2. Configure OVS:
./bin/ovs-vsctl set Open_vSwitch . other_config:dpdk-multi-seg-mbufs=true

3. Create one bridge with datapath_type "netdev", add one vhostuserclient
port "vhost1" like this:

# ovs-vsctl show
 Bridge "ovsdpdk_br0"
 Port "vhost1"
 Interface "vhost1"
 type: dpdkvhostuserclient
 options: {vhost-server-path="/vhost1.sock"}
 Port "ovsdpdk_br0"
 Interface "ovsdpdk_br0"
 type: internal

4. Start VM.

5. Set up on host:

# ovs-vsctl set interface vhost1 mtu_request=9000
# ovs-vsctl set interface ovsdpdk_br0 mtu_request=9000
# ifconfig ovsdpdk_br0 192.168.114.20 netmask 255.255.255.0

6. Set up inside VM:

# ifconfig eth0 192.168.114.10 netmask 255.255.255.0 mtu 9000

7. Try to scp large file from VM to Host:

# scp file root@192.168.114.20:./
file 13% 2112KB 434.8KB/s - *stalled*

At the same time in tcpdump on the host side:

14:17:51.162168 IP (tos 0x8, ttl 64, id 30567, offset 0, flags [DF], proto TCP 
(6), length 9000)
 192.168.114.10.43648 > 192.168.114.20.ssh: Flags [.], cksum 0xc831 (incorrect 
-> 0x1be9), seq 480:9428, ack 761, win 256, options [nop,nop,TS val 216598 ecr 
1685821578], length 8948
14:17:51.582171 IP (tos 0x8, ttl 64, id 30568, offset 0, flags [DF], proto TCP 
(6), length 9000)
 192.168.114.10.43648 > 192.168.114.20.ssh: Flags [.], cksum 0xc807 (incorrect 
-> 0x980f), seq 480:9428, ack 761, win 256, options [nop,nop,TS val 216640 ecr 
1685821578], length 8948
14:17:52.422175 IP (tos 0x8, ttl 64, id 30569, offset 0, flags [DF], proto TCP 
(6), length 9000)
 192.168.114.10.43648 > 192.168.114.20.ssh: Flags [.], cksum 0xc7b3 (incorrect 
-> 0x1b6b), seq 480:9428, ack 761, win 256, options [nop,nop,TS val 216724 ecr 
1685821578], length 8948
14:17:54.115128 IP (tos 0x8, ttl 64, id 30570, offset 0, flags [DF], proto TCP 
(6), length 9000)
 192.168.114.10.43648 > 192.168.114.20.ssh: Flags [.], cksum 0xc70b (incorrect 
-> 0x9713), seq 480:9428, ack 761, win 256, options [nop,nop,TS val 216892 ecr 
1685821578], length 8948

And in OVS log:

2018-08-09T11:17:50Z|2|netdev_linux(pmd8)|ERR|Sending multiseg mbuf as a 
plain data. nb_segs: 4
2018-08-09T11:17:50Z|3|netdev_linux(pmd8)|ERR|Sending multiseg mbuf as a 
plain data. nb_segs: 5
2018-08-09T11:17:50Z|4|netdev_linux(pmd8)|ERR|Sending multiseg mbuf as a 
plain data. nb_segs: 4
2018-08-09T11:17:50Z|5|netdev_linux(pmd8)|ERR|Sending multiseg mbuf as a 
plain data. nb_segs: 5
2018-08-09T11:17:50Z|6|netdev_linux(pmd8)|ERR|Sending multiseg mbuf as a 
plain data. nb_segs: 5
2018-08-09T11:17:51Z|7|netdev_linux(pmd8)|ERR|Sending multiseg mbuf as a 

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

[Aaron Conole]

"Chandran, Sugesh"  writes:

> Hi Aron,
> Thank you for pointing out.
> I will send out the patch again with right branch convention to make the 
> robot happy.

No need.  We can ignore the bot in this case.  A committer can do the
right thing.  Just an FYI for future commits.

Thanks!

> Regards
> _Sugesh
>
>
>> -Original Message-
>> From: Aaron Conole [mailto:acon...@bytheb.org]
>> Sent: Thursday, August 9, 2018 2:15 PM
>> To: 0-day Robot 
>> Cc: Chandran, Sugesh ; d...@openvswitch.org
>> Subject: Re: [ovs-dev] [ovs-dev, Branch2.8] netdev-dpdk: Fix failure to 
>> configure
>> flow control at netdev-init.
>> 
>> Hi Sugesh (and robot),
>> 
>> 0-day Robot  writes:
>> 
>> > Bleep bloop.  Greetings Sugesh Chandran, I am a robot and I have tried out
>> your patch.
>> > Thanks for your contribution.
>> >
>> > I encountered some error that I wasn't expecting.  See the details below.
>> >
>> >
>> > git-am:
>> > Failed to merge in the changes.
>> > Patch failed at 0001 netdev-dpdk: Fix failure to configure flow control at
>> netdev-init.
>> > The copy of the patch that failed is found in:
>> >
>> > /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-app
>> > ly/patch When you have resolved this problem, run "git am --resolved".
>> > If you prefer to skip this patch, run "git am --skip" instead.
>> > To restore the original branch and stop patching, run "git am --abort".
>> 
>> This happened because Branch2.8 doesn't match any such branch in the
>> repository.  It isn't a problem with your patch, per-se.
>> 
>> In the future, if you use branch-x.y the bot will detect the branch and 
>> switch to
>> using that branch.  Then the checks will be more robust.
>> 
>> > Please check this out.  If you feel there has been an error, please
>> > email acon...@bytheb.org
>> >
>> > Thanks,
>> > 0-day Robot
>> > ___
>> > dev mailing list
>> > d...@openvswitch.org
>> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> ___
> 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] tests: Don't log to syslog during tests.

[Ilya Maximets]

The patch is good.

The following incremental needed to make it complete:

diff --git a/python/ovs/vlog.py b/python/ovs/vlog.py
index d7a84d8..7d416c3 100644
--- a/python/ovs/vlog.py
+++ b/python/ovs/vlog.py
@@ -305,9 +305,11 @@ class Vlog(object):
 return
 
 logger = logging.getLogger('syslog')
-# If there is no infrastructure to support python syslog, disable
-# the logger to avoid repeated errors.
-if not os.path.exists("/dev/log"):
+# Disable the logger if there is no infrastructure to support python
+# syslog (to avoid repeated errors) or if the "null" syslog method
+# requested by environment.
+if not os.path.exists("/dev/log") \
+   or os.environ.get('OVS_SYSLOG_METHOD') == "null":
 logger.disabled = True
 return


With above change, tests leaves the crystal clear syslog.

Beside that,
Acked-by: Ilya Maximets 

On 09.08.2018 02:04, Ben Pfaff wrote:
> Until now, "make check" generated a huge amount of output to syslog.  This
> commit suppresses it.
> 
> CC: Ilya Maximets 
> Signed-off-by: Ben Pfaff 
> ---
>  NEWS  |  2 ++
>  lib/automake.mk   |  2 ++
>  lib/syslog-null.c | 60 
> +++
>  lib/syslog-null.h | 22 
>  lib/vlog.c| 12 +--
>  lib/vlog.man  |  7 ++-
>  lib/vlog.xml  | 11 +-
>  tests/atlocal.in  |  4 
>  8 files changed, 116 insertions(+), 4 deletions(-)
>  create mode 100644 lib/syslog-null.c
>  create mode 100644 lib/syslog-null.h
> 
> diff --git a/NEWS b/NEWS
> index 7875f6673e34..ae21340e9046 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,5 +1,7 @@
>  Post-v2.10.0
>  -
> +   - The environment variable OVS_SYSLOG_METHOD, if set, is now used
> + as the default syslog method.
>  
>  
>  v2.10.0 - xx xxx 
> diff --git a/lib/automake.mk b/lib/automake.mk
> index fb43aa1413b2..63e9d72ac18a 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -280,6 +280,8 @@ lib_libopenvswitch_la_SOURCES = \
>   lib/syslog-direct.h \
>   lib/syslog-libc.c \
>   lib/syslog-libc.h \
> + lib/syslog-null.c \
> + lib/syslog-null.h \
>   lib/syslog-provider.h \
>   lib/table.c \
>   lib/table.h \
> diff --git a/lib/syslog-null.c b/lib/syslog-null.c
> new file mode 100644
> index ..9dbd13911c21
> --- /dev/null
> +++ b/lib/syslog-null.c
> @@ -0,0 +1,60 @@
> +/*
> + * Copyright (c) 2015, 2016 Nicira, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +#include "syslog-null.h"
> +
> +#include 
> +
> +#include "compiler.h"
> +#include "syslog-provider.h"
> +#include "util.h"
> +
> +static void syslog_null_open(struct syslogger *this, int facility);
> +static void syslog_null_log(struct syslogger *this, int pri, const char 
> *msg);
> +
> +static struct syslog_class syslog_null_class = {
> +syslog_null_open,
> +syslog_null_log,
> +};
> +
> +struct syslog_null {
> +struct syslogger parent;
> +};
> +
> +/* This function  creates object that delegate all logging to null's
> + * syslog implementation. */
> +struct syslogger *
> +syslog_null_create(void)
> +{
> +struct syslog_null *this = xmalloc(sizeof *this);
> +
> +this->parent.class = _null_class;
> +this->parent.prefix = "";
> +
> +return >parent;
> +}
> +
> +static void
> +syslog_null_open(struct syslogger *this OVS_UNUSED, int facility OVS_UNUSED)
> +{
> +/* Nothing to do. */
> +}
> +
> +static void
> +syslog_null_log(struct syslogger *this OVS_UNUSED, int pri OVS_UNUSED,
> +const char *msg OVS_UNUSED)
> +{
> +/* Nothing to do. */
> +}
> diff --git a/lib/syslog-null.h b/lib/syslog-null.h
> new file mode 100644
> index ..0f7731dc4dcc
> --- /dev/null
> +++ b/lib/syslog-null.h
> @@ -0,0 +1,22 @@
> +/*
> + * Copyright (c) 2015 Nicira, Inc.
> + *
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific 

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

[Ilya Maximets]

On 09.08.2018 02:05, Ben Pfaff wrote:
> On Wed, Aug 08, 2018 at 03:35:31PM +0300, Ilya Maximets wrote:
>> Each run of the testsuite produces millions lines in a system
>> log. This is completely unnecessary and makes it difficult to
>> use system logs on test / build servers.
>>
>> This series is aimed to disable most of the syslog messages.
>> There are still few logs that requires significant changes in
>> tests or code to disable. They will be removed separately if
>> needed.
> 
> What do you think of the following approach?
> https://patchwork.ozlabs.org/patch/955273/

That's cool. I forget about ability to change syslog method.
And we don't need to change any tests with this approach.
I'll reply with my comments to the patch.

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

[ovs-dev] Interop ITX 2018 Attendees List

[Maria Hopkins via dev]

Hi,

Good day to you,


This is an outstanding offer for the Interop ITX 2018.


I am writing to check if you would be interested in acquiring the list
of *37,393
*attendees participated in Interop ITX 2018 for your marketing and sales
initiatives.


This is an opportunity to acquire the list of attendees contact details for
a robust marketing campaign which will eventually help you convert the
compiled leads in to phenomenal sales deal and also you can per-invite them
to visit your booth.

You will receive the file for permanent usage where you can use this list
for multiple campaigns.

Please find below mentioned data fields for your review.

*Company Name, Company URL, Contact Name, Title, Phone number, Fax Number,
Email Address, Company Address, Industry type, SIC Code, and Social Media
Link.*


Please revert with your interest.

Regards,

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

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

[Chandran, Sugesh]

Hi Aron,
Thank you for pointing out.
I will send out the patch again with right branch convention to make the robot 
happy.


Regards
_Sugesh


> -Original Message-
> From: Aaron Conole [mailto:acon...@bytheb.org]
> Sent: Thursday, August 9, 2018 2:15 PM
> To: 0-day Robot 
> Cc: Chandran, Sugesh ; d...@openvswitch.org
> Subject: Re: [ovs-dev] [ovs-dev, Branch2.8] netdev-dpdk: Fix failure to 
> configure
> flow control at netdev-init.
> 
> Hi Sugesh (and robot),
> 
> 0-day Robot  writes:
> 
> > Bleep bloop.  Greetings Sugesh Chandran, I am a robot and I have tried out
> your patch.
> > Thanks for your contribution.
> >
> > I encountered some error that I wasn't expecting.  See the details below.
> >
> >
> > git-am:
> > Failed to merge in the changes.
> > Patch failed at 0001 netdev-dpdk: Fix failure to configure flow control at
> netdev-init.
> > The copy of the patch that failed is found in:
> >
> > /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-app
> > ly/patch When you have resolved this problem, run "git am --resolved".
> > If you prefer to skip this patch, run "git am --skip" instead.
> > To restore the original branch and stop patching, run "git am --abort".
> 
> This happened because Branch2.8 doesn't match any such branch in the
> repository.  It isn't a problem with your patch, per-se.
> 
> In the future, if you use branch-x.y the bot will detect the branch and 
> switch to
> using that branch.  Then the checks will be more robust.
> 
> > Please check this out.  If you feel there has been an error, please
> > email acon...@bytheb.org
> >
> > Thanks,
> > 0-day Robot
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Aaron Conole]

Hi Sugesh (and robot),

0-day Robot  writes:

> Bleep bloop.  Greetings Sugesh Chandran, I am a robot and I have tried out 
> your patch.
> Thanks for your contribution.
>
> I encountered some error that I wasn't expecting.  See the details below.
>
>
> git-am:
> Failed to merge in the changes.
> Patch failed at 0001 netdev-dpdk: Fix failure to configure flow control at 
> netdev-init.
> The copy of the patch that failed is found in:
>
> /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
> When you have resolved this problem, run "git am --resolved".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".

This happened because Branch2.8 doesn't match any such branch in the
repository.  It isn't a problem with your patch, per-se.

In the future, if you use branch-x.y the bot will detect the branch and
switch to using that branch.  Then the checks will be more robust.

> Please check this out.  If you feel there has been an error, please
> email acon...@bytheb.org
>
> Thanks,
> 0-day Robot
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH Branch2.7] netdev-dpdk: Fix failure to configure flow control at netdev-init.

[Sugesh Chandran]

This patch backports the commit from the latest OVS master to OVS-2.7.

Configuring flow control at ixgbe netdev-init is throwing error in port
start.
For eg: without this fix, user cannot configure flow control on ixgbe dpdk
port as below,

"
ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \
options:dpdk-devargs=:05:00.1 options:rx-flow-ctrl=true
"

Instead,  it must be configured as two different commands,

"
ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \
   options:dpdk-devargs=:05:00.1
ovs-vsctl set Interface dpdk0 options:rx-flow-ctrl=true
"

The DPDK ixgbe driver is now validating all the 'rte_eth_fc_conf' fields before
trying to configuring the dpdk ethdev. Hence OVS can no longer set the
'dont care' fields to just '0' as before. This commit make sure all the
'rte_eth_fc_conf' fields are populated with default values before the dev
init.
Also to avoid read error on unsupported ports, the flow control parameters
are now read only when user is trying to configure/update it.

Signed-off-by: Sugesh Chandran 
---
 lib/netdev-dpdk.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index d82c9e2ae..5d3867508 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -830,14 +830,6 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 
 mbp_priv = rte_mempool_get_priv(dev->dpdk_mp->mp);
 dev->buf_size = mbp_priv->mbuf_data_room_size - RTE_PKTMBUF_HEADROOM;
-
-/* Get the Flow control configuration for DPDK-ETH */
-diag = rte_eth_dev_flow_ctrl_get(dev->port_id, >fc_conf);
-if (diag) {
-VLOG_DBG("cannot get flow control parameters on port=%d, err=%d",
- dev->port_id, diag);
-}
-
 return 0;
 }
 
@@ -1306,6 +1298,11 @@ netdev_dpdk_set_config(struct netdev *netdev, const 
struct smap *args,
 
 fc_mode = fc_mode_set[tx_fc_en][rx_fc_en];
 if (dev->fc_conf.mode != fc_mode || autoneg != dev->fc_conf.autoneg) {
+err = rte_eth_dev_flow_ctrl_get(dev->port_id, >fc_conf);
+if (err) {
+VLOG_WARN("Cannot get flow control parameters on port "
+"%"PRIu16", err=%d", dev->port_id, err);
+}
 dev->fc_conf.mode = fc_mode;
 dev->fc_conf.autoneg = autoneg;
 dpdk_eth_flow_ctrl_setup(dev);
-- 
2.17.1

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

Re: [ovs-dev] [PATCH v7 00/13] Support multi-segment mbufs

[Ilya Maximets]

On 09.08.2018 11:38, Lam, Tiago wrote:
> Hi Ilya,
> 
> On 09/08/2018 09:27, Ilya Maximets wrote:
>> Hmm. I found that this series modifies only dpdk related components
>> and doesn't pay any attention to others.
>> dp_packet API was modified to respect the segmented packets, but
>> there are many places, where code uses packet data directly using
>> only dp_packet_data() pointer and dp_packet_size().
>> For example, at least following functions will read the wrong memory
>> and probably generate segfault:
>>
> 
> No, that's not correct. Actually, this has been explained back in June
> [1], where Eelco raised the same concerns. There, I replied with:
> 
> "This series takes the approach of not allocating new mbufs in
> `DPBUF_DPDK` dp_packets. This is to avoid the case where a
> dp_packet_put() call, for example, allocates two mbufs to create enough
> space, and returns a pointer to the data which isn't contiguous in
> memory (because it is spread across two mbufs). Most of the code in OvS
> that uses the dp_packet API relies on the assumption that memory
> returned is contiguous in memory."
> 
> But please do refer to the link for a full explanation. In fact, if that
> was the case most of the tests in system-traffic would fail, and that
> would show a flaw in the design.
> 
> Now, obviously this doesn't mean that in the odd case something may have
> slip through the cracks, but all the tests that have been run so far
> have given enough confidence.
> 
> Tiago.
> 
> [1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/348502.html

I don't understand what you're talking about.
Just look at the following case:

1. Applying following patch, just to be sure:
---
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 0c42268..bc995bc 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1433,6 +1433,11 @@ netdev_linux_tap_batch_send(struct netdev *netdev_,
 ssize_t retval;
 int error;
 
+if (packet->mbuf.nb_segs > 1) {
+VLOG_ERR_RL(,
+"Sending multiseg mbuf as a plain data. nb_segs: %d",
+packet->mbuf.nb_segs);
+}
 do {
 retval = write(netdev->tap_fd, dp_packet_data(packet), size);
 error = retval < 0 ? errno : 0;
---

2. Configure OVS:
./bin/ovs-vsctl set Open_vSwitch . other_config:dpdk-multi-seg-mbufs=true

3. Create one bridge with datapath_type "netdev", add one vhostuserclient
   port "vhost1" like this:

# ovs-vsctl show
Bridge "ovsdpdk_br0"
Port "vhost1"
Interface "vhost1"
type: dpdkvhostuserclient
options: {vhost-server-path="/vhost1.sock"}
Port "ovsdpdk_br0"
Interface "ovsdpdk_br0"
type: internal

4. Start VM.

5. Set up on host:

# ovs-vsctl set interface vhost1 mtu_request=9000
# ovs-vsctl set interface ovsdpdk_br0 mtu_request=9000
# ifconfig ovsdpdk_br0 192.168.114.20 netmask 255.255.255.0

6. Set up inside VM:

# ifconfig eth0 192.168.114.10 netmask 255.255.255.0 mtu 9000

7. Try to scp large file from VM to Host:

# scp file root@192.168.114.20:./
file 13% 2112KB 434.8KB/s - *stalled*

At the same time in tcpdump on the host side:

14:17:51.162168 IP (tos 0x8, ttl 64, id 30567, offset 0, flags [DF], proto TCP 
(6), length 9000)
192.168.114.10.43648 > 192.168.114.20.ssh: Flags [.], cksum 0xc831 
(incorrect -> 0x1be9), seq 480:9428, ack 761, win 256, options [nop,nop,TS val 
216598 ecr 1685821578], length 8948
14:17:51.582171 IP (tos 0x8, ttl 64, id 30568, offset 0, flags [DF], proto TCP 
(6), length 9000)
192.168.114.10.43648 > 192.168.114.20.ssh: Flags [.], cksum 0xc807 
(incorrect -> 0x980f), seq 480:9428, ack 761, win 256, options [nop,nop,TS val 
216640 ecr 1685821578], length 8948
14:17:52.422175 IP (tos 0x8, ttl 64, id 30569, offset 0, flags [DF], proto TCP 
(6), length 9000)
192.168.114.10.43648 > 192.168.114.20.ssh: Flags [.], cksum 0xc7b3 
(incorrect -> 0x1b6b), seq 480:9428, ack 761, win 256, options [nop,nop,TS val 
216724 ecr 1685821578], length 8948
14:17:54.115128 IP (tos 0x8, ttl 64, id 30570, offset 0, flags [DF], proto TCP 
(6), length 9000)
192.168.114.10.43648 > 192.168.114.20.ssh: Flags [.], cksum 0xc70b 
(incorrect -> 0x9713), seq 480:9428, ack 761, win 256, options [nop,nop,TS val 
216892 ecr 1685821578], length 8948

And in OVS log:

2018-08-09T11:17:50Z|2|netdev_linux(pmd8)|ERR|Sending multiseg mbuf as a 
plain data. nb_segs: 4
2018-08-09T11:17:50Z|3|netdev_linux(pmd8)|ERR|Sending multiseg mbuf as a 
plain data. nb_segs: 5
2018-08-09T11:17:50Z|4|netdev_linux(pmd8)|ERR|Sending multiseg mbuf as a 
plain data. nb_segs: 4
2018-08-09T11:17:50Z|5|netdev_linux(pmd8)|ERR|Sending multiseg mbuf as a 
plain data. nb_segs: 5
2018-08-09T11:17:50Z|6|netdev_linux(pmd8)|ERR|Sending multiseg mbuf as a 
plain data. nb_segs: 5
2018-08-09T11:17:51Z|7|netdev_linux(pmd8)|ERR|Sending multiseg mbuf 

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

[0-day Robot]

Bleep bloop.  Greetings Sugesh Chandran, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

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


git-am:
Failed to merge in the changes.
Patch failed at 0001 netdev-dpdk: Fix failure to configure flow control at 
netdev-init.
The copy of the patch that failed is found in:
   
/var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch
When you have resolved this problem, run "git am --resolved".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


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

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

[ovs-dev] [PATCH Branch2.8] netdev-dpdk: Fix failure to configure flow control at netdev-init.

[Sugesh Chandran]

This patch backports the commit from the latest OVS master to OVS-2.8.

Configuring flow control at ixgbe netdev-init is throwing error in port
start.
For eg: without this fix, user cannot configure flow control on ixgbe dpdk
port as below,

"
ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \
options:dpdk-devargs=:05:00.1 options:rx-flow-ctrl=true
"

Instead,  it must be configured as two different commands,

"
ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \
   options:dpdk-devargs=:05:00.1
ovs-vsctl set Interface dpdk0 options:rx-flow-ctrl=true
"

The DPDK ixgbe driver is now validating all the 'rte_eth_fc_conf' fields before
trying to configuring the dpdk ethdev. Hence OVS can no longer set the
'dont care' fields to just '0' as before. This commit make sure all the
'rte_eth_fc_conf' fields are populated with default values before the dev
init.
Also to avoid read error on unsupported ports, the flow control parameters
are now read only when user is trying to configure/update it.

Signed-off-by: Sugesh Chandran 
---
 lib/netdev-dpdk.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 9bf76ee32..f7c80e0dc 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -866,13 +866,6 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 mbp_priv = rte_mempool_get_priv(dev->dpdk_mp->mp);
 dev->buf_size = mbp_priv->mbuf_data_room_size - RTE_PKTMBUF_HEADROOM;
 
-/* Get the Flow control configuration for DPDK-ETH */
-diag = rte_eth_dev_flow_ctrl_get(dev->port_id, >fc_conf);
-if (diag) {
-VLOG_DBG("cannot get flow control parameters on port=%"PRIu8", err=%d",
- dev->port_id, diag);
-}
-
 return 0;
 }
 
@@ -1404,6 +1397,12 @@ netdev_dpdk_set_config(struct netdev *netdev, const 
struct smap *args,
 
 fc_mode = fc_mode_set[tx_fc_en][rx_fc_en];
 if (dev->fc_conf.mode != fc_mode || autoneg != dev->fc_conf.autoneg) {
+/* Get the Flow control configuration for DPDK-ETH */
+err = rte_eth_dev_flow_ctrl_get(dev->port_id, >fc_conf);
+if (err) {
+VLOG_WARN("Cannot get flow control parameters on port "
+"%"PRIu16", err=%d", dev->port_id, err);
+}
 dev->fc_conf.mode = fc_mode;
 dev->fc_conf.autoneg = autoneg;
 dpdk_eth_flow_ctrl_setup(dev);
-- 
2.17.1

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

Re: [ovs-dev] OVS-DPDK public meeting

[Kevin Traynor]

Next meeting: 21st August 16:00 UTC.

Minutes of meeting 8th August 2018.
Attendees: Eelco, Thomas, Matteo, Ian, Ophir, Johann, Yipeng, Tiago,
Simon, Kevin.

===
GENERAL
===
- OVS 2.10
-- No info on anything that would make 2.10 slip
-- No news is good news :-)

- Updates to OVS to use latest DPDK
-- DPDK 18.11 will be an LTS and will be required for hardware offload
-- Suggestion to move OVS master to use DPDK 18.08 interim step
-- Would give time to flush out any integration bugs using later DPDK
releases before DPDK 18.11 is available

- Branches for future work
-- Request from Ophir to have a branch for hardware offload as it will
require significant effort. Ian will talk with Ben about it where it
should be.
-- Suggestion for the future to have a new branch working with latest
DPDK (e.g. 19.02/19.05 etc)to catch and feedback any integration issues.


PERFORMANCE/FEATURES


- Outstanding bug fixes:
-- dpif-netdev: Avoid reordering of packets in a batch with same megaflow.
-- This has been reported in the wild and would need to go on branches.
Question about whether this should try to be included for 2.10 release
or not. But anyway patch is not merged in master yet.

- Tiago has posted an RFC of TSO

- Move OVS 2.7 to use 16.11.7
-- Any reason not to? No one had any


HARDWARE OFFLOAD

- Ophir gave us an update and it seems like there has been good progress
and collaboration between vendors.
- There would need to be some new functionality wrt adding several ports
at once but it wouldn't break backward compatibility.

On 07/25/2017 02:25 PM, Kevin Traynor wrote:
> Hi All,
> 
> The OVS-DPDK public meetings have moved to Wednesday's at the same time.
> Everyone is welcome, it's a chance to share status/plans etc.
> 
> It's scheduled for every 2 weeks starting 26th July. Meeting time is
> currently @ 4pm UTC.
> 
> You can connect through Bluejeans or through dial in:
> 
> https://bluejeans.com/139318596
> 
> US: +1.408.740.7256 
> UK: +44.203.608.5256 
> Germany: +49.32.221.091256 
> Ireland: +353.1.697.1256 
> Other numbers at https://www.bluejeans.com/numbers
> Meeting ID: 139318596
> 
> thanks,
> Kevin.
> 

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

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

[Timothy Redaelli]

On Wed, 8 Aug 2018 13:49:12 -0300
Flavio Leitner  wrote:

> Actually, we also have dependencies in the systemd service
> to the network service:
> 
> [Unit]
>   
> Description=Open vSwitch Forwarding Unit
> After=ovsdb-server.service network-pre.target
> systemd-udev-settle.service Before=network.target network.service
>   ^^^ 
> 
> Looks like that would be gone, right?

I checked and "network.service" is an auto-generated service
(systemd-sysv-generator - Unit generator for SysV init scripts)
from /etc/rc.d/init.d/network that brings Up and Down the interfaces
(if you don't use Network Manager).

The explicit dependency is needed in order to avoid having it to run
before openvswitch / ovsdb-server.
Moreover it's also harmless to have it when network.service doesn't
exists since, in this case, systemd will only ignore the dependency.

For example, NetworkManager.service has "Before=network.target
network.service" on Fedora Rawhide too, since Fedora Rawhide still have
the support for the legacy network scripts (/etc/rc.d/init.d/network, in
network-scripts package).

> fbl

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

Re: [ovs-dev] [ovs-dev, v2] netdev: Retry getting interfaces on inconsistent dumps from kernel

[0-day Robot]

Bleep bloop.  Greetings Daniel Alvarez, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

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


checkpatch:
ERROR: Too many signoffs; are you missing Co-authored-by lines?
ERROR: Co-authored-by/Signed-off-by corruption
Lines checked: 74, Warnings: 0, Errors: 2


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

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

[ovs-dev] [PATCH v2] netdev: Retry getting interfaces on inconsistent dumps from kernel

[Daniel Alvarez]

This patch in glibc [0] is fixing a bug where we may be getting
inconsistent dumps from the kernel when listing interfaces due to
a race condition.

This could happen if we try to retrieve them while interfaces are
being added/removed from the system at the same time.
For systems running against old glibc versions, this patch is retrying
the operation up to 3 times and then proceeding by logging a
warning.

Note that 3 times should be enough to not delay the operation much
and since it's unlikely that we hit the race condition 3 times in
a row. Still, if this happened, this patch is not changing the
current behavior.

[0] 
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=c1f86a33ca32e26a9d6e29fc961e5ecb5e2e5eb4

Signed-off-by: Daniel Alvarez 
Co-authored-by: Jiri Benc 
---
 lib/netdev.c | 16 
 1 file changed, 16 insertions(+)

Signed-off-by: Daniel Alvarez 
---
 lib/netdev.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/lib/netdev.c b/lib/netdev.c
index 82ffeb901..690d28409 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -2039,19 +2039,36 @@ netdev_get_addrs(const char dev[], struct in6_addr 
**paddr,
 struct in6_addr *addr_array, *mask_array;
 const struct ifaddrs *ifa;
 int cnt = 0, i = 0;
+int retries = 3;
 
 ovs_mutex_lock(_addr_list_lock);
 if (!if_addr_list) {
 int err;
 
+retry:
 err = getifaddrs(_addr_list);
 if (err) {
 ovs_mutex_unlock(_addr_list_lock);
 return -err;
 }
+retries--;
 }
 
 for (ifa = if_addr_list; ifa; ifa = ifa->ifa_next) {
+if (!ifa->ifa_name) {
+if (retries) {
+/* Older versions of glibc have a bug on race condition with
+ * address addition which may cause one of the returned
+ * ifa_name values to be NULL. In such case, we know that we've
+ * got an inconsistent dump. Retry but beware of an endless
+ * loop. */
+freeifaddrs(if_addr_list);
+goto retry;
+} else {
+VLOG_WARN("Proceeding with an inconsistent dump of "
+  "interfaces from the kernel. Some may be missing");
+}
+}
 if (ifa->ifa_addr && ifa->ifa_name && ifa->ifa_netmask) {
 int family;
 
-- 
2.15.2 (Apple Git-101.1)

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

Re: [ovs-dev] [PATCH v7 00/13] Support multi-segment mbufs

[Lam, Tiago]

Hi Ilya,

On 09/08/2018 09:27, Ilya Maximets wrote:
> Hmm. I found that this series modifies only dpdk related components
> and doesn't pay any attention to others.
> dp_packet API was modified to respect the segmented packets, but
> there are many places, where code uses packet data directly using
> only dp_packet_data() pointer and dp_packet_size().
> For example, at least following functions will read the wrong memory
> and probably generate segfault:
> 

No, that's not correct. Actually, this has been explained back in June
[1], where Eelco raised the same concerns. There, I replied with:

"This series takes the approach of not allocating new mbufs in
`DPBUF_DPDK` dp_packets. This is to avoid the case where a
dp_packet_put() call, for example, allocates two mbufs to create enough
space, and returns a pointer to the data which isn't contiguous in
memory (because it is spread across two mbufs). Most of the code in OvS
that uses the dp_packet API relies on the assumption that memory
returned is contiguous in memory."

But please do refer to the link for a full explanation. In fact, if that
was the case most of the tests in system-traffic would fail, and that
would show a flaw in the design.

Now, obviously this doesn't mean that in the odd case something may have
slip through the cracks, but all the tests that have been run so far
have given enough confidence.

Tiago.

[1] https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/348502.html

>   * All the netdev-{linux, dummy, bsd} send functions, because
> they are using code like this:
> 
> write(netdev->tap_fd, dp_packet_data(packet), dp_packet_size(packet));
> 
>   * Tunnel extracting functions like 'udp_extract_tnl_md', because
> they're trying to calculate packet checksums using raw calls to
> 'csum_continue'.
> 
>   * CONTROLLER_UPCALL will aslo be broken, because it uses direct xmemdup.
> 
>   * Possibly, much more functions.
> 
> If It's true, when I'm afraid that this patch set is far from being
> ready for merge.
> 
> On 25.07.2018 17:19, Tiago Lam wrote:
>> Overview
>> 
>> This patchset introduces support for multi-segment mbufs to OvS-DPDK.
>> Multi-segment mbufs are typically used when the size of an mbuf is
>> insufficient to contain the entirety of a packet's data. Instead, the
>> data is split across numerous mbufs, each carrying a portion, or
>> 'segment', of the packet data. Mbufs are chained via their 'next'
>> attribute (an mbuf pointer).
>>
>> The main motivation behind the support for multi-segment mbufs is to
>> later introduce TSO (use case i. below) / GRO in OvS-DPDK, which is
>> planned to be introduced after this series.
>>
>> Use Cases
>> =
>> i.  Handling oversized (guest-originated) frames, which are marked
>> for hardware accelration/offload (TSO, for example).
>>
>> Packets which originate from a non-DPDK source may be marked for
>> offload; as such, they may be larger than the permitted ingress
>> interface's MTU, and may be stored in an oversized dp-packet. In
>> order to transmit such packets over a DPDK port, their contents
>> must be copied to a DPDK mbuf (via dpdk_do_tx_copy). However, in
>> its current implementation, that function only copies data into
>> a single mbuf; if the space available in the mbuf is exhausted,
>> but not all packet data has been copied, then it is lost.
>> Similarly, when cloning a DPDK mbuf, it must be considered
>> whether that mbuf contains multiple segments. Both issues are
>> resolved within this patchset.
>>
>> ii. Handling jumbo frames.
>>
>> While OvS already supports jumbo frames, it does so by increasing
>> mbuf size, such that the entirety of a jumbo frame may be handled
>> in a single mbuf. This is certainly the preferred, and most
>> performant approach (and remains the default).
>>
>> Enabling multi-segment mbufs
>> 
>> Multi-segment and single-segment mbufs are mutually exclusive, and the
>> user must decide on which approach to adopt on init. The introduction
>> of a new OVSDB field, 'dpdk-multi-seg-mbufs', facilitates this.
>>
>> This is a global boolean value, which determines how jumbo frames are
>> represented across all DPDK ports. In the absence of a user-supplied
>> value, 'dpdk-multi-seg-mbufs' defaults to false, i.e. multi-segment
>> mbufs must be explicitly enabled / single-segment mbufs remain the
>> default.
>>
>> Setting the field is identical to setting existing DPDK-specific OVSDB
>> fields:
>>
>> ovs-vsctl set Open_vSwitch . other_config:dpdk-init=true
>> ovs-vsctl set Open_vSwitch . other_config:dpdk-lcore-mask=0x10
>> ovs-vsctl set Open_vSwitch . other_config:dpdk-socket-mem=4096,0
>> ==> ovs-vsctl set Open_vSwitch . other_config:dpdk-multi-seg-mbufs=true
>>
>> Performance notes (based on v8)
>> =
>> In order to test for regressions in performance, tests were run on top
>> of master 88125d6 and v8 of this 

Re: [ovs-dev] [PATCH v7 00/13] Support multi-segment mbufs

[Ilya Maximets]

Hmm. I found that this series modifies only dpdk related components
and doesn't pay any attention to others.
dp_packet API was modified to respect the segmented packets, but
there are many places, where code uses packet data directly using
only dp_packet_data() pointer and dp_packet_size().
For example, at least following functions will read the wrong memory
and probably generate segfault:

  * All the netdev-{linux, dummy, bsd} send functions, because
they are using code like this:

write(netdev->tap_fd, dp_packet_data(packet), dp_packet_size(packet));

  * Tunnel extracting functions like 'udp_extract_tnl_md', because
they're trying to calculate packet checksums using raw calls to
'csum_continue'.

  * CONTROLLER_UPCALL will aslo be broken, because it uses direct xmemdup.

  * Possibly, much more functions.

If It's true, when I'm afraid that this patch set is far from being
ready for merge.

On 25.07.2018 17:19, Tiago Lam wrote:
> Overview
> 
> This patchset introduces support for multi-segment mbufs to OvS-DPDK.
> Multi-segment mbufs are typically used when the size of an mbuf is
> insufficient to contain the entirety of a packet's data. Instead, the
> data is split across numerous mbufs, each carrying a portion, or
> 'segment', of the packet data. Mbufs are chained via their 'next'
> attribute (an mbuf pointer).
> 
> The main motivation behind the support for multi-segment mbufs is to
> later introduce TSO (use case i. below) / GRO in OvS-DPDK, which is
> planned to be introduced after this series.
> 
> Use Cases
> =
> i.  Handling oversized (guest-originated) frames, which are marked
> for hardware accelration/offload (TSO, for example).
> 
> Packets which originate from a non-DPDK source may be marked for
> offload; as such, they may be larger than the permitted ingress
> interface's MTU, and may be stored in an oversized dp-packet. In
> order to transmit such packets over a DPDK port, their contents
> must be copied to a DPDK mbuf (via dpdk_do_tx_copy). However, in
> its current implementation, that function only copies data into
> a single mbuf; if the space available in the mbuf is exhausted,
> but not all packet data has been copied, then it is lost.
> Similarly, when cloning a DPDK mbuf, it must be considered
> whether that mbuf contains multiple segments. Both issues are
> resolved within this patchset.
> 
> ii. Handling jumbo frames.
> 
> While OvS already supports jumbo frames, it does so by increasing
> mbuf size, such that the entirety of a jumbo frame may be handled
> in a single mbuf. This is certainly the preferred, and most
> performant approach (and remains the default).
> 
> Enabling multi-segment mbufs
> 
> Multi-segment and single-segment mbufs are mutually exclusive, and the
> user must decide on which approach to adopt on init. The introduction
> of a new OVSDB field, 'dpdk-multi-seg-mbufs', facilitates this.
> 
> This is a global boolean value, which determines how jumbo frames are
> represented across all DPDK ports. In the absence of a user-supplied
> value, 'dpdk-multi-seg-mbufs' defaults to false, i.e. multi-segment
> mbufs must be explicitly enabled / single-segment mbufs remain the
> default.
> 
> Setting the field is identical to setting existing DPDK-specific OVSDB
> fields:
> 
> ovs-vsctl set Open_vSwitch . other_config:dpdk-init=true
> ovs-vsctl set Open_vSwitch . other_config:dpdk-lcore-mask=0x10
> ovs-vsctl set Open_vSwitch . other_config:dpdk-socket-mem=4096,0
> ==> ovs-vsctl set Open_vSwitch . other_config:dpdk-multi-seg-mbufs=true
> 
> Performance notes (based on v8)
> =
> In order to test for regressions in performance, tests were run on top
> of master 88125d6 and v8 of this patchset, both with the multi-segment
> mbufs option enabled and disabled.
> 
> VSperf was used to run the phy2phy_cont and pvp_cont tests with varying
> packet sizes of 64B, 1500B and 7000B, on a 10Gbps interface.
> 
> Test | Size | Master | Multi-seg disabled | Multi-seg enabled
> -
> p2p  |  64  | ~22.7  |  ~22.65|   ~18.3
> p2p  | 1500 |  ~1.6  |~1.6|~1.6
> p2p  | 7000 | ~0.36  |   ~0.36|   ~0.36
> pvp  |  64  |  ~6.7  |~6.7|~6.3
> pvp  | 1500 |  ~1.6  |~1.6|~1.6
> pvp  | 7000 | ~0.36  |   ~0.36|   ~0.36
> 
> Packet size is in bytes, while all packet rates are reported in mpps
> (aggregated).
> 
> No noticeable regression has been observed (certainly everything is
> within the ± 5% margin of existing performance), aside from the 64B
> packet size case when multi-segment mbuf is enabled. This is
> expected, however, because of how Tx vectoriszed functions are
> incompatible with multi-segment mbufs on some PMDs. The PMD under
> use during these tests was the 

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

[Chandran, Sugesh]

Regards
_Sugesh


> -Original Message-
> From: Stokes, Ian
> Sent: Wednesday, August 8, 2018 2:08 PM
> To: Chandran, Sugesh ; ovs-
> d...@openvswitch.org
> Subject: RE: [PATCHv3] netdev-dpdk: Fix failure to configure flow control at
> netdev-init.
> 
> > Configuring flow control at ixgbe netdev-init is throwing error in
> > port start.
> >
> > For eg: without this fix, user cannot configure flow control on ixgbe
> > dpdk port as below,
> >
> > "
> > ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \
> > options:dpdk-devargs=:05:00.1 options:rx-flow-ctrl=true "
> >
> > Instead,  it must be configured as two different commands,
> >
> > "
> > ovs-vsctl add-port br0 dpdk0 -- set Interface dpdk0 type=dpdk \
> >options:dpdk-devargs=:05:00.1
> > ovs-vsctl set Interface dpdk0 options:rx-flow-ctrl=true "
> >
> > The DPDK ixgbe driver is now validating all the 'rte_eth_fc_conf'
> > fields before trying to configuring the dpdk ethdev. Hence OVS can no
> > longer set the 'dont care' fields to just '0' as before. This commit
> > make sure all the 'rte_eth_fc_conf' fields are populated with default
> > values before the dev init.
> > Also to avoid read error on unsupported ports, the flow control
> > parameters are now read only when user is trying to configure/update it.
> >
> > Signed-off-by: Sugesh Chandran 
> > ---
> > V1 -> V2
> > Read DPDK port flow-control parameters only when reconfiguration is
> > required.
> > This will avoid flow control read error on unsupported ports.
> >
> > V2 -> V3
> >  Minor modifications to incorporate review comments from Ian.
> >  Merged to latest master.
> >
> > ---
> 
> Thanks for the work on this Sugesh, I'll add this to the pull request this 
> week. I've
> backported it to 2.20, 2.9 also. However the flow control logic changed
> between then and 2.8/2.7. Can you check this approach against those branches
> if a backported is required there?
[Sugesh] Thank you Ian for your help. I will send out patches for 2.8 and 2.7 
releases.

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

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