[PATCH net-next] openvswitch: do not propagate headroom updates to internal port

[PATCH net-next] openvswitch: do not propagate headroom updates to internal port

[Paolo Abeni]

After commit 3a927bc7cf9d ("ovs: propagate per dp max headroom to
all vports") the need_headroom for the internal vport is updated
accordingly to the max needed headroom in its datapath.

That avoids the pskb_expand_head() costs when sending/forwarding
packets towards tunnel devices, at least for some scenarios.

We still require such copy when using the ovs-preferred configuration
for vxlan tunnels:

    br_int
  /       \
tap      vxlan
           (remote_ip:X)

br_phy
     \
    NIC

where the route towards the IP 'X' is via 'br_phy'.

When forwarding traffic from the tap towards the vxlan device, we
will call pskb_expand_head() in vxlan_build_skb() because
br-phy->needed_headroom is equal to tun->needed_headroom.

With this change we avoid updating the internal vport needed_headroom,
so that in the above scenario no head copy is needed, giving 5%
performance improvement in UDP throughput test.

As a trade-off, packets sent from the internal port towards a tunnel
device will now experience the head copy overhead. The rationale is
that the latter use-case is less relevant performance-wise.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/openvswitch/vport-internal_dev.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c
index 04a3128adcf0..3e7747549f90 100644
--- a/net/openvswitch/vport-internal_dev.c
+++ b/net/openvswitch/vport-internal_dev.c
@@ -126,18 +126,12 @@ internal_get_stats(struct net_device *dev, struct rtnl_link_stats64 *stats)
 	}
 }
 
-static void internal_set_rx_headroom(struct net_device *dev, int new_hr)
-{
-	dev->needed_headroom = new_hr < 0 ? 0 : new_hr;
-}
-
 static const struct net_device_ops internal_dev_netdev_ops = {
 	.ndo_open = internal_dev_open,
 	.ndo_stop = internal_dev_stop,
 	.ndo_start_xmit = internal_dev_xmit,
 	.ndo_set_mac_address = eth_mac_addr,
 	.ndo_get_stats64 = internal_get_stats,
-	.ndo_set_rx_headroom = internal_set_rx_headroom,
 };
 
 static struct rtnl_link_ops internal_dev_link_ops __read_mostly = {
@@ -154,7 +148,7 @@ static void do_setup(struct net_device *netdev)
 
 	netdev->priv_flags &= ~IFF_TX_SKB_SHARING;
 	netdev->priv_flags |= IFF_LIVE_ADDR_CHANGE | IFF_OPENVSWITCH |
-			      IFF_PHONY_HEADROOM | IFF_NO_QUEUE;
+			      IFF_NO_QUEUE;
 	netdev->needs_free_netdev = true;
 	netdev->priv_destructor = internal_dev_destructor;
 	netdev->ethtool_ops = &internal_dev_ethtool_ops;
@@ -195,7 +189,6 @@ static struct vport *internal_dev_create(const struct vport_parms *parms)
 		err = -ENOMEM;
 		goto error_free_netdev;
 	}
-	vport->dev->needed_headroom = vport->dp->max_headroom;
 
 	dev_net_set(vport->dev, ovs_dp_get_net(vport->dp));
 	internal_dev = internal_dev_priv(vport->dev);
-- 
2.13.6

Re: [PATCH net-next] openvswitch: do not propagate headroom updates to internal port

[Pravin Shelar]

On Thu, Nov 30, 2017 at 6:35 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> After commit 3a927bc7cf9d ("ovs: propagate per dp max headroom to
> all vports") the need_headroom for the internal vport is updated
> accordingly to the max needed headroom in its datapath.
>
> That avoids the pskb_expand_head() costs when sending/forwarding
> packets towards tunnel devices, at least for some scenarios.
>
> We still require such copy when using the ovs-preferred configuration
> for vxlan tunnels:
>
>     br_int
>   /       \
> tap      vxlan
>            (remote_ip:X)
>
> br_phy
>      \
>     NIC
>
> where the route towards the IP 'X' is via 'br_phy'.
>
> When forwarding traffic from the tap towards the vxlan device, we
> will call pskb_expand_head() in vxlan_build_skb() because
> br-phy->needed_headroom is equal to tun->needed_headroom.
>
> With this change we avoid updating the internal vport needed_headroom,
> so that in the above scenario no head copy is needed, giving 5%
> performance improvement in UDP throughput test.
>
> As a trade-off, packets sent from the internal port towards a tunnel
> device will now experience the head copy overhead. The rationale is
> that the latter use-case is less relevant performance-wise.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Acked-by: Pravin B Shelar <pshelar@ovn.org>

Thanks.

Re: [PATCH net-next] openvswitch: do not propagate headroom updates to internal port

[David Miller]

From: Paolo Abeni <pabeni@redhat.com>
Date: Thu, 30 Nov 2017 15:35:33 +0100

> After commit 3a927bc7cf9d ("ovs: propagate per dp max headroom to
> all vports") the need_headroom for the internal vport is updated
> accordingly to the max needed headroom in its datapath.
> 
> That avoids the pskb_expand_head() costs when sending/forwarding
> packets towards tunnel devices, at least for some scenarios.
> 
> We still require such copy when using the ovs-preferred configuration
> for vxlan tunnels:
> 
>     br_int
>   /       \
> tap      vxlan
>            (remote_ip:X)
> 
> br_phy
>      \
>     NIC
> 
> where the route towards the IP 'X' is via 'br_phy'.
> 
> When forwarding traffic from the tap towards the vxlan device, we
> will call pskb_expand_head() in vxlan_build_skb() because
> br-phy->needed_headroom is equal to tun->needed_headroom.
> 
> With this change we avoid updating the internal vport needed_headroom,
> so that in the above scenario no head copy is needed, giving 5%
> performance improvement in UDP throughput test.
> 
> As a trade-off, packets sent from the internal port towards a tunnel
> device will now experience the head copy overhead. The rationale is
> that the latter use-case is less relevant performance-wise.
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Applied, thanks.