[ovs-dev] [PATCH v4] netdev-dpdk: leverage the mempool offload support

[Hemant Agrawal]

DPDK 16.07 introduced the support for mempool offload support.
rte_pktmbuf_pool_create is the recommended method for creating pktmbuf
pools. Buffer pools created with rte_mempool_create may not get offloaded
to the underlying offloaded mempools.

This patch, changes the rte_mempool_create to use helper wrapper
"rte_pktmbuf_pool_create" provided by dpdk, so that it can leverage
offloaded mempools.

Signed-off-by: Hemant Agrawal 
Acked-by: Jianbo Liu 
Acked-by: Kevin Traynor 
---
v4:
  * fix the comment as suggested
v3:
  * adding OVS_UNUSED for mp parameter
v2:
  * removing rte_pktmbuf_init as per review comment

 lib/netdev-dpdk.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ddc651b..9fa60fd 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -451,22 +451,19 @@ free_dpdk_buf(struct dp_packet *p)
 }
 
 static void
-ovs_rte_pktmbuf_init(struct rte_mempool *mp,
+ovs_rte_pktmbuf_init(struct rte_mempool *mp OVS_UNUSED,
  void *opaque_arg OVS_UNUSED,
  void *_p,
  unsigned i OVS_UNUSED)
 {
 struct rte_mbuf *pkt = _p;
 
-rte_pktmbuf_init(mp, opaque_arg, _p, i);
-
 dp_packet_init_dpdk((struct dp_packet *) pkt, pkt->buf_len);
 }
 
 static struct dpdk_mp *
 dpdk_mp_create(int socket_id, int mtu)
 {
-struct rte_pktmbuf_pool_private mbp_priv;
 struct dpdk_mp *dmp;
 unsigned mp_size;
 char *mp_name;
@@ -478,9 +475,6 @@ dpdk_mp_create(int socket_id, int mtu)
 dmp->socket_id = socket_id;
 dmp->mtu = mtu;
 dmp->refcount = 1;
-mbp_priv.mbuf_data_room_size = MBUF_SIZE(mtu) - sizeof(struct dp_packet);
-mbp_priv.mbuf_priv_size = sizeof(struct dp_packet)
-  - sizeof(struct rte_mbuf);
 /* XXX: this is a really rough method of provisioning memory.
  * It's impossible to determine what the exact memory requirements are
  * when the number of ports and rxqs that utilize a particular mempool can
@@ -496,18 +490,24 @@ dpdk_mp_create(int socket_id, int mtu)
 mp_name = xasprintf("ovs_mp_%d_%d_%u", dmp->mtu, dmp->socket_id,
 mp_size);
 
-dmp->mp = rte_mempool_create(mp_name, mp_size, MBUF_SIZE(mtu),
- MP_CACHE_SZ,
- sizeof(struct rte_pktmbuf_pool_private),
- rte_pktmbuf_pool_init, _priv,
- ovs_rte_pktmbuf_init, NULL,
- socket_id, 0);
+dmp->mp = rte_pktmbuf_pool_create(mp_name, mp_size,
+  MP_CACHE_SZ,
+  sizeof (struct dp_packet)
+ - sizeof (struct rte_mbuf),
+  MBUF_SIZE(mtu)
+ - sizeof(struct dp_packet),
+  socket_id);
 if (dmp->mp) {
 VLOG_DBG("Allocated \"%s\" mempool with %u mbufs",
  mp_name, mp_size);
 }
 free(mp_name);
 if (dmp->mp) {
+/* rte_pktmbuf_pool_create has done some initialization of the
+ * rte_mbuf part of each dp_packet, while ovs_rte_pktmbuf_init
+ * initializes some OVS specific fields of dp_packet.
+ */
+rte_mempool_obj_iter(dmp->mp, ovs_rte_pktmbuf_init, NULL);
 return dmp;
 }
 } while (rte_errno == ENOMEM && (mp_size /= 2) >= MIN_NB_MBUF);
-- 
1.9.1

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

Re: [ovs-dev] OVS performance with Kernel Datapath of Linux upstream vs Linux OVS tree.

[Kapil Adhikesavalu]

Hi Jarno,

That's great! Thanks for the clarification. So, if anything it should only
improve the performance when I move to OVS tree kernel module.

On Thu, 13 Apr 2017, 1:05 AM Jarno Rajahalme,  wrote:

>
> > On Apr 12, 2017, at 12:49 AM, Kapil Adhikesavalu 
> wrote:
> >
> > Hi,
> >
> > Is there any performance difference with using the OVS kernel Datapath
> > available part of Linux upstream Vs the module built from Linux OVS tree.
> >
>
> OVS tree kernel module has an Exact Match Cache, which generally improves
> performance. Upstream linux openvswitch module does not have it.
>
> > So far i have been using the DP part of the Linux upstream and as NAT
> > feature requires Linux version 4.6, i plan to switch to DP module built
> > from OVS tree.
> >
> > 1. In general is there any performance between using these two ? or would
> > it vary based on the features being in use.
> > 2. I am currently using VXLAN and L2 forwarding + NAT(plan to use it),
> > would like to know if there could be any performance difference expected
> > when i switch from upstream DP to KLM.
> >
> > I didn't any specific mention about performance in FAQ -
> > http://docs.openvswitch.org/en/latest/faq/releases/ expect for this
> > statement 'Certain features require kernel support to function or to have
> > reasonable performance.’
> >
>
> This note relates to using OVS with an older (upstream) kernel module.
> While we maintain backwards compatibility, newer features perform better
> with newer kernel module having explicit support for the feature.
>
>   Jarno
>
> > Regards
> > Kapil.
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
> --

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

Re: [ovs-dev] [PATCH 1/3] dpif-netlink: Fix memory leak on port del on WIN32.

[Alin Serdean]

Thanks!

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Joe Stringer
> Sent: Thursday, April 13, 2017 12:00 AM
> To: ovs dev 
> Cc: Eric Garver 
> Subject: Re: [ovs-dev] [PATCH 1/3] dpif-netlink: Fix memory leak on port del
> on WIN32.
> 
> On 12 April 2017 at 13:19, Joe Stringer  wrote:
> > From: Eric Garver 
> >
> > Fixes: da467899ab6e ("Windows: Add internal switch port per OVS bridge")
> > Signed-off-by: Eric Garver 
> > Acked-by: Alin Gabriel Serdean 
> > Signed-off-by: Joe Stringer 
> > ---
> 
> Applied to master and branch-2.7.
> ___
> 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] Minutes: OvS Offload Discussion at Netdev 2.1

[Simon Horman]

On Wed, Apr 12, 2017 at 03:01:29PM -0300, Flavio Leitner wrote:
> On Sat, Apr 08, 2017 at 04:47:57PM -0400, Simon Horman wrote:
> > At Netdev 2.1 a meeting was held to discuss OvS offload.  Minutes of the
> > discussion follow. I apologise in advance for any errors or omissions;
> > doubly for any errors in the attendee list.
> > 
> > Topic: OVS Hardware Offload Using TC
> > Date: 7th April 2017
> > Location: Netdev 2.1, Montreal
> > Attendees: Aaron Conole, Ben LaHaise, Eran Ben Elisha, Hannes Frederic Sowa,
> >   Jakub Kicinski, Jiri Pirko, Joe Stringer, John Fastabend, Nick Viljoen,
> >   Rashid Khan, Rony Efraim, Simon Horman
> > 
> > Joe raised 2 concerns:
> > 
> > 1) How to enable users to understand whether offload is
> >successful and if not, why not?
> > 
> >   a) There is functionality in the v7[1] patchset to report which flows
> >  are present in hardware.
> > 
> >   b) New error reporting infrastructure from the kernel is forthcoming It
> >  should allow TC to provide more error information if a flow can't be
> >  added to hardware. This could be made available to users - e.g. logged
> >  - to allow them better understand the reason for the failure.
> > 
> > 2) Maintenance burden falling on existing maintainers
> > 
> >   a) Simon offered to take some of the maintenance burden
> >  immediately as he is already a committer.
> > 
> >   b) The aim is to ensure that in future there are other committers
> >  who are interested in this feature.
> > 
> >   There was consensus that if the feature-set does not grow there should be
> >   discussion of deprecating the HW offload support provided by [1].
> > 
> > Joe raised issue of whether OVS should probe hardware capabilities at 
> > runtime.
> > John suggested this may be complex; potential combinatorial set is too 
> > large.
> > 
> > Rony then raised the increased complexity of using multiple NICs of
> > different types with different offload capabilities, this was tabled to a
> > later date.
> > 
> > Joe has expressed a desire for more testing. There was a general agreement
> > to contribute tests.
> > 
> > [1] [PATCH ovs V7 00/24] Introducing HW offload support for openvswitch
> 
> Thanks for the minutes.  I wasn't there so this helps to understand
> what has been discussed.
> 
> Have you discussed how we are going to tag/document this feature?  For
> instance, are we going to say this is "experimental"?

My recollection is that important detail was not discussed at the meeting.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] tests: Add test for CT action with setting labels.

[Jarno Rajahalme]

> On Apr 12, 2017, at 2:06 PM, Joe Stringer  wrote:
> 
> On 21 March 2017 at 15:51, Jarno Rajahalme  wrote:
>> This test clearly demonstrates the bit order of labels in the OpenFlow
>> wire format.
>> 
>> Signed-off-by: Jarno Rajahalme 
> 
> Acked-by: Joe Stringer 

Thanks for the review! Pushed to master,

  Jarno

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

Re: [ovs-dev] [PATCH 2/3] netdev-dpdk: Fix device leak on port deletion.

[Darrell Ball]

On 4/3/17, 8:04 AM, "ovs-dev-boun...@openvswitch.org on behalf of Ilya 
Maximets"  
wrote:

Currently, once created device in dpdk will exist forever
even after del-port operation untill we manually call
'ovs-appctl netdev-dpdk/detach ', where  is not
the port's name but the name of dpdk eth device or pci address.

Few issues with current implementation:

1. Different API for usual (system) and DPDK devices.
   (We have to call 'ovs-appctl netdev-dpdk/detach' each
time after 'del-port' to actually free the device)
   This is a big issue mostly for virtual DPDK devices.

2. Follows from 1:
   For DPDK devices 'del-port' leads just to
   'rte_eth_dev_stop' and subsequent 'add-port' will
   just start the already existing device. Such behaviour
   will not reset the device to initial state as it could
   be expected. For example: virtual pcap pmd will continue
   reading input file instead of reading it from the beginning.

3. Follows from 2:
   After execution of the following commands 'port1' will be
   configured with the 'old-options' while 'ovs-vsctl show'
   will show us 'new-options' in dpdk-devargs field:

 ovs-vsctl add-port port1 -- set interface port1 type=dpdk \
   options:dpdk-devargs=,
 ovs-vsctl del-port port1
 ovs-vsctl add-port port1 -- set interface port1 type=dpdk \
   options:dpdk-devargs=,

4. Follows from 1:
   Not detached device consumes 'port_id'. Since we have very
   limited number of 'port_id's (32 in common case) this may
   lead to quick exhausting of id pool and inability to add any
   other port.

To avoid above issues we need to detach all the attached devices on
port destruction.
appctl 'netdev-dpdk/detach' removed because not needed anymore.

CC: Ciara Loftus 
Fixes: 55e075e65ef9 ("netdev-dpdk: Arbitrary 'dpdk' port naming")
Fixes: 69876ed78611 ("netdev-dpdk: Add support for virtual DPDK PMDs 
(vdevs)")
Signed-off-by: Ilya Maximets 
---
 Documentation/howto/dpdk.rst |  5 ++--
 lib/netdev-dpdk.c| 66 
+++-
 2 files changed, 18 insertions(+), 53 deletions(-)

diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index dc63f7d..20d8975 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -342,10 +342,9 @@ Then it can be attached to OVS::
 $ ovs-vsctl add-port br0 dpdkx -- set Interface dpdkx type=dpdk \
 options:dpdk-devargs=:01:00.0
 
-It is also possible to detach a port from ovs, the user has to remove the
-port using the del-port command, then it can be detached using::
+Detaching will be performed while processing del-port command::
 
-$ ovs-appctl netdev-dpdk/detach :01:00.0
+$ ovs-vsctl del-port dpdkx
 
 This feature is not supported with VFIO and does not work with some NICs.
 For more information please refer to the `DPDK Port Hotplug Framework
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index c8d7108..658a454 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -359,6 +359,9 @@ struct netdev_dpdk {
 /* Device arguments for dpdk ports */
 char *devargs;
 
+/* If true, device was attached by rte_eth_dev_attach(). */
+bool attached;
+

OVS should not try to keep (replicate) internal state of the rte;
to check “attached state” query the rte for valid port.


 /* In dpdk_list. */
 struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
 
@@ -851,6 +854,7 @@ common_construct(struct netdev *netdev, unsigned int 
port_no,
 dev->max_packet_len = MTU_TO_FRAME_LEN(dev->mtu);
 ovsrcu_index_init(>vid, -1);
 dev->vhost_reconfigured = false;
+dev->attached = false;
 
 ovsrcu_init(>qos_conf, NULL);
 
@@ -996,10 +1000,21 @@ static void
 netdev_dpdk_destruct(struct netdev *netdev)
 {
 struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
+char devname[RTE_ETH_NAME_MAX_LEN];
 
 ovs_mutex_lock(_mutex);
 
 rte_eth_dev_stop(dev->port_id);
+
+if (dev->attached) {

Same comment above


+rte_eth_dev_close(dev->port_id);
+if (rte_eth_dev_detach(dev->port_id, devname) < 0) {
+VLOG_ERR("Device '%s' can not be detached", dev->devargs);
+} else {
+VLOG_INFO("Device '%s' detached", devname);
+}
+}
+
 free(dev->devargs);
 common_destruct(dev);
 

Re: [ovs-dev] [PATCH] tests: Add test for CT action with setting labels.

[Joe Stringer]

On 21 March 2017 at 15:51, Jarno Rajahalme  wrote:
> This test clearly demonstrates the bit order of labels in the OpenFlow
> wire format.
>
> Signed-off-by: Jarno Rajahalme 

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

Re: [ovs-dev] [PATCH] compat: vxlan: Fix NULL dereference in dst_cache.

[Joe Stringer]

On 6 April 2017 at 06:51, Joe Stringer  wrote:
> Neelakantam reports:
>
> BUG: unable to handle kernel NULL pointer dereference
> RIP: 0010:[]  [] 
> dst_cache_get_ip4+0xc/0x50 [openvswitch]
> Call Trace:
>  [] vxlan_get_route.isra.41+0xea/0x130 [openvswitch]
>  [] ? __skb_get_hash+0x39/0x160
>  [] ovs_vxlan_fill_metadata_dst+0x170/0x1e0 [openvswitch]
>  [] ovs_dev_fill_metadata_dst+0x9e/0xd0 [openvswitch]
>  [] output_userspace+0xfe/0x180 [openvswitch]
>  [] do_execute_actions+0x63d/0x8f0 [openvswitch]
>  [] ovs_execute_actions+0x41/0x130 [openvswitch]
>  [] ovs_dp_process_packet+0x94/0x140 [openvswitch]
>  [] ovs_vport_receive+0x73/0xd0 [openvswitch]
>  [] ? enqueue_task_fair+0x402/0x6c0
>  [] ? sched_clock_cpu+0x85/0xc0
>  [] ? check_preempt_curr+0x75/0xa0
>  [] ? ttwu_do_wakeup+0x19/0xd0
>  [] ? ttwu_do_activate.constprop.84+0x5d/0x70
>  [] ? try_to_wake_up+0x1b6/0x300
>  [] ? __wake_up+0x44/0x50
>  [] internal_dev_xmit+0x24/0x60 [openvswitch]
>  [] dev_hard_start_xmit+0x171/0x3b0
>  [] sch_direct_xmit+0x104/0x200
>  [] dev_queue_xmit+0x236/0x570
>  [] macvlan_start_xmit+0x3c/0xc0 [macvlan]
>  [] dev_hard_start_xmit+0x171/0x3b0
>  [] sch_direct_xmit+0x104/0x200
>  [] dev_queue_xmit+0x236/0x570
>  [] macvtap_get_user+0x414/0x720 [macvtap]
>  [] macvtap_sendmsg+0x2b/0x30 [macvtap]
>  [] handle_tx+0x2fc/0x550 [vhost_net]
>  [] handle_tx_kick+0x15/0x20 [vhost_net]
>  [] vhost_worker+0xfb/0x1e0 [vhost]
>  [] ? vhost_dev_reset_owner+0x50/0x50 [vhost]
>  [] kthread+0xcf/0xe0
>  [] ? kthread_create_on_node+0x140/0x140
>  [] ret_from_fork+0x58/0x90
>  [] ? kthread_create_on_node+0x140/0x140
>
> ovs_vxlan_fill_metadata_dst() calls vxlan_get_route() with no dst_cache,
> handle this case and don't attempt to use dst_cache.
>
> Reported-by: Neelakantam Gaddam 
> Signed-off-by: Joe Stringer 
> ---

Neelakantam reported separately that it fixes the issue, so I applied
this to master, branch-2.7 and branch-2.6.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 2/3] dpif-netlink: Handle netlink errors on port del.

[Joe Stringer]

On 12 April 2017 at 13:19, Joe Stringer  wrote:
> From: Eric Garver 
>
> The return code of dpif_netlink_port_query__() was not being checked.
>
> Fixes: da467899ab6e ("Windows: Add internal switch port per OVS bridge")
> Signed-off-by: Eric Garver 
> Acked-by: Alin Gabriel Serdean 
> Signed-off-by: Joe Stringer 

Applied to master and branch-2.7.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/3] dpif-netlink: Fix memory leak on port del on WIN32.

[Joe Stringer]

On 12 April 2017 at 13:19, Joe Stringer  wrote:
> From: Eric Garver 
>
> Fixes: da467899ab6e ("Windows: Add internal switch port per OVS bridge")
> Signed-off-by: Eric Garver 
> Acked-by: Alin Gabriel Serdean 
> Signed-off-by: Joe Stringer 
> ---

Applied to master and branch-2.7.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpif-netlink: fix memory leak in dpif_netlink_port_del__() on WIN32

[Eric Garver]

On Wed, Apr 12, 2017 at 01:21:30PM -0700, Joe Stringer wrote:
> On 12 April 2017 at 12:43, Eric Garver  wrote:
> > On Wed, Apr 12, 2017 at 04:12:51PM +, Alin Serdean wrote:
> >> Thanks a lot for fixing the leak Eric!
> >>
> >> We should apply on 2.7 as well, but needs a rebase.
> >>
> >> Is it okay if we add the function call on 2.7 as well or should we limit 
> >> it too WIN32?
> >
> > I don't think it would hurt anything. If others disagree I can submit a
> > 2.7 only patch.
> >
> > Joe,
> > Do you have an opinion?
> 
> Hi Eric,
> 
> I think that if you split this patch up and simplified it then it
> would make it easier to review, apply, and backport. Really the
> refactor belongs with the rtnetlink series.

You're right.

> 
> Since I was already looking at this, I went ahead and did it:
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/330743.html
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/330741.html
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/330742.html

Great.

> 
> I'll apply the first two to master and branch-2.7 shortly. Feel free
> to take the third patch into your rtnetlink series when you resubmit.

Sounds like a plan. Thanks.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH] dpif-netlink: fix memory leak in dpif_netlink_port_del__() on WIN32

[Joe Stringer]

On 12 April 2017 at 12:43, Eric Garver  wrote:
> On Wed, Apr 12, 2017 at 04:12:51PM +, Alin Serdean wrote:
>> Thanks a lot for fixing the leak Eric!
>>
>> We should apply on 2.7 as well, but needs a rebase.
>>
>> Is it okay if we add the function call on 2.7 as well or should we limit it 
>> too WIN32?
>
> I don't think it would hurt anything. If others disagree I can submit a
> 2.7 only patch.
>
> Joe,
> Do you have an opinion?

Hi Eric,

I think that if you split this patch up and simplified it then it
would make it easier to review, apply, and backport. Really the
refactor belongs with the rtnetlink series.

Since I was already looking at this, I went ahead and did it:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/330743.html
https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/330741.html
https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/330742.html

I'll apply the first two to master and branch-2.7 shortly. Feel free
to take the third patch into your rtnetlink series when you resubmit.

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

[ovs-dev] [PATCH 1/3] dpif-netlink: Fix memory leak on port del on WIN32.

[Joe Stringer]

From: Eric Garver 

Fixes: da467899ab6e ("Windows: Add internal switch port per OVS bridge")
Signed-off-by: Eric Garver 
Acked-by: Alin Gabriel Serdean 
Signed-off-by: Joe Stringer 
---
 lib/dpif-netlink.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index bad575c9cf65..e0acf5cce3cf 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -963,6 +963,7 @@ dpif_netlink_port_del__(struct dpif_netlink *dpif, 
odp_port_t port_no)
  temp_dpif_port.name);
 };
 }
+dpif_port_destroy(_dpif_port);
 #endif
 error = dpif_netlink_vport_transact(, NULL, NULL);
 
-- 
2.11.1

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

[ovs-dev] [PATCH 3/3] dpif-netlink: Refactor dpif_netlink_port_del__().

[Joe Stringer]

From: Eric Garver 

Signed-off-by: Eric Garver 
Acked-by: Alin Gabriel Serdean 
Signed-off-by: Joe Stringer 
---
 lib/dpif-netlink.c | 27 ++-
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index e27524754a05..860df6013827 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -60,9 +60,6 @@ VLOG_DEFINE_THIS_MODULE(dpif_netlink);
 #ifdef _WIN32
 #include "wmi.h"
 enum { WINDOWS = 1 };
-static int dpif_netlink_port_query__(const struct dpif_netlink *dpif,
- odp_port_t port_no, const char *port_name,
- struct dpif_port *dpif_port);
 #else
 enum { WINDOWS = 0 };
 #endif
@@ -224,6 +221,9 @@ static void dpif_netlink_vport_to_ofpbuf(const struct 
dpif_netlink_vport *,
  struct ofpbuf *);
 static int dpif_netlink_vport_from_ofpbuf(struct dpif_netlink_vport *,
   const struct ofpbuf *);
+static int dpif_netlink_port_query__(const struct dpif_netlink *dpif,
+ odp_port_t port_no, const char *port_name,
+ struct dpif_port *dpif_port);
 
 static struct dpif_netlink *
 dpif_netlink_cast(const struct dpif *dpif)
@@ -948,31 +948,32 @@ dpif_netlink_port_del__(struct dpif_netlink *dpif, 
odp_port_t port_no)
 OVS_REQ_WRLOCK(dpif->upcall_lock)
 {
 struct dpif_netlink_vport vport;
+struct dpif_port dpif_port;
 int error;
 
+error = dpif_netlink_port_query__(dpif, port_no, NULL, _port);
+if (error) {
+return error;
+}
+
 dpif_netlink_vport_init();
 vport.cmd = OVS_VPORT_CMD_DEL;
 vport.dp_ifindex = dpif->dp_ifindex;
 vport.port_no = port_no;
 #ifdef _WIN32
-struct dpif_port temp_dpif_port;
-
-error = dpif_netlink_port_query__(dpif, port_no, NULL, _dpif_port);
-if (error) {
-return error;
-}
-if (!strcmp(temp_dpif_port.type, "internal")) {
-if (!delete_wmi_port(temp_dpif_port.name)){
+if (!strcmp(dpif_port.type, "internal")) {
+if (!delete_wmi_port(dpif_port.name)){
 VLOG_ERR("Could not delete wmi port with name: %s",
- temp_dpif_port.name);
+ dpif_port.name);
 };
 }
-dpif_port_destroy(_dpif_port);
 #endif
 error = dpif_netlink_vport_transact(, NULL, NULL);
 
 vport_del_channels(dpif, port_no);
 
+dpif_port_destroy(_port);
+
 return error;
 }
 
-- 
2.11.1

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

[ovs-dev] [PATCH 2/3] dpif-netlink: Handle netlink errors on port del.

[Joe Stringer]

From: Eric Garver 

The return code of dpif_netlink_port_query__() was not being checked.

Fixes: da467899ab6e ("Windows: Add internal switch port per OVS bridge")
Signed-off-by: Eric Garver 
Acked-by: Alin Gabriel Serdean 
Signed-off-by: Joe Stringer 
---
 lib/dpif-netlink.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index e0acf5cce3cf..e27524754a05 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -956,7 +956,11 @@ dpif_netlink_port_del__(struct dpif_netlink *dpif, 
odp_port_t port_no)
 vport.port_no = port_no;
 #ifdef _WIN32
 struct dpif_port temp_dpif_port;
-dpif_netlink_port_query__(dpif, port_no, NULL, _dpif_port);
+
+error = dpif_netlink_port_query__(dpif, port_no, NULL, _dpif_port);
+if (error) {
+return error;
+}
 if (!strcmp(temp_dpif_port.type, "internal")) {
 if (!delete_wmi_port(temp_dpif_port.name)){
 VLOG_ERR("Could not delete wmi port with name: %s",
-- 
2.11.1

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

Re: [ovs-dev] [PATCH] dpif-netlink: fix memory leak in dpif_netlink_port_del__() on WIN32

[Eric Garver]

On Wed, Apr 12, 2017 at 04:12:51PM +, Alin Serdean wrote:
> Thanks a lot for fixing the leak Eric!
> 
> We should apply on 2.7 as well, but needs a rebase.
> 
> Is it okay if we add the function call on 2.7 as well or should we limit it 
> too WIN32?

I don't think it would hurt anything. If others disagree I can submit a
2.7 only patch.

Joe,
Do you have an opinion?

> Acked-by: Alin Gabriel Serdean 
> 
> > -Original Message-
> > From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> > boun...@openvswitch.org] On Behalf Of Eric Garver
> > Sent: Tuesday, April 11, 2017 4:15 PM
> > To: ovs dev 
> > Subject: Re: [ovs-dev] [PATCH] dpif-netlink: fix memory leak in
> > dpif_netlink_port_del__() on WIN32
> > 
> > Do any of the windows folks have any feediback on the below patch?
> > 
> > Thanks.
> > Eric.
> > 
> > On Tue, Apr 04, 2017 at 03:31:29PM -0700, Joe Stringer wrote:
> > > On 4 April 2017 at 13:31, Eric Garver  wrote:
> > > > Furthermore, the return code of dpif_netlink_port_query__() was not
> > > > being checked.
> > > >
> > > > Signed-off-by: Eric Garver 
> > > > ---
> > >
> > > For context, yes this adds the call to the Linux path as well but this
> > > is already in the proposed rtnetlink patch series so I'm not concerned
> > > about this.
> > >
> > > Here's my Ack. I'd like some windows folk to be aware and take a look
> > > too though:
> > > Acked-by: Joe Stringer 
> ___
> 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 performance with Kernel Datapath of Linux upstream vs Linux OVS tree.

[Jarno Rajahalme]

> On Apr 12, 2017, at 12:49 AM, Kapil Adhikesavalu  wrote:
> 
> Hi,
> 
> Is there any performance difference with using the OVS kernel Datapath
> available part of Linux upstream Vs the module built from Linux OVS tree.
> 

OVS tree kernel module has an Exact Match Cache, which generally improves 
performance. Upstream linux openvswitch module does not have it.

> So far i have been using the DP part of the Linux upstream and as NAT
> feature requires Linux version 4.6, i plan to switch to DP module built
> from OVS tree.
> 
> 1. In general is there any performance between using these two ? or would
> it vary based on the features being in use.
> 2. I am currently using VXLAN and L2 forwarding + NAT(plan to use it),
> would like to know if there could be any performance difference expected
> when i switch from upstream DP to KLM.
> 
> I didn't any specific mention about performance in FAQ -
> http://docs.openvswitch.org/en/latest/faq/releases/ expect for this
> statement 'Certain features require kernel support to function or to have
> reasonable performance.’
> 

This note relates to using OVS with an older (upstream) kernel module. While we 
maintain backwards compatibility, newer features perform better with newer 
kernel module having explicit support for the feature.

  Jarno

> Regards
> Kapil.
> ___
> 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 ovs V7 00/24] Introducing HW offload support for openvswitch

[Flavio Leitner]

On Wed, Apr 12, 2017 at 02:53:13PM -0300, Marcelo Ricardo Leitner wrote:
> Hi,
> 
> On Wed, Apr 12, 2017 at 01:13:36PM -0300, Flavio Leitner wrote:
> > 
> > Hi Marcelo,
> > 
> > Could you please confirm if this patch series fixes the aggregation
> > issue in your environment?
> 
> Yes it did. I can see IPv4 L3 matches in datapath now and their stats
> are updated accordingly.

Great, thanks Marcelo!
fbl

> > On Fri, Apr 07, 2017 at 04:12:47PM +0300, Roi Dayan wrote:
> > > This patch series introduces rule offload functionality to dpif-netlink
> > > via netdev ports new flow offloading API. The user can specify whether to
> > > enable rule offloading or not via OVS configuration. Netdev providers
> > > are able to implement netdev flow offload API in order to offload rules.
[...]

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

Re: [ovs-dev] [PATCH ovs V7 21/24] dpctl: Add an option to dump only certain kinds of flows

[Joe Stringer]

On 12 April 2017 at 10:53, Flavio Leitner  wrote:
> On Wed, Apr 12, 2017 at 10:04:50AM -0700, Joe Stringer wrote:
>> On 12 April 2017 at 08:22, Flavio Leitner  wrote:
>> > On Fri, Apr 07, 2017 at 04:13:08PM +0300, Roi Dayan wrote:
>> >> From: Paul Blakey 
>> >>
>> >> Usage:
>> >> # to dump all datapath flows (default):
>> >> ovs-dpctl dump-flows
>> >>
>> >> # to dump only flows that in kernel datapath:
>> >> ovs-dpctl dump-flows type=ovs
>> >>
>> >> # to dump only flows that are offloaded:
>> >> ovs-dpctl dump-flows type=offloaded
>> >
>> >
>> > It should return an error if the type= is wrong/unavailable.
>>
>> I wonder if we should also add a piece in verbose mode (-m) at the end
>> of each flow that says something like "type=software", "type=hardware"
>> (which should match the name of the ovs-dpctl dump-flows type
>> argument)?
>
> Makes sense. Perhaps offloaded isn't the correct term, neither
> software or hardware, because it seems TC can decide to not offload to
> the HW and that is not visible to OVS.  As a result, you could pass
> "type=offloaded" and see rules that are actually in TC software.
> Perhaps I missed something.

wrt 'offloaded', seems fine to me. If it's offloaded from regular OVS
path to TC path, even in software, this is still more accurately
describing what happened than to say 'hardware' since the tc-policy
could be sw-only. So 'ovs' and 'offloaded' seem OK.

The main point I was highlighting above was that if you just dumped
the flows in regular mode, you don't have any indication where the
flows are being executed. Maybe that's fine, but if ovs-dpctl
dump-flows ends up printing both sources it might be easier to notice
something odd going on if there were an indication where the flow came
from. The tradeoff is we end up printing even more information on the
commandline, hence why I figured it might be better to only display it
in verbose mode.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] Minutes: OvS Offload Discussion at Netdev 2.1

[Flavio Leitner]

On Sat, Apr 08, 2017 at 04:47:57PM -0400, Simon Horman wrote:
> At Netdev 2.1 a meeting was held to discuss OvS offload.  Minutes of the
> discussion follow. I apologise in advance for any errors or omissions;
> doubly for any errors in the attendee list.
> 
> Topic: OVS Hardware Offload Using TC
> Date: 7th April 2017
> Location: Netdev 2.1, Montreal
> Attendees: Aaron Conole, Ben LaHaise, Eran Ben Elisha, Hannes Frederic Sowa,
>   Jakub Kicinski, Jiri Pirko, Joe Stringer, John Fastabend, Nick Viljoen,
>   Rashid Khan, Rony Efraim, Simon Horman
> 
> Joe raised 2 concerns:
> 
> 1) How to enable users to understand whether offload is
>successful and if not, why not?
> 
>   a) There is functionality in the v7[1] patchset to report which flows
>  are present in hardware.
> 
>   b) New error reporting infrastructure from the kernel is forthcoming It
>  should allow TC to provide more error information if a flow can't be
>  added to hardware. This could be made available to users - e.g. logged
>  - to allow them better understand the reason for the failure.
> 
> 2) Maintenance burden falling on existing maintainers
> 
>   a) Simon offered to take some of the maintenance burden
>  immediately as he is already a committer.
> 
>   b) The aim is to ensure that in future there are other committers
>  who are interested in this feature.
> 
>   There was consensus that if the feature-set does not grow there should be
>   discussion of deprecating the HW offload support provided by [1].
> 
> Joe raised issue of whether OVS should probe hardware capabilities at runtime.
> John suggested this may be complex; potential combinatorial set is too large.
> 
> Rony then raised the increased complexity of using multiple NICs of
> different types with different offload capabilities, this was tabled to a
> later date.
> 
> Joe has expressed a desire for more testing. There was a general agreement
> to contribute tests.
> 
> [1] [PATCH ovs V7 00/24] Introducing HW offload support for openvswitch

Thanks for the minutes.  I wasn't there so this helps to understand
what has been discussed.

Have you discussed how we are going to tag/document this feature?  For
instance, are we going to say this is "experimental"?

Thanks,
-- 
Flavio

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

Re: [ovs-dev] [PATCH ovs V7 21/24] dpctl: Add an option to dump only certain kinds of flows

[Flavio Leitner]

On Wed, Apr 12, 2017 at 10:04:50AM -0700, Joe Stringer wrote:
> On 12 April 2017 at 08:22, Flavio Leitner  wrote:
> > On Fri, Apr 07, 2017 at 04:13:08PM +0300, Roi Dayan wrote:
> >> From: Paul Blakey 
> >>
> >> Usage:
> >> # to dump all datapath flows (default):
> >> ovs-dpctl dump-flows
> >>
> >> # to dump only flows that in kernel datapath:
> >> ovs-dpctl dump-flows type=ovs
> >>
> >> # to dump only flows that are offloaded:
> >> ovs-dpctl dump-flows type=offloaded
> >
> >
> > It should return an error if the type= is wrong/unavailable.
> 
> I wonder if we should also add a piece in verbose mode (-m) at the end
> of each flow that says something like "type=software", "type=hardware"
> (which should match the name of the ovs-dpctl dump-flows type
> argument)?

Makes sense. Perhaps offloaded isn't the correct term, neither
software or hardware, because it seems TC can decide to not offload to
the HW and that is not visible to OVS.  As a result, you could pass
"type=offloaded" and see rules that are actually in TC software.
Perhaps I missed something.

-- 
Flavio

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

Re: [ovs-dev] [PATCH ovs V7 13/24] netdev-tc-offloads: Implement netdev flow put using tc interface

[Flavio Leitner]

On Fri, Apr 07, 2017 at 04:13:00PM +0300, Roi Dayan wrote:
> From: Paul Blakey 
> 
> Currently only tunnel offload is supported.
> 
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> Reviewed-by: Simon Horman 
> ---
>  lib/dpif-netlink.c   |   4 +-
>  lib/netdev-tc-offloads.c | 390 
> ++-
>  2 files changed, 386 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 4cce810..83b6c51 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -1941,6 +1941,8 @@ parse_flow_put(struct dpif_netlink *dpif, struct 
> dpif_flow_put *put)
>  return EOPNOTSUPP;
>  }
>  
> +memset(, 0, sizeof match);
> +
>  err = parse_key_and_mask_to_match(put->key, put->key_len, put->mask,
>put->mask_len, );
>  if (err) {
> @@ -2012,7 +2014,7 @@ parse_flow_put(struct dpif_netlink *dpif, struct 
> dpif_flow_put *put)
>  
>  VLOG_DBG("added flow");
>  } else if (err != EEXIST) {
> -VLOG_ERR_RL(, "failed adding flow: %s", ovs_strerror(err));
> +VLOG_ERR_RL(, "failed to offload flow: %s", ovs_strerror(err));
>  }
>  
>  out:
> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> index 4d1bea6..4ef29d1 100644
> --- a/lib/netdev-tc-offloads.c
> +++ b/lib/netdev-tc-offloads.c
> @@ -473,16 +473,392 @@ netdev_tc_flow_dump_next(struct netdev_flow_dump *dump,
>  return false;
>  }
>  
> +static int
> +parse_put_flow_set_action(struct tc_flower *flower, const struct nlattr *set,
> +  size_t set_len)
> +{
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
> +const struct nlattr *set_attr;
> +size_t set_left;
> +
> +NL_ATTR_FOR_EACH_UNSAFE(set_attr, set_left, set, set_len) {
> +if (nl_attr_type(set_attr) == OVS_KEY_ATTR_TUNNEL) {
> +const struct nlattr *tunnel = nl_attr_get(set_attr);
> +const size_t tunnel_len = nl_attr_get_size(set_attr);
> +const struct nlattr *tun_attr;
> +size_t tun_left;
> +
> +flower->set.set = true;
> +NL_ATTR_FOR_EACH_UNSAFE(tun_attr, tun_left, tunnel, tunnel_len) {
> +switch (nl_attr_type(tun_attr)) {
> +case OVS_TUNNEL_KEY_ATTR_ID: {
> +flower->set.id = nl_attr_get_be64(tun_attr);
> +}
> +break;
> +case OVS_TUNNEL_KEY_ATTR_IPV4_SRC: {
> +flower->set.ipv4.ipv4_src = nl_attr_get_be32(tun_attr);
> +}
> +break;
> +case OVS_TUNNEL_KEY_ATTR_IPV4_DST: {
> +flower->set.ipv4.ipv4_dst = nl_attr_get_be32(tun_attr);
> +}
> +break;
> +case OVS_TUNNEL_KEY_ATTR_IPV6_SRC: {
> +flower->set.ipv6.ipv6_src =
> +nl_attr_get_in6_addr(tun_attr);
> +}
> +break;
> +case OVS_TUNNEL_KEY_ATTR_IPV6_DST: {
> +flower->set.ipv6.ipv6_dst =
> +nl_attr_get_in6_addr(tun_attr);
> +}
> +break;
> +case OVS_TUNNEL_KEY_ATTR_TP_SRC: {
> +flower->set.tp_src = nl_attr_get_be16(tun_attr);
> +}
> +break;
> +case OVS_TUNNEL_KEY_ATTR_TP_DST: {
> +flower->set.tp_dst = nl_attr_get_be16(tun_attr);
> +}
> +break;
> +}
> +}
> +} else {
> +VLOG_DBG_RL(, "unsupported set action type: %d",
> +nl_attr_type(set_attr));
> +return EOPNOTSUPP;
> +}
> +}
> +return 0;
> +}
> +
> +static int
> +test_key_and_mask(struct match *match) {
> +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20);
> +const struct flow *key = >flow;
> +struct flow *mask = >wc.masks;
> +
> +if (mask->pkt_mark) {
> +VLOG_DBG_RL(, "offloading attribute pkt_mark isn't supported");
> +return EOPNOTSUPP;
> +}
> +
> +if (mask->recirc_id && key->recirc_id != 0) {
> +VLOG_DBG_RL(, "offloading attribute recirc_id isn't supported");
> +return EOPNOTSUPP;
> +}
> +mask->recirc_id = 0;
> +
> +if (mask->dp_hash) {
> +VLOG_DBG_RL(, "offloading attribute dp_hash isn't supported");
> +return EOPNOTSUPP;
> +}
> +
> +if (mask->conj_id) {
> +VLOG_DBG_RL(, "offloading attribute conj_id isn't supported");
> +return EOPNOTSUPP;
> +}
> +
> +if (mask->skb_priority) {
> +VLOG_DBG_RL(, "offloading attribute skb_priority isn't 
> supported");
> +return EOPNOTSUPP;
> +}
> +
> +if (mask->actset_output) {
> +VLOG_DBG_RL(,

Re: [ovs-dev] [RFC][PATCH] netdev-dpdk: add support for TSO

[Aaron Conole]

Hi Mark,

Mark Kavanagh  writes:

> TCP Segmentation Offload (TSO) is a feature which enables
> the TCP/IP network stack to delegate segmentation of a TCP
> segment to the NIC, thus saving compute resources.
>
> This commit adds support for TSO in the DPDK vHost-User backend,
> to OvS v2.6.1; this enables a guest to offload segmentation of
> TCP segments that it sends to OvS.
>
> This patch is not intended for upstreaming, but rather was produced
> in response to requests for an updated version of the initial TSO RFC
> patch posted here:
> https://mail.openvswitch.org/pipermail/ovs-dev/2016-June/316414.html
>
> Signed-off-by: Mark Kavanagh 
> ---

...

> diff --git a/lib/packets.c b/lib/packets.c
> index e4c29d5..2417ba2 100644
> --- a/lib/packets.c
> +++ b/lib/packets.c
> @@ -33,6 +33,10 @@
>  #include "dp-packet.h"
>  #include "unaligned.h"
>  
> +#ifdef DPDK_NETDEV
> +#include "rte_ether.h"
> +#endif
> +
>  const struct in6_addr in6addr_exact = IN6ADDR_EXACT_INIT;
>  const struct in6_addr in6addr_all_hosts = IN6ADDR_ALL_HOSTS_INIT;
>  
> @@ -204,6 +208,11 @@ eth_push_vlan(struct dp_packet *packet, ovs_be16 tpid, 
> ovs_be16 tci)
>  memmove(veh, (char *)veh + VLAN_HEADER_LEN, 2 * ETH_ADDR_LEN);
>  veh->veth_type = tpid;
>  veh->veth_tci = tci & htons(~VLAN_CFI);
> +
> +#ifdef DPDK_NETDEV
> +struct rte_mbuf *pkt = &(packet->mbuf);
> +pkt->l2_len += sizeof(struct vlan_hdr);
> +#endif
>  }
>  
>  /* Removes outermost VLAN header (if any is present) from 'packet'.
> @@ -221,6 +230,11 @@ eth_pop_vlan(struct dp_packet *packet)
>  memmove((char *)veh + VLAN_HEADER_LEN, veh, 2 * ETH_ADDR_LEN);
>  dp_packet_resize_l2(packet, -VLAN_HEADER_LEN);
>  }
> +
> +#ifdef DPDK_NETDEV
> +struct rte_mbuf *pkt = &(packet->mbuf);
> +pkt->l2_len -= sizeof(struct vlan_hdr);
> +#endif
>  }
>  
>  /* Set ethertype of the packet. */

Would it be better to change the dp_packet_resize_l2 call?  Are you
worried about the mpls case?
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V7 00/24] Introducing HW offload support for openvswitch

[Flavio Leitner]

Hi Marcelo,

Could you please confirm if this patch series fixes the aggregation
issue in your environment?

Thanks,
fbl


On Fri, Apr 07, 2017 at 04:12:47PM +0300, Roi Dayan wrote:
> This patch series introduces rule offload functionality to dpif-netlink
> via netdev ports new flow offloading API. The user can specify whether to
> enable rule offloading or not via OVS configuration. Netdev providers
> are able to implement netdev flow offload API in order to offload rules.
> 
> This patch series also implements one offload scheme for netdev-linux,
> using TC flower classifier, which was chosen because its sort of natural
> to state OVS DP rules for this classifier. However, the code can be
> extended to support other classifiers such as U32, eBPF, etc which support
> offload as well.
> 
> The use-case we are currently addressing is the newly sriov switchdev mode
> in the Linux kernel which was introduced in version 4.8.
> This series was tested against sriov vfs vports representors of the
> Mellanox 100G ConnectX-4 series exposed by the mlx5 kernel driver.
> 
> 
> V6->V7:
> - Fix L3 IPv4 matching got broken
> - Refactor offloads test and testsuite to have same prefix
> - Better handling of unsupported match attributes
> 
> V5->V6:
> - Rebase over master branch, fix compilation issue
> - Add Nicira copyright to tc interface
> 
> V4->V5:
> - Fix compat
> - Fix VXLAN IPv6 tunnel matching
> - Fix order of actions in dump flows
> - Update ovs-dpctl man page about the addtion of type to dump-flows
> 
> Travis
> https://travis-ci.org/roidayan/ovs/builds/213735371
> AppVeyor
> https://ci.appveyor.com/project/roidayan/ovs/build/1.0.18
> 
> V3->V4:
> - Move declarations to the right commit with implementation
> - Fix tc_get_flower flow return false success 
> - Fix memory leaks - not releasing tc_transact replies
> - Fix travis failure for OSX compilation
> - Fix loop in dpif_netlink_flow_dump_next
> - Fix declared default value for tc-policy in vswitch.xml
> - Refactor loop in netdev_tc_flow_dump_next
> - Add missing error checks in parse_flow_put
> - Fix handling modify request where old rule is in hw and new
>   rule is not supported and needs to be in sw.
> - Use 2 hashmaps instead of 1 for faster reverse lookup of ufid from tc
> - Init ports when enabling hw-offload after OVS is running
> 
> TODO: Fix breaking of datapath compilation
>   Fix testsuite failures
> 
> Travis
> https://travis-ci.org/roidayan/ovs/builds/210549325
> AppVeyor
> https://ci.appveyor.com/project/roidayan/ovs/build/1.0.15
> 
> V2->V3:
> - Code styling fixes
> - Bug fixes
> - Using already available macros/functions to match current OVS code
> - Refactored code according to V2 review
> - Replaced bool option skip-hw for string option tc-policy
> - Added hw offload tests using policy skip_hw
> - Fixed most compatability compiling issues
> - Travis
> https://travis-ci.org/roidayan/ovs/builds/199610124
> - AppVeyor
> https://ci.appveyor.com/project/roidayan/ovs/build/1.0.14
> - Fixed compiling with DPDK enabled
> 
> TODO:
> - need to fix datapath compiling issues found in travis after adding tc
>   compatability headers
> - need to fix failing test cases because of get_ifindex
> 
> V1->V2:
> - Added generic netdev flow offloads API.
> - Implemented relevant flow API in netdev-linux (and netdev-vport).
> - Added a other_config hw-offload option to enable offloading (defaults 
> to false).
> - Fixed coding style to conform with OVS.
> - Policy removed for now. (Will be discussed how best implemented later).
> 
> 
> Thanks,
> Paul & Roi
> 
> 
> Paul Blakey (24):
>   tc: Add tc flower interface
>   netdev: Adding a new netdev api to be used for offloading flows
>   other-config: Add hw-offload switch to control netdev flow offloading
>   other-config: Add tc-policy switch to control tc flower flag
>   dpif: Save added ports in a port map for netdev flow api use
>   dpif-netlink: Flush added ports using netdev flow api
>   netdev-tc-offloads: Implement netdev flow flush using tc interface
>   dpif-netlink: Dump netdevs flows on flow dump
>   netdev-tc-offloads: Add ufid to tc/netdev map
>   netdev-tc-offloads: Implement netdev flow dump api using tc interface
>   dpif-netlink: Use netdev flow put api to insert a flow
>   netdev-tc-offloads: Add flower mask to priority map
>   netdev-tc-offloads: Implement netdev flow put using tc interface
>   dpif-netlink: Use netdev flow del api to delete a flow
>   netdev-tc-offloads: Implement netdev flow del using tc interface
>   dpif-netlink: Use netdev flow get api to query a flow
>   netdev-tc-offloads: Implement flow get using tc interface
>   netdev-linux: Disallow setting policing when configured with hw
> offload
>   netdev-vport: Use common 

Re: [ovs-dev] [PATCH] dpif-netlink: fix memory leak in dpif_netlink_port_del__() on WIN32

[Alin Serdean]

Thanks a lot for fixing the leak Eric!

We should apply on 2.7 as well, but needs a rebase.

Is it okay if we add the function call on 2.7 as well or should we limit it too 
WIN32?

Acked-by: Alin Gabriel Serdean 

> -Original Message-
> From: ovs-dev-boun...@openvswitch.org [mailto:ovs-dev-
> boun...@openvswitch.org] On Behalf Of Eric Garver
> Sent: Tuesday, April 11, 2017 4:15 PM
> To: ovs dev 
> Subject: Re: [ovs-dev] [PATCH] dpif-netlink: fix memory leak in
> dpif_netlink_port_del__() on WIN32
> 
> Do any of the windows folks have any feediback on the below patch?
> 
> Thanks.
> Eric.
> 
> On Tue, Apr 04, 2017 at 03:31:29PM -0700, Joe Stringer wrote:
> > On 4 April 2017 at 13:31, Eric Garver  wrote:
> > > Furthermore, the return code of dpif_netlink_port_query__() was not
> > > being checked.
> > >
> > > Signed-off-by: Eric Garver 
> > > ---
> >
> > For context, yes this adds the call to the Linux path as well but this
> > is already in the proposed rtnetlink patch series so I'm not concerned
> > about this.
> >
> > Here's my Ack. I'd like some windows folk to be aware and take a look
> > too though:
> > Acked-by: Joe Stringer 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovs V7 21/24] dpctl: Add an option to dump only certain kinds of flows

[Flavio Leitner]

On Fri, Apr 07, 2017 at 04:13:08PM +0300, Roi Dayan wrote:
> From: Paul Blakey 
> 
> Usage:
> # to dump all datapath flows (default):
> ovs-dpctl dump-flows
> 
> # to dump only flows that in kernel datapath:
> ovs-dpctl dump-flows type=ovs
> 
> # to dump only flows that are offloaded:
> ovs-dpctl dump-flows type=offloaded


It should return an error if the type= is wrong/unavailable.

Flavio

> 
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> Reviewed-by: Simon Horman 
> ---
>  lib/dpctl.c   | 21 ++-
>  lib/dpctl.man |  7 -
>  lib/dpif-netdev.c |  3 ++-
>  lib/dpif-netlink.c| 62 
> ++-
>  lib/dpif-provider.h   |  6 +++--
>  lib/dpif.c|  4 +--
>  lib/dpif.h|  3 ++-
>  ofproto/ofproto-dpif-upcall.c |  3 ++-
>  ofproto/ofproto-dpif.c|  2 +-
>  9 files changed, 84 insertions(+), 27 deletions(-)
> 
> diff --git a/lib/dpctl.c b/lib/dpctl.c
> index 11be857..f534de2 100644
> --- a/lib/dpctl.c
> +++ b/lib/dpctl.c
> @@ -762,6 +762,7 @@ dpctl_dump_flows(int argc, const char *argv[], struct 
> dpctl_params *dpctl_p)
>  char *name;
>  
>  char *filter = NULL;
> +char *type = NULL;
>  struct flow flow_filter;
>  struct flow_wildcards wc_filter;
>  
> @@ -774,22 +775,29 @@ dpctl_dump_flows(int argc, const char *argv[], struct 
> dpctl_params *dpctl_p)
>  struct dpif_flow_dump *flow_dump;
>  struct dpif_flow f;
>  int pmd_id = PMD_ID_NULL;
> +int lastargc = 0;
>  int error;
>  
> -if (argc > 1 && !strncmp(argv[argc - 1], "filter=", 7)) {
> -filter = xstrdup(argv[--argc] + 7);
> +while (argc > 1 && lastargc != argc) {
> +lastargc = argc;
> +if (!strncmp(argv[argc - 1], "filter=", 7)) {
> +filter = xstrdup(argv[--argc] + 7);
> +} else if (!strncmp(argv[argc - 1], "type=", 5)) {
> +type = xstrdup(argv[--argc] + 5);
> +}
>  }
> +
>  name = (argc == 2) ? xstrdup(argv[1]) : get_one_dp(dpctl_p);
>  if (!name) {
>  error = EINVAL;
> -goto out_freefilter;
> +goto out_free;
>  }
>  
>  error = parsed_dpif_open(name, false, );
>  free(name);
>  if (error) {
>  dpctl_error(dpctl_p, error, "opening datapath");
> -goto out_freefilter;
> +goto out_free;
>  }
>  
>  
> @@ -818,7 +826,7 @@ dpctl_dump_flows(int argc, const char *argv[], struct 
> dpctl_params *dpctl_p)
>  BUILD_ASSERT(PMD_ID_NULL != NON_PMD_CORE_ID);
>  
>  ds_init();
> -flow_dump = dpif_flow_dump_create(dpif, false);
> +flow_dump = dpif_flow_dump_create(dpif, false, (type ? type : "dpctl"));
>  flow_dump_thread = dpif_flow_dump_thread_create(flow_dump);
>  while (dpif_flow_dump_next(flow_dump_thread, , 1)) {
>  if (filter) {
> @@ -870,8 +878,9 @@ out_dpifclose:
>  simap_destroy(_portno);
>  hmap_destroy(_names);
>  dpif_close(dpif);
> -out_freefilter:
> +out_free:
>  free(filter);
> +free(type);
>  return error;
>  }
>  
> diff --git a/lib/dpctl.man b/lib/dpctl.man
> index 2fcbc94..efd459f 100644
> --- a/lib/dpctl.man
> +++ b/lib/dpctl.man
> @@ -99,7 +99,7 @@ default.  When multiple datapaths exist, then a datapath 
> name is
>  required.
>  .
>  .TP
> -.DO "[\fB\-m \fR| \fB\-\-more\fR]" \*(DX\fBdump\-flows\fR "[\fIdp\fR] 
> [\fBfilter=\fIfilter\fR]"
> +.DO "[\fB\-m \fR| \fB\-\-more\fR]" \*(DX\fBdump\-flows\fR "[\fIdp\fR] 
> [\fBfilter=\fIfilter\fR] [\fBtype=\fItype\fR]"
>  Prints to the console all flow entries in datapath \fIdp\fR's flow
>  table.  Without \fB\-m\fR or \fB\-\-more\fR, output omits match fields
>  that a flow wildcards entirely; with \fB\-m\fR or \fB\-\-more\fR,
> @@ -112,6 +112,11 @@ not an OpenFlow flow: besides other differences, it 
> never contains wildcards.)
>  The \fIfilter\fR is also useful to match wildcarded fields in the datapath
>  flow. As an example, \fBfilter='tcp,tp_src=100'\fR will match the
>  datapath flow containing '\fBtcp(src=80/0xff00,dst=8080/0xff)\fR'.
> +.IP
> +If \fBtype=\fItype\fR is specified, only displays flows of a specific type.
> +\fItype\fR can be \fBoffloaded\fR to display only offloaded rules or 
> \fBOVS\fR
> +to display only non-offloaded rules.
> +By default both offloaded and non-offloaded rules are displayed.
>  .
>  .IP "\*(DX\fBadd\-flow\fR [\fIdp\fR] \fIflow actions\fR"
>  .TP
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index a14a2eb..a70e3e1 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -2612,7 +2612,8 @@ dpif_netdev_flow_dump_cast(struct dpif_flow_dump *dump)
>  }
>  
>  static struct dpif_flow_dump *
> -dpif_netdev_flow_dump_create(const struct dpif *dpif_, bool terse)
> +dpif_netdev_flow_dump_create(const struct dpif *dpif_, bool terse,
> +  

Re: [ovs-dev] [PATCH ovs V7 07/24] netdev-tc-offloads: Implement netdev flow flush using tc interface

[Flavio Leitner]

On Fri, Apr 07, 2017 at 04:12:54PM +0300, Roi Dayan wrote:
> From: Paul Blakey 
> 
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> Reviewed-by: Simon Horman 
> ---
>  lib/netdev-tc-offloads.c | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/netdev-tc-offloads.c b/lib/netdev-tc-offloads.c
> index 946c2a6..2ea7fe3 100644
> --- a/lib/netdev-tc-offloads.c
> +++ b/lib/netdev-tc-offloads.c
> @@ -75,10 +75,20 @@
>  
>  VLOG_DEFINE_THIS_MODULE(netdev_tc_offloads);
>  
> +static struct vlog_rate_limit rl_err = VLOG_RATE_LIMIT_INIT(, 5);

That doesn't look right.


> +
>  int
> -netdev_tc_flow_flush(struct netdev *netdev OVS_UNUSED)
> +netdev_tc_flow_flush(struct netdev *netdev)
>  {
> -return EOPNOTSUPP;
> +int ifindex = netdev_get_ifindex(netdev);
> +
> +if (ifindex < 0) {
> +VLOG_ERR_RL(_err, "failed to get ifindex for %s: %s",
> +netdev_get_name(netdev), ovs_strerror(-ifindex));
> +return -ifindex;
> +}
> +
> +return tc_flush(ifindex);
>  }
>  
>  int
> -- 
> 2.7.4
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

-- 
Flavio

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

Re: [ovs-dev] [PATCH ovs V7 06/24] dpif-netlink: Flush added ports using netdev flow api

[Flavio Leitner]

On Fri, Apr 07, 2017 at 04:12:53PM +0300, Roi Dayan wrote:
> From: Paul Blakey 
> 
> If netdev flow offloading is enabled, flush all
> added ports using netdev flow api.
> 
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> Reviewed-by: Simon Horman 
> ---

Acked-by: Flavio Leitner 


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

Re: [ovs-dev] [PATCH ovs V7 05/24] dpif: Save added ports in a port map for netdev flow api use

[Flavio Leitner]

On Fri, Apr 07, 2017 at 04:12:52PM +0300, Roi Dayan wrote:
> From: Paul Blakey 
> 
> To use netdev flow offloading api, dpifs needs to iterate over
> added ports. This addition inserts the added dpif ports in a hash map,
> The map will also be used to translate dpif ports to netdevs.
> 
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> Reviewed-by: Simon Horman 
> ---

Acked-by: Flavio Leitner 


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

Re: [ovs-dev] [PATCH ovs V7 04/24] other-config: Add tc-policy switch to control tc flower flag

[Flavio Leitner]

On Fri, Apr 07, 2017 at 04:12:51PM +0300, Roi Dayan wrote:
> From: Paul Blakey 
> 
> Add a new configuration tc-policy option that controls tc
> flower flag. Possible options are none, skip_sw, skip_hw.
> The default is none which is to insert the rule both to sw and hw.
> This option is only relevant if hw-offload is enabled.
> 
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> Reviewed-by: Simon Horman 
> ---
>  lib/netdev.c |  6 ++
>  lib/tc.c | 43 ++-
>  lib/tc.h |  1 +
>  vswitchd/vswitch.xml | 17 +
>  4 files changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/netdev.c b/lib/netdev.c
> index 977844a..6bde14b 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -54,6 +54,9 @@
>  #include "openvswitch/vlog.h"
>  #include "flow.h"
>  #include "util.h"
> +#ifdef __linux__
> +#include "tc.h"
> +#endif
>  
>  VLOG_DEFINE_THIS_MODULE(netdev);
>  
> @@ -2115,6 +2118,9 @@ netdev_set_flow_api_enabled(const struct smap 
> *ovs_other_config)
>  
>  VLOG_INFO("netdev: Flow API Enabled");
>  
> +tc_set_policy(smap_get_def(ovs_other_config, "tc-policy",
> +   TC_POLICY_DEFAULT));
> +
>  ovsthread_once_done();

Well, this enforces that the user sets tc-policy before enabling HW
offloading, otherwise it won't get set, right?



>  }
>  }
> diff --git a/lib/tc.c b/lib/tc.c
> index cd06025..8d3ddb6 100644
> --- a/lib/tc.c
> +++ b/lib/tc.c
> @@ -39,6 +39,14 @@ VLOG_DEFINE_THIS_MODULE(tc);
>  
>  static struct vlog_rate_limit parse_err = VLOG_RATE_LIMIT_INIT(5, 5);
>  
> +enum tc_offload_policy {
> +TC_POLICY_NONE,
> +TC_POLICY_SKIP_SW,
> +TC_POLICY_SKIP_HW
> +};
> +
> +static enum tc_offload_policy tc_policy = TC_POLICY_NONE;
> +
>  /* Returns tc handle 'major':'minor'. */
>  unsigned int
>  tc_make_handle(unsigned int major, unsigned int minor)
> @@ -765,6 +773,18 @@ tc_get_flower(int ifindex, int prio, int handle, struct 
> tc_flower *flower)
>  return error;
>  }
>  
> +static int
> +tc_get_tc_cls_policy(enum tc_offload_policy policy)
> +{
> +if (policy == TC_POLICY_SKIP_HW) {
> +return TCA_CLS_FLAGS_SKIP_HW;
> +} else if (policy == TC_POLICY_SKIP_SW) {
> +return TCA_CLS_FLAGS_SKIP_SW;
> +}
> +
> +return 0;
> +}
> +
>  static void
>  nl_msg_put_act_push_vlan(struct ofpbuf *request, uint16_t vid, uint8_t prio)
>  {
> @@ -1062,7 +1082,7 @@ nl_msg_put_flower_options(struct ofpbuf *request, 
> struct tc_flower *flower)
>  }
>  }
>  
> -nl_msg_put_u32(request, TCA_FLOWER_FLAGS, 0);
> +nl_msg_put_u32(request, TCA_FLOWER_FLAGS, 
> tc_get_tc_cls_policy(tc_policy));
>  
>  if (flower->tunnel.tunnel) {
>  nl_msg_put_flower_tunnel(request, flower);
> @@ -1107,3 +1127,24 @@ tc_replace_flower(int ifindex, uint16_t prio, uint32_t 
> handle,
>  
>  return error;
>  }
> +
> +void
> +tc_set_policy(const char *policy)
> +{
> +if (!policy) {
> +return;
> +}
> +
> +if (!strcmp(policy, "skip_sw")) {
> +tc_policy = TC_POLICY_SKIP_SW;
> +} else if (!strcmp(policy, "skip_hw")) {
> +tc_policy = TC_POLICY_SKIP_HW;
> +} else if (!strcmp(policy, "none")) {
> +tc_policy = TC_POLICY_NONE;
> +} else {
> +VLOG_WARN("tc: Invalid policy '%s'", policy);
> +return;
> +}
> +
> +VLOG_INFO("tc: Using policy '%s'", policy);
> +}
> diff --git a/lib/tc.h b/lib/tc.h
> index ec2e05b..1b3b3b6 100644
> --- a/lib/tc.h
> +++ b/lib/tc.h
> @@ -124,5 +124,6 @@ int tc_flush(int ifindex);
>  int tc_dump_flower_start(int ifindex, struct nl_dump *dump);
>  int parse_netlink_to_tc_flower(struct ofpbuf *reply,
> struct tc_flower *flower);
> +void tc_set_policy(const char *policy);
>  
>  #endif /* tc.h */
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index de86049..9c4ea90 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -183,6 +183,23 @@
>  
>
>  
> +   +  type='{"type": "string"}'>
> +
> +Specified the policy used with HW offloading.
> +Options:
> +none- Add software rule and offload rule to 
> HW.
> +skip_sw - Offload rule to HW only.
> +skip_hw - Add software rule without offloading 
> rule to HW.
> +
> +
> +This is only relevant if HW offloading is enabled (hw-offload).
> +
> +
> +  The default value is none.
> +
> +  
> +
>type='{"type": "boolean"}'>
>  
> -- 
> 2.7.4
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

-- 
Flavio

Re: [ovs-dev] [PATCH ovs V7 03/24] other-config: Add hw-offload switch to control netdev flow offloading

[Flavio Leitner]

On Fri, Apr 07, 2017 at 04:12:50PM +0300, Roi Dayan wrote:
> From: Paul Blakey 
> 
> Add a new configuration option - hw-offload that enables netdev
> flow api. Enabling this option will allow offloading flows
> using netdev implementation instead of the kernel datapath.
> This configuration option defaults to false - disabled.
> 
> Signed-off-by: Paul Blakey 
> Reviewed-by: Roi Dayan 
> Reviewed-by: Simon Horman 
> ---
>  lib/netdev.c | 30 ++
>  lib/netdev.h |  2 ++
>  vswitchd/bridge.c|  1 +
>  vswitchd/vswitch.xml | 11 +++
>  4 files changed, 44 insertions(+)
> 
> diff --git a/lib/netdev.c b/lib/netdev.c
> index f7b80b2..977844a 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -2092,7 +2092,37 @@ netdev_init_flow_api(struct netdev *netdev)
>  {
>  const struct netdev_class *class = netdev->netdev_class;
>  
> +if (!netdev_flow_api_enabled) {
> +return EOPNOTSUPP;
> +}
> +
>  return (class->init_flow_api
>  ? class->init_flow_api(netdev)
>  : EOPNOTSUPP);
>  }
> +
> +bool netdev_flow_api_enabled = false;
> +
> +#ifdef __linux__
> +void
> +netdev_set_flow_api_enabled(const struct smap *ovs_other_config)
> +{
> +if (smap_get_bool(ovs_other_config, "hw-offload", false)) {
> +static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
> +
> +if (ovsthread_once_start()) {
> +netdev_flow_api_enabled = true;
> +
> +VLOG_INFO("netdev: Flow API Enabled");
> +
> +ovsthread_once_done();
> +}
> +}
> +}
> +#else
> +void
> +netdev_set_flow_api_enabled(const struct smap *ovs_other_config OVS_UNUSED)
> +{
> +netdev_flow_api_enabled = false;
> +}
> +#endif
> diff --git a/lib/netdev.h b/lib/netdev.h
> index 6d2db7d..e2d1799 100644
> --- a/lib/netdev.h
> +++ b/lib/netdev.h
> @@ -178,6 +178,8 @@ int netdev_flow_get(struct netdev *, struct match *, 
> struct nlattr **actions,
>  int netdev_flow_del(struct netdev *, const ovs_u128 *,
>  struct dpif_flow_stats *);
>  int netdev_init_flow_api(struct netdev *);
> +extern bool netdev_flow_api_enabled;
> +void netdev_set_flow_api_enabled(const struct smap *ovs_other_config);
>  
>  /* native tunnel APIs */
>  /* Structure to pass parameters required to build a tunnel header. */
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 867a26d..0f65858 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -2953,6 +2953,7 @@ bridge_run(void)
>  cfg = ovsrec_open_vswitch_first(idl);
>  
>  if (cfg) {
> +netdev_set_flow_api_enabled(>other_config);
>  dpdk_init(>other_config);
>  }
>  
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 14297bf..de86049 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -170,6 +170,17 @@
>  
>The default is 1.
>  
> +
> +
> +   +  type='{"type": "boolean"}'>
> +
> +  Set this value to true to enable netdev flow offload.
> +
> +
> +  The default value is false. Changing this value 
> requires
> +  restarting the daemon
> +


Since this is a compile time feature, I think somehow we need to 
document that it's only available on Linux. Something like:

Currently Open vSwitch supports hardware offloading on
Linux systems.  On other systems, this value is ignored.



This looks better to me:

---8<---
diff --git a/lib/netdev.c b/lib/netdev.c
index 64f5e85..4b5f1990 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -2330,13 +2330,13 @@ netdev_ports_flow_init(void)
 }
 
 void
-netdev_set_flow_api_enabled(const struct smap *ovs_other_config)
+netdev_set_flow_api_enabled(bool enabled)
 {
 if (!netdev_flow_api_supported) {
 return;
 }
 
-if (smap_get_bool(ovs_other_config, "hw-offload", false)) {
+if (enabled) {
 static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
 
 if (ovsthread_once_start()) {
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 0f65858..18bbeb6 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2953,7 +2953,8 @@ bridge_run(void)
 cfg = ovsrec_open_vswitch_first(idl);
 
 if (cfg) {
-netdev_set_flow_api_enabled(>other_config);
+netdev_set_flow_api_enabled(smap_get_bool(>other_config,
+"hw-offload", false));
 dpdk_init(>other_config);
 }
---8<---



Also that it's exporting a variable, so maybe this would be better:
(compile tested on Linux only)


---8<---
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index acce90e..0adc9c7 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1088,7 +1088,7 @@ dpif_netlink_flow_flush(struct dpif *dpif_)
 flow.cmd = OVS_FLOW_CMD_DEL;
 flow.dp_ifindex = dpif->dp_ifindex;
 
-if 

Re: [ovs-dev] [PATCH] docs: Update version numbers in doc config.

[Russell Bryant]

On Wed, Apr 12, 2017 at 9:12 AM, Stephen Finucane  wrote:
> On Wed, 2017-04-12 at 08:58 -0400, Russell Bryant wrote:
>> On Wed, Apr 12, 2017 at 7:29 AM, Stephen Finucane 
>> wrote:
>> > On Tue, 2017-04-11 at 13:15 -0400, Russell Bryant wrote:
>> > > Update the version numbers in the documentation config to reflect
>> > > 2.7.90
>> > > instead of 2.6.0.
>> > >
>> > > This patch also updates the build system to automatically update
>> > > this
>> > > file.
>> > > conf.py is now a generated file from conf.py.in.  We still
>> > > include
>> > > conf.py
>> > > in the tree because it's needed for the docs to be automatically
>> > > generated
>> > > for docs.openvswitch.org.
>> > >
>> > > Signed-off-by: Russell Bryant 
>> >
>> > Looks OK, but how would we prevent this getting out of date? If
>> > we're
>> > going to include PATCH information, I imagine this would need to be
>> > updated as part of every commit?
>>
>> It doesn't change with every commit.  ".90" is fixed.  "2.7.90" is
>> the
>> version OVS uses for the master branch that will eventually become
>> 2.8.
>
> I realized that about 10 minutes ago after diving further into the git
> history of NEWS :) Why '.90' , out of curiosity?

It predates my involvement, but I think it's roughly:

2.7.90 === much more than 2.7.0, but not quite 2.8.0

>> In terms of keeping it up to date, conf.py is going to show a change
>> as soon as the version changes, but it should only be when a new
>> branch gets created, or after a point release is made from a release
>> branch.
>
> Gotcha.
>
>> > Seeing as a more generic version of this information is already
>> > available in 'NEWS', I imagine we could just parse that instead.
>> > Lemme
>> > draft something quickly and see what you think.
>>
>> Cool, I'll take a look.
>
> Given the above, what I've done works but it likely unnecessary. Unless
> we want to update the version in real time (by parsing git logs using
> dulwich or similar), this solution is probably more consistent with
> OVS' build system. As such:
>
> Acked-by: Stephen Finucane 
>
> Cheers :)
> Stephen

Thanks for the detailed review and feedback!  I'll apply this approach, then.

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

Re: [ovs-dev] [RFC 0/5] role-based access controls for ovsdb-server, ovn-sb

[Lance Richardson]

> From: "Lance Richardson" 
> To: "Ben Pfaff" 
> Cc: d...@openvswitch.org, "mickeys dev" , "Russell 
> Bryant" 
> Sent: Thursday, 6 April, 2017 12:19:46 PM
> Subject: Re: [RFC 0/5] role-based access controls for ovsdb-server, ovn-sb
> 
> > From: "Ben Pfaff" 
> > To: "Lance Richardson" 
> > Cc: d...@openvswitch.org, russe...@ovn.org, "mickeys dev"
> > 
> > Sent: Thursday, 6 April, 2017 12:03:44 PM
> > Subject: Re: [RFC 0/5] role-based access controls for ovsdb-server, ovn-sb
> > 
> > On Mon, Mar 27, 2017 at 02:56:08PM -0400, Lance Richardson wrote:
> > > This series implements role-based access control infrastructure for
> > > ovsdb-server, and uses that infrastructure to apply role-based access
> > > controls to the OVN_Southbound database. This implementation follows
> > > the outline discussed at:
> > > 
> > >  https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329801.html
> > > 
> > > With this series applied, enabling role-based ACLs is a matter of:
> > > 
> > > - Configuring southbound ovsdb-server and ovn-controller to use SSL,
> > >   configuring an ovn-controller "role" for SSL connections via e.g.:
> > >  ovn-sbctl set-connection role=ovn-controller pssl:6642
> > > - Using unique certificates for each ovn-controller with a unique
> > >   CN for each chassis, generated e.g. via:
> > >  ovs-pki -B 1024 req+sign chassis1 switch
> > >  ovs-pki -B 1024 req+sign chassis2 switch
> > >  ovs-pki -B 1024 req+sign chassis3 switch
> > > - Starting the southbound ovsdb-server with the "--rbac" command-line
> > >   option:
> > >  --rbac=db:OVN_Southbound,RBAC_Role
> > 
> > This series is promising.
> > 
> > I'm a little concerned about additional per-DB command-line options
> > because it makes it hard to add and remove databases at runtime.
> > 

Hi Ben,

Could we extend the database schema format to add something like:

"_rbac_role": 

If so, I think we could eliminate the need to do anything extra for RBAC
support as databases are added/removed at runtime, and the --rbac= command-
line option would no longer be necessary.

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

Re: [ovs-dev] [PATCH] docs: Update version numbers in doc config.

[Stephen Finucane]

On Wed, 2017-04-12 at 08:58 -0400, Russell Bryant wrote:
> On Wed, Apr 12, 2017 at 7:29 AM, Stephen Finucane 
> wrote:
> > On Tue, 2017-04-11 at 13:15 -0400, Russell Bryant wrote:
> > > Update the version numbers in the documentation config to reflect
> > > 2.7.90
> > > instead of 2.6.0.
> > > 
> > > This patch also updates the build system to automatically update
> > > this
> > > file.
> > > conf.py is now a generated file from conf.py.in.  We still
> > > include
> > > conf.py
> > > in the tree because it's needed for the docs to be automatically
> > > generated
> > > for docs.openvswitch.org.
> > > 
> > > Signed-off-by: Russell Bryant 
> > 
> > Looks OK, but how would we prevent this getting out of date? If
> > we're
> > going to include PATCH information, I imagine this would need to be
> > updated as part of every commit?
> 
> It doesn't change with every commit.  ".90" is fixed.  "2.7.90" is
> the
> version OVS uses for the master branch that will eventually become
> 2.8.

I realized that about 10 minutes ago after diving further into the git
history of NEWS :) Why '.90' , out of curiosity?

> In terms of keeping it up to date, conf.py is going to show a change
> as soon as the version changes, but it should only be when a new
> branch gets created, or after a point release is made from a release
> branch.

Gotcha.

> > Seeing as a more generic version of this information is already
> > available in 'NEWS', I imagine we could just parse that instead.
> > Lemme
> > draft something quickly and see what you think.
> 
> Cool, I'll take a look.

Given the above, what I've done works but it likely unnecessary. Unless
we want to update the version in real time (by parsing git logs using
dulwich or similar), this solution is probably more consistent with
OVS' build system. As such:

Acked-by: Stephen Finucane 

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

Re: [ovs-dev] OVN meeting report

[Valentine Sinitsyn]

Hi,

On 04.04.2017 15:29, Valentine Sinitsyn wrote:

On 03.04.2017 20:29, Valentine Sinitsyn wrote:

Hi Ben,

On 23.03.2017 08:11, Ben Pfaff wrote:

Hello everyone.  I am not sure whether I am going to be able to attend
the OVN meeting tomorrow, because I will be in another possibly
distracting meeting, so I'm going to give my report here.

Toward the end of last week I did a full pass of reviews through
patchwork.  The most notable result, I think, is that I applied patches
that add 802.1ad support.  For OVN, this makes it more reasonable to
consider adding support for tagged logical ports--currently, OVN drops
all tagged logical packets--which I've heard requested once or twice,
because it means that they can now be gatewayed to physical ports within
an outer VLAN.  I don't have any plans to work on that, but I think that
it is worth pointing out.

The OVS "Open Source Day" talks have been scheduled at OpenStack
Boston.  They are all on Wednesday:
https://www.openstack.org/summit/boston-2017/summit-schedule/#track=135

I've been spending what dev time I have on database clustering.  Today,
I managed to get it working, with many caveats.  It will take weeks or
months longer to get it finished, tested, and ready for posting.  (If
you want what I have, check out the raft3 branch in my ovs-reviews repo
at github.)

I've checked out your raft3 branch, and even learned how to create an
OVSDB cluster. Thanks for the docs!

What I don't get though is how do I instruct IDL to connect to the
cluster now? Do I just connect to a random server, or there should be
some dispatcher, or whatever?

OK I see this is an ongoing work in your branch.


I had some time to play with raft3 branch last week.

I added very basic and hacky replica set support to IDL and brought up 
an OVN setup with clustered southbound database. It works to some 
extent, yet if I try to throw several hundreds of logical ports into the 
mix, the database becomes inconsistent. The reason is probably the race 
window between when the raft leader appends a log entry to other nodes 
(so a client such as ovn-northd already sees it) and the entry really 
appears in the leader's log itself. Not sure if it is my bug or not. The 
original code had some minor issues as well (which is absolutely normal 
for WIP) - I can send my (rather trivial) patches if there is any interest.


Is there some design outline for the missing implementation bits? 
Specifically, it would be good to know the following:


1. With clustered OVSDB, a client such as IDL needs two JSON RPC 
connections: to the leader (to commit transactions), and a read-only one 
to an arbitrary replica set (scaling reads). Will it be implemented on 
ovsdb_idl level or encapsulated inside jsonrpc_session? The former seems 
natural yet multiple remotes support went to jsonrpc_session already.


2. How does the client know which replica set member is currently a 
leader? I just loop over remotes until one accepts the transaction 
(which is an awful idea). It would be nice to send some sort of cluster 
metadata snapshot to JSON RPC client during initial handshake. 
Alternatively, one can extend the "not leader" error object with a 
leader URL.


3. For eventual consistency reasons, if an IDL reads from one member (A) 
but writes to another one (B), it can try to delete a row not yet in A's 
database. This would make all further requests fail with "inconsistent 
data" error and basically is what I observe in my tests. How do you plan 
to overcome this?


Thanks in advance!

Valentine



Best,
Valentine



Thanks,
Valentine


___
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] docs: Update version numbers in doc config.

[Russell Bryant]

On Wed, Apr 12, 2017 at 7:29 AM, Stephen Finucane  wrote:
> On Tue, 2017-04-11 at 13:15 -0400, Russell Bryant wrote:
>> Update the version numbers in the documentation config to reflect
>> 2.7.90
>> instead of 2.6.0.
>>
>> This patch also updates the build system to automatically update this
>> file.
>> conf.py is now a generated file from conf.py.in.  We still include
>> conf.py
>> in the tree because it's needed for the docs to be automatically
>> generated
>> for docs.openvswitch.org.
>>
>> Signed-off-by: Russell Bryant 
>
> Looks OK, but how would we prevent this getting out of date? If we're
> going to include PATCH information, I imagine this would need to be
> updated as part of every commit?

It doesn't change with every commit.  ".90" is fixed.  "2.7.90" is the
version OVS uses for the master branch that will eventually become
2.8.

In terms of keeping it up to date, conf.py is going to show a change
as soon as the version changes, but it should only be when a new
branch gets created, or after a point release is made from a release
branch.

> Seeing as a more generic version of this information is already
> available in 'NEWS', I imagine we could just parse that instead. Lemme
> draft something quickly and see what you think.

Cool, I'll take a look.

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

Re: [ovs-dev] [PATCH] docs: Update version numbers in doc config.

[Stephen Finucane]

On Tue, 2017-04-11 at 13:15 -0400, Russell Bryant wrote:
> Update the version numbers in the documentation config to reflect
> 2.7.90
> instead of 2.6.0.
> 
> This patch also updates the build system to automatically update this
> file.
> conf.py is now a generated file from conf.py.in.  We still include
> conf.py
> in the tree because it's needed for the docs to be automatically
> generated
> for docs.openvswitch.org.
> 
> Signed-off-by: Russell Bryant 

Looks OK, but how would we prevent this getting out of date? If we're
going to include PATCH information, I imagine this would need to be
updated as part of every commit?

Seeing as a more generic version of this information is already
available in 'NEWS', I imagine we could just parse that instead. Lemme
draft something quickly and see what you think.

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

[ovs-dev] Sync on PTAP, EXT-382 and NSH

[Jan Scheurich]

Hi,

The team is making good progress in preparing the various patch packages. We 
have a lot of things working in our gitlab repo 
(https://gitlab.com/JScheurich/ovs). v3 of the L3 tunneling patches are out on 
the mailing list since a week 
(https://mail.openvswitch.org/pipermail/ovs-dev/2017-April/330488.html) and are 
waiting for review.

The PTAP series will follow next week. Also the NSH MD1 fields and generic 
encap/decap actions for Ethernet and NSH MD1 are mostly done.

Let's have a look at the status and work out a plan how to accelerate the 
review and merging in order to achieve the agreed target to upstream these 
changes in time for OVS 2.8.

Thank you,
Jan

Link to the Google design doc:
https://docs.google.com/document/d/1oWMYUH8sjZJzWa72o2q9kU0N6pNE-rwZcLH3-kbbDR8/edit


.
--> Join Skype Meeting
  This is an online meeting for Skype for Business, the professional meetings 
and communications app formerly known as Lync.
Join by phone

+492115343925 (Germany)  English (United 
States)
89925 (Germany) English (United States)

Find a local number

Conference ID: 70849799
 Forgot your dial-in PIN? 
|Help


To join a Lync / Skype for Business meeting from an Ericsson standard video 
room, add 77 before the Conference ID (e.g. 771234567 where 1234567 is the 
conference ID).To join from a video room outside of Ericsson add one of the 
domains after 77 and Conference ID (e.g. 771234567@ .ericsson.net, where 
=emea/apac/amcs).  For assistance contact the IT Service Desk.
[!OC([1033])!]
.


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

[ovs-dev] OVS performance with Kernel Datapath of Linux upstream vs Linux OVS tree.

[Kapil Adhikesavalu]

Hi,

Is there any performance difference with using the OVS kernel Datapath
available part of Linux upstream Vs the module built from Linux OVS tree.

So far i have been using the DP part of the Linux upstream and as NAT
feature requires Linux version 4.6, i plan to switch to DP module built
from OVS tree.

1. In general is there any performance between using these two ? or would
it vary based on the features being in use.
2. I am currently using VXLAN and L2 forwarding + NAT(plan to use it),
would like to know if there could be any performance difference expected
when i switch from upstream DP to KLM.

I didn't any specific mention about performance in FAQ -
http://docs.openvswitch.org/en/latest/faq/releases/ expect for this
statement 'Certain features require kernel support to function or to have
reasonable performance.'

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