* Re: [pull request][net-next 0/3] Mellanox, mlx5 GRE tunnel offloads
From: Tom Herbert @ 2017-09-05 21:13 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Saeed Mahameed, Saeed Mahameed, David S. Miller,
Linux Netdev List
In-Reply-To: <87fuc1q7en.fsf@stressinduktion.org>
> The situation with encapsulation is even more complicated:
>
> We are basically only interested in the UDP/vxlan/Ethernet/IP/UDP
> constellation. If we do the fragmentation inside the vxlan tunnel and
> carry over the skb hash to all resulting UDP/vxlan packets source ports,
> we are fine and reordering on the receiver NIC won't happen in this
> case. If the fragmentation happens on the outer UDP header, this will
> result in reordering of the inner L2 flow. Unfortunately this depends on
> how the vxlan tunnel was set up, how other devices do that and (I
> believe so) on the kernel version.
>
This really isn't that complicated. The assumption that an IP network
always delivers packets in order is simply wrong. The inventors of
VXLAN must have know full well that when you use IP, packets can and
eventually will be delivered out of order. This isn't just because of
fragmentation, there are many other reasons that packets can be
delivered OOO. This also must have been known when IP/GRE and any
other protocol that carries L2 over IP was invented. If OOO is an
issue for these protocols then they need to be fixed-- this is not a
concern with IP protocol nor the stack.
Tom
^ permalink raw reply
* Re: [PATCH 6/10] ixgbe: Use ARRAY_SIZE macro
From: David Miller @ 2017-09-05 21:22 UTC (permalink / raw)
To: joe; +Cc: thomas, netdev, linux-kernel
In-Reply-To: <1504641678.28899.12.camel@perches.com>
From: Joe Perches <joe@perches.com>
Date: Tue, 05 Sep 2017 13:01:18 -0700
> On Tue, 2017-09-05 at 21:45 +0200, Thomas Meyer wrote:
>> On Tue, Sep 05, 2017 at 11:50:44AM -0700, David Miller wrote:
>> > From: Thomas Meyer <thomas@m3y3r.de>
>> > Date: Sun, 03 Sep 2017 14:19:31 +0200
>> >
>> > > Use ARRAY_SIZE macro, rather than explicitly coding some variant of it
>> > > yourself.
>> > > Found with: find -type f -name "*.c" -o -name "*.h" | xargs perl -p -i -e
>> > > 's/\bsizeof\s*\(\s*(\w+)\s*\)\s*\ /\s*sizeof\s*\(\s*\1\s*\[\s*0\s*\]\s*\)
>> > > /ARRAY_SIZE(\1)/g' and manual check/verification.
>> > >
>> > > Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
>> >
>> > This should be submitted to the Intel ethernet driver maintainers.
>>
>> Hi,
>>
>> my script checks the output of get_maintainer scripts and only sends to "open
>> list" entries.
>>
>> The intel-wired-lan@lists.osuosl.org is moderated, so that's why the patch
>> wasn't send there.
>>
>> Strangely the lists for nouveau@lists.freedesktop.org and
>> intel-gvt-dev@lists.freedesktop.org appears as open lists in the MAINTAINERS
>> file but seems to be also moderated lists... At least I got some reply that my
>> message awaits approval. Maybe an update to the MAINTAINERS file is missing
>> here?
>>
>> I may drop above check in my script and send to all mailing lists that
>> get_maintainer.pl will return.
>
> There's a difference between moderated and subscriber-only
> entries in MAINTAINERS.
>
> get_maintainers will by default list moderated lists and
> not show subscriber-only lists unless using the -s switch.
Furthermore, nothing prevented you from CC:'ing the maintainer,
Jeff Kirscher.
^ permalink raw reply
* Re: [PATCH net-next] net: cpsw: Don't handle SIOC[GS]HWTSTAMP when CPTS is disabled
From: Grygorii Strashko @ 2017-09-05 21:25 UTC (permalink / raw)
To: Richard Cochran, David Miller; +Cc: stefan.sorensen, netdev, linux-omap
In-Reply-To: <20170831074807.3dbbt6qhqxdquipa@localhost>
Hi
On 08/31/2017 02:48 AM, Richard Cochran wrote:
> On Wed, Aug 30, 2017 at 02:47:45PM -0700, David Miller wrote:
>> It should not be required to disable a Kconfig option just to get PHY
>> timestamping to work properly.
>
> Well, if the MAC driver handles the ioctl and enables time stamping,
> then the PHY driver's time stamping remains disabled. We don't have a
> way to choose PHY time stamping at run time.
>
>> Rather, if the CPTS code returns -EOPNOTSUPP we should try to
>> fallthrough to the PHY library based methods.
>
> I agree that it would be better for the core (rather than the
> individual drivers) to handle this case.
I'd like to clarify one thing here - what is the preferable time-stamping
device: PHY over MAC, or MAC over PHY?
my understanding it's PHY and ethtool_get_ts_info() seems already implemented this way.
>
> There are a few callers of .ndo_do_ioctl to consider. Besides
> dev_ifsioc() there is at least vlan_dev_ioctl() that needs to handle
> the EOPNOTSUPP.
Sry, I've not tried to do solution in Net core, but below patch updates CPSW
driver to selected PHY time-stamping over MAC if supported without disabling CPTS in Kconfig
(at least it's expected to fix it) - not tested as I do not have HW with dp83640 phy.
-----------------------------------------------------------------------
>From 51347692087732320f2f5615030f5f36ed3c7724 Mon Sep 17 00:00:00 2001
From: Grygorii Strashko <grygorii.strashko@ti.com>
Date: Tue, 5 Sep 2017 15:24:44 -0500
Subject: [PATCH] net: ethernet: cpsw: allow phy timestamping over mac
Allow phy timestamping to be used over mac timestamping if supported.
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
drivers/net/ethernet/ti/cpsw.c | 21 +++++++++++++++++----
drivers/net/ethernet/ti/cpts.c | 2 ++
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 95ac926..8831cb9 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -291,6 +291,10 @@ struct cpsw_ss_regs {
#define CPSW_MAX_BLKS_TX_SHIFT 4
#define CPSW_MAX_BLKS_RX 5
+#define HAS_PHY_TXTSTAMP(p) ((p) && (p)->drv && (p)->drv->txtstamp)
+#define HAS_PHY_TSTAMP(p) ((p) && (p)->drv && \
+ ((p)->drv->rxtstamp || (p)->drv->rxtstamp))
+
struct cpsw_host_regs {
u32 max_blks;
u32 blk_cnt;
@@ -1600,6 +1604,8 @@ static netdev_tx_t cpsw_ndo_start_xmit(struct sk_buff *skb,
{
struct cpsw_priv *priv = netdev_priv(ndev);
struct cpsw_common *cpsw = priv->cpsw;
+ int slave_no = cpsw_slave_index(cpsw, priv);
+ struct phy_device *phy = cpsw->slaves[slave_no].phy;
struct cpts *cpts = cpsw->cpts;
struct netdev_queue *txq;
struct cpdma_chan *txch;
@@ -1611,8 +1617,9 @@ static netdev_tx_t cpsw_ndo_start_xmit(struct sk_buff *skb,
return NET_XMIT_DROP;
}
- if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP &&
- cpts_is_tx_enabled(cpts) && cpts_can_timestamp(cpts, skb))
+ if ((skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
+ cpts_can_timestamp(cpts, skb) &&
+ (cpts_is_tx_enabled(cpts) || HAS_PHY_TXTSTAMP(phy)))
skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
q_idx = skb_get_queue_mapping(skb);
@@ -1810,20 +1817,26 @@ static int cpsw_ndo_ioctl(struct net_device *dev, struct ifreq *req, int cmd)
struct cpsw_priv *priv = netdev_priv(dev);
struct cpsw_common *cpsw = priv->cpsw;
int slave_no = cpsw_slave_index(cpsw, priv);
+ struct phy_device *phy = cpsw->slaves[slave_no].phy;
+
if (!netif_running(dev))
return -EINVAL;
switch (cmd) {
case SIOCSHWTSTAMP:
+ if (HAS_PHY_TSTAMP(phy))
+ break;
return cpsw_hwtstamp_set(dev, req);
case SIOCGHWTSTAMP:
+ if (HAS_PHY_TSTAMP(phy))
+ break;
return cpsw_hwtstamp_get(dev, req);
}
- if (!cpsw->slaves[slave_no].phy)
+ if (!phy)
return -EOPNOTSUPP;
- return phy_mii_ioctl(cpsw->slaves[slave_no].phy, req, cmd);
+ return phy_mii_ioctl(phy, req, cmd);
}
static void cpsw_ndo_tx_timeout(struct net_device *ndev)
diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
index c2121d2..f257f54 100644
--- a/drivers/net/ethernet/ti/cpts.c
+++ b/drivers/net/ethernet/ti/cpts.c
@@ -421,6 +421,8 @@ void cpts_tx_timestamp(struct cpts *cpts, struct sk_buff *skb)
u64 ns;
struct skb_shared_hwtstamps ssh;
+ if (!cpts->rx_enable)
+ return;
if (!(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS))
return;
ns = cpts_find_ts(cpts, skb, CPTS_EV_TX);
--
2.10.1
--
regards,
-grygorii
^ permalink raw reply related
* Re: [PATCH net-next 1/2] tun: reserve extra headroom only when XDP is set
From: David Miller @ 2017-09-05 21:26 UTC (permalink / raw)
To: jasowang; +Cc: netdev, linux-kernel, mst, kubakici
In-Reply-To: <1504496169-31190-1-git-send-email-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Mon, 4 Sep 2017 11:36:08 +0800
> We reserve headroom unconditionally which could cause unnecessary
> stress on socket memory accounting because of increased trusesize. Fix
> this by only reserve extra headroom when XDP is set.
>
> Cc: Jakub Kicinski <kubakici@wp.pl>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 2/2] tun: rename generic_xdp to skb_xdp
From: David Miller @ 2017-09-05 21:26 UTC (permalink / raw)
To: jasowang; +Cc: netdev, linux-kernel, mst, daniel
In-Reply-To: <1504496169-31190-2-git-send-email-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Mon, 4 Sep 2017 11:36:09 +0800
> Rename "generic_xdp" to "skb_xdp" to avoid confusing it with the
> generic XDP which will be done at netif_receive_skb().
>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] cxgb4: fix memory leak
From: David Miller @ 2017-09-05 21:28 UTC (permalink / raw)
To: ganeshgr; +Cc: netdev, nirranjan, indranil, venkatesh
In-Reply-To: <1504503988-21903-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Mon, 4 Sep 2017 11:16:28 +0530
> do not reuse the loop counter which is used iterate over
> the ports, so that sched_tbl will be freed for all the ports.
>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] cxgb4: Fix pause frame count in t4_get_port_stats
From: David Miller @ 2017-09-05 21:28 UTC (permalink / raw)
To: ganeshgr; +Cc: netdev, nirranjan, indranil, venkatesh, leedom
In-Reply-To: <1504504056-21956-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Mon, 4 Sep 2017 11:17:36 +0530
> MPS_STAT_CTL[CountPauseStatTx] and MPS_STAT_CTL[CountPauseStatRx]
> only control whether or not Pause Frames will be counted as part
> of the 64-Byte Tx/Rx Frame counters. These bits do not control
> whether Pause Frames are counted in the Total Tx/Rx Frames/Bytes
> counters.
>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
> Signed-off-by: Casey Leedom <leedom@chelsio.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] cxgb4: Ignore MPS_TX_INT_CAUSE[Bubble] for T6
From: David Miller @ 2017-09-05 21:28 UTC (permalink / raw)
To: ganeshgr; +Cc: netdev, nirranjan, indranil, venkatesh, leedom
In-Reply-To: <1504504534-22115-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Mon, 4 Sep 2017 11:25:34 +0530
> MPS_TX_INT_CAUSE[Bubble] is a normal condition for T6, hence
> ignore this interrupt for T6.
>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
> Signed-off-by: Casey Leedom <leedom@chelsio.com>
Applied.
^ permalink raw reply
* Re: [net v2] gianfar: Fix Tx flow control deactivation
From: David Miller @ 2017-09-05 21:35 UTC (permalink / raw)
To: claudiu.manoil; +Cc: netdev
In-Reply-To: <1504511128-25294-1-git-send-email-claudiu.manoil@nxp.com>
From: Claudiu Manoil <claudiu.manoil@nxp.com>
Date: Mon, 4 Sep 2017 10:45:28 +0300
> The wrong register is checked for the Tx flow control bit,
> it should have been maccfg1 not maccfg2.
> This went unnoticed for so long probably because the impact is
> hardly visible, not to mention the tangled code from adjust_link().
> First, link flow control (i.e. handling of Rx/Tx link level pause frames)
> is disabled by default (needs to be enabled via 'ethtool -A').
> Secondly, maccfg2 always returns 0 for tx_flow_oldval (except for a few
> old boards), which results in Tx flow control remaining always on
> once activated.
>
> Fixes: 45b679c9a3ccd9e34f28e6ec677b812a860eb8eb ("gianfar: Implement PAUSE frame generation support")
>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
Please do not put an empty line between Fixes: and other tags. All tags
are equal and should be in one uninterrupted block of text.
Applied, and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net-next v7] net: stmmac: Delete dead code for MDIO registration
From: David Miller @ 2017-09-05 21:36 UTC (permalink / raw)
To: romain.perier
Cc: peppe.cavallaro, alexandre.torgue, andrew, f.fainelli, netdev,
linux-kernel
In-Reply-To: <20170904084136.21490-1-romain.perier@collabora.com>
From: Romain Perier <romain.perier@collabora.com>
Date: Mon, 4 Sep 2017 10:41:36 +0200
> This code is no longer used, the logging function was changed by commit
> fbca164776e4 ("net: stmmac: Use the right logging function in stmmac_mdio_register").
> It was previously showing information about the type of the IRQ, if it's
> polled, ignored or a normal interrupt. As we don't want information loss,
> I have moved this code to phy_attached_print().
>
> Fixes: fbca164776e4 ("net: stmmac: Use the right logging function in stmmac_mdio_register")
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
Applied.
^ permalink raw reply
* Re: [PATCH v3 net-next 0/2] Use correct sk->sk_prot for IPV6
From: David Miller @ 2017-09-05 21:38 UTC (permalink / raw)
To: ilyal; +Cc: netdev, davejwatson, aviadye
In-Reply-To: <1504520041-55634-1-git-send-email-ilyal@mellanox.com>
From: Ilya Lesokhin <ilyal@mellanox.com>
Date: Mon, 4 Sep 2017 13:13:59 +0300
> The tls ulp overrides sk->prot with a new tls specific proto structs.
> The tls specific structs were previously based on the ipv4 specific
> tcp_prot sturct.
> As a result, attaching the tls ulp to an ipv6 tcp socket replaced
> some ipv6 callback with the ipv4 equivalents.
>
> This patch adds ipv6 tls proto structs and uses them when
> attached to ipv6 sockets.
>
> Changed since v2:
> - Dropped patch to fix IPV6_ADDRFORM setsockopt
> There was some disagreement about the correct way of fixinig it,
> and this series does not depend on it.
>
> Changes since v1:
> - TLS now dependes on IPV6
> This fixes complication issues when TLS is built-in and IPV6 is a module.
> The downside should be small as it is unlikely that there are kernel TLS
> users who can't afford to include IPV6 in thier kernel.
This is not acceptable.
Forcing such a huge piece of infrastructure like ipv6 to be statically
built just because someone wants to enable TLS is not going to pass.
Every distrubtion out there will enable all features, including TLS,
so effectively you are forcing every distribution to build ipv6
non-modules.
Sorry, you will have to fix this in a way that allows TLS and IPV6
to be modular.
^ permalink raw reply
* Re: [PATCH net-next] rxrpc: Make service connection lookup always check for retry
From: David Miller @ 2017-09-05 21:41 UTC (permalink / raw)
To: dhowells; +Cc: netdev, linux-afs, linux-kernel
In-Reply-To: <150453530801.20359.17059994007095255872.stgit@warthog.procyon.org.uk>
From: David Howells <dhowells@redhat.com>
Date: Mon, 04 Sep 2017 15:28:28 +0100
> When an RxRPC service packet comes in, the target connection is looked up
> by an rb-tree search under RCU and a read-locked seqlock; the seqlock retry
> check is, however, currently skipped if we got a match, but probably
> shouldn't be in case the connection we found gets replaced whilst we're
> doing a search.
>
> Make the lookup procedure always go through need_seqretry(), even if the
> lookup was successful. This makes sure we always pick up on a write-lock
> event.
>
> On the other hand, since we don't take a ref on the object, but rely on RCU
> to prevent its destruction after dropping the seqlock, I'm not sure this is
> necessary.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
Applied, thanks David.
^ permalink raw reply
* Re: [PATCH net V2] vhost_net: correctly check tx avail during rx busy polling
From: David Miller @ 2017-09-05 21:48 UTC (permalink / raw)
To: jasowang; +Cc: mst, netdev, kvm, virtualization, linux-kernel
In-Reply-To: <1504574525-4711-1-git-send-email-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Tue, 5 Sep 2017 09:22:05 +0800
> We check tx avail through vhost_enable_notify() in the past which is
> wrong since it only checks whether or not guest has filled more
> available buffer since last avail idx synchronization which was just
> done by vhost_vq_avail_empty() before. What we really want is checking
> pending buffers in the avail ring. Fix this by calling
> vhost_vq_avail_empty() instead.
>
> This issue could be noticed by doing netperf TCP_RR benchmark as
> client from guest (but not host). With this fix, TCP_RR from guest to
> localhost restores from 1375.91 trans per sec to 55235.28 trans per
> sec on my laptop (Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz).
>
> Fixes: 030881372460 ("vhost_net: basic polling support")
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net v2] net: sched: don't use GFP_KERNEL under spin lock
From: David Miller @ 2017-09-05 21:49 UTC (permalink / raw)
To: jakub.kicinski
Cc: netdev, eric.dumazet, jiri, chrism, xiyou.wangcong, jhs,
oss-drivers
In-Reply-To: <20170905153123.13209-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Tue, 5 Sep 2017 08:31:23 -0700
> The new TC IDR code uses GFP_KERNEL under spin lock. Which leads
> to:
...
> Try to preallocate the memory with idr_prealloc(GFP_KERNEL)
> (as suggested by Eric Dumazet), and change the allocation
> flags under spin lock.
>
> Fixes: 65a206c01e8e ("net/sched: Change act_api and act_xxx modules to use IDR")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net] rds: Fix non-atomic operation on shared flag variable
From: David Miller @ 2017-09-05 21:50 UTC (permalink / raw)
To: Haakon.Bugge-QHcLZuEGTsvQT0dZR+AlfA
Cc: santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
knut.omang-QHcLZuEGTsvQT0dZR+AlfA,
wei.lin.guay-QHcLZuEGTsvQT0dZR+AlfA
In-Reply-To: <20170905154201.30198-1-Haakon.Bugge-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
From: Håkon Bugge <Haakon.Bugge-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Date: Tue, 5 Sep 2017 17:42:01 +0200
> The bits in m_flags in struct rds_message are used for a plurality of
> reasons, and from different contexts. To avoid any missing updates to
> m_flags, use the atomic set_bit() instead of the non-atomic equivalent.
>
> Signed-off-by: Håkon Bugge <haakon.bugge-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Wei Lin Guay <wei.lin.guay-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH][next v2] rocker: fix kcalloc parameter order
From: David Miller @ 2017-09-05 21:57 UTC (permalink / raw)
To: zahari.doychev; +Cc: jiri, netdev
In-Reply-To: <20170905194958.3436-1-zahari.doychev@linux.com>
From: Zahari Doychev <zahari.doychev@linux.com>
Date: Tue, 5 Sep 2017 21:49:58 +0200
> The function calls to kcalloc use wrong parameter order and incorrect flags
> values. GFP_KERNEL is used instead of flags now and the order is corrected.
>
> The change was done using the following coccinelle script:
>
> @@
> expression E1,E2;
> type T;
> @@
>
> -kcalloc(E1, E2, sizeof(T))
> +kcalloc(E2, sizeof(T), GFP_KERNEL)
>
> Signed-off-by: Zahari Doychev <zahari.doychev@linux.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH 0/3] drivers: net: xgene: Misc bug fixes
From: David Miller @ 2017-09-05 21:58 UTC (permalink / raw)
To: isubramanian; +Cc: netdev, dnelson, patches, linux-arm-kernel
In-Reply-To: <1504635392-19089-1-git-send-email-isubramanian@apm.com>
From: Iyappan Subramanian <isubramanian@apm.com>
Date: Tue, 5 Sep 2017 11:16:29 -0700
> This patch set fixes bugs related to handling the case for ACPI for,
> reading and programming tx/rx delay values.
>
> Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map
From: Chenbo Feng @ 2017-09-05 21:59 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Chenbo Feng, Daniel Borkmann, linux-security-module,
Jeffrey Vander Stoep, netdev, SELinux, Lorenzo Colitti
In-Reply-To: <20170901020520.uifv6b7tvelgxumf@ast-mbp>
On Thu, Aug 31, 2017 at 7:05 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Thu, Aug 31, 2017 at 01:56:34PM -0700, Chenbo Feng wrote:
>> From: Chenbo Feng <fengc@google.com>
>>
>> Introduce a pointer into struct bpf_map to hold the security information
>> about the map. The actual security struct varies based on the security
>> models implemented. Place the LSM hooks before each of the unrestricted
>> eBPF operations, the map_update_elem and map_delete_elem operations are
>> checked by security_map_modify. The map_lookup_elem and map_get_next_key
>> operations are checked by securtiy_map_read.
>>
>> Signed-off-by: Chenbo Feng <fengc@google.com>
>
> ...
>
>> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
>> if (IS_ERR(map))
>> return PTR_ERR(map);
>>
>> + err = security_map_read(map);
>> + if (err)
>> + return -EACCES;
>> +
>> key = memdup_user(ukey, map->key_size);
>> if (IS_ERR(key)) {
>> err = PTR_ERR(key);
>> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
>> if (IS_ERR(map))
>> return PTR_ERR(map);
>>
>> + err = security_map_modify(map);
>
> I don't feel these extra hooks are really thought through.
> With such hook you'll disallow map_update for given map. That's it.
> The key/values etc won't be used in such security decision.
> In such case you don't need such hooks in update/lookup at all.
> Only in map_creation and object_get calls where FD can be received.
> In other words I suggest to follow standard unix practices:
> Do permissions checks in open() and allow read/write() if FD is valid.
> Same here. Do permission checks in prog_load/map_create/obj_pin/get
> and that will be enough to jail bpf subsystem.
> bpf cmds that need to be fast (like lookup and update) should not
> have security hooks.
>
Thanks for pointing out this. I agree we should only do checks on
creating and passing
the object from one processes to another. And if we can still
distinguish the read/write operation
when obtaining the file fd, that would be great. But that may require
us to add a new mode
field into bpf_map struct and change the attribute struct when doing
the bpf syscall. How do you
think about this approach? Or we can do simple checks for
bpf_obj_create and bpf_obj_use when
creating the object and passing the object respectively but this
solution cannot distinguish map modify and
read.
^ permalink raw reply
* Re: [RFC net-next] net: sch_clsact: add support for global per-netns classifier mode
From: Roopa Prabhu @ 2017-09-05 22:01 UTC (permalink / raw)
To: Cong Wang
Cc: Nikolay Aleksandrov, Linux Kernel Network Developers, David Ahern,
Jiri Pirko, Jamal Hadi Salim
In-Reply-To: <CAM_iQpUYGH4BQ-ZUShbRAy6aJD2fYuM+oc_kWYyQ51n+===CQw@mail.gmail.com>
On Tue, Sep 5, 2017 at 11:18 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Tue, Sep 5, 2017 at 5:48 AM, Nikolay Aleksandrov
> <nikolay@cumulusnetworks.com> wrote:
>> Hi all,
>> This RFC adds a new mode for clsact which designates a device's egress
>> classifier as global per netns. The packets that are not classified for
>> a particular device will be classified using the global classifier.
>> We have needed a global classifier for some time now for various
>> purposes and setting the single bridge or loopback/vrf device as the
>> global classifier device is acceptable for us. Doing it this way avoids
>> the act/cls device and queue dependencies.
>>
>> This is strictly an RFC patch just to show the intent, if we agree on
>> the details the proposed patch will have support for both ingress and
>> egress, and will be using a static key to avoid the fast path test when no
>> global classifier has been configured.
>>
>> Example (need a modified tc that adds TCA_OPTIONS when using q_clsact):
>> $ tc qdisc add dev lo clsact global
>> $ tc filter add dev lo egress protocol ip u32 match ip dst 4.3.2.1/32 action drop
>>
>> the last filter will be global for all devices that don't have a
>> specific egress_cl_list (i.e. have clsact configured).
>
> Sorry this is too ugly.
>
> netdevice is still implied in your command line even if you treat it
> as global. It is essentially hard to bypass netdevice layer since
> netdevice is the core of L2 and also where everything begins.
>
> Maybe the best we can do here is make tc filters standalone
> as tc actions so that filters can exist before qdisc's and netdevices.
> But this probably requires significant works to make it working
> with both existing non-standalone and bindings standalones
> with qdisc's.
yes, like Nikolay says we have been discussing this as well. Nikolay's
patch is a cleaver and most importantly non-invasive
way today given the anchor point for tc rules is a netdev. we have
also considered a separate implicit tc anchor device.
lo seemed like a good fit for all global rules given it is already
available. And it is not uncommon to hang off global
network config on the loopback interface.
^ permalink raw reply
* Re: [PATCH 1/3] security: bpf: Add eBPF LSM hooks to security module
From: Chenbo Feng @ 2017-09-05 22:24 UTC (permalink / raw)
To: Stephen Smalley
Cc: Chenbo Feng, linux-security-module, SELinux, netdev,
Alexei Starovoitov, Lorenzo Colitti
In-Reply-To: <1504270223.8240.2.camel@tycho.nsa.gov>
On Fri, Sep 1, 2017 at 5:50 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Thu, 2017-08-31 at 13:56 -0700, Chenbo Feng wrote:
>> From: Chenbo Feng <fengc@google.com>
>>
>> Introduce 5 LSM hooks to provide finer granularity controls on eBPF
>> related operations including create eBPF maps, modify and read eBPF
>> maps
>> content and load eBPF programs to the kernel. Hooks use the new
>> security
>> pointer inside the eBPF map struct to store the owner's security
>> information and the different security modules can perform different
>> checks based on the information stored inside the security field.
>>
>> Signed-off-by: Chenbo Feng <fengc@google.com>
>> ---
>> include/linux/lsm_hooks.h | 41
>> +++++++++++++++++++++++++++++++++++++++++
>> include/linux/security.h | 36 ++++++++++++++++++++++++++++++++++++
>> security/security.c | 28 ++++++++++++++++++++++++++++
>> 3 files changed, 105 insertions(+)
>>
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index ce02f76a6188..3aaf9a08a983 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -1353,6 +1353,32 @@
>> * @inode we wish to get the security context of.
>> * @ctx is a pointer in which to place the allocated security
>> context.
>> * @ctxlen points to the place to put the length of @ctx.
>> + *
>> + * Security hooks for using the eBPF maps and programs
>> functionalities through
>> + * eBPF syscalls.
>> + *
>> + * @bpf_map_create:
>> + * Check permissions prior to creating a new bpf map.
>> + * Return 0 if the permission is granted.
>> + *
>> + * @bpf_map_modify:
>> + * Check permission prior to insert, update and delete map
>> content.
>> + * @map pointer to the struct bpf_map that contains map
>> information.
>> + * Return 0 if the permission is granted.
>> + *
>> + * @bpf_map_read:
>> + * Check permission prior to read a bpf map content.
>> + * @map pointer to the struct bpf_map that contains map
>> information.
>> + * Return 0 if the permission is granted.
>> + *
>> + * @bpf_prog_load:
>> + * Check permission prior to load eBPF program.
>> + * Return 0 if the permission is granted.
>> + *
>> + * @bpf_post_create:
>> + * Initialize the bpf object security field inside struct
>> bpf_maps and
>> + * it is used for future security checks.
>> + *
>> */
>> union security_list_options {
>> int (*binder_set_context_mgr)(struct task_struct *mgr);
>> @@ -1685,6 +1711,14 @@ union security_list_options {
>> struct audit_context *actx);
>> void (*audit_rule_free)(void *lsmrule);
>> #endif /* CONFIG_AUDIT */
>> +
>> +#ifdef CONFIG_BPF_SYSCALL
>> + int (*bpf_map_create)(void);
>> + int (*bpf_map_read)(struct bpf_map *map);
>> + int (*bpf_map_modify)(struct bpf_map *map);
>> + int (*bpf_prog_load)(void);
>> + int (*bpf_post_create)(struct bpf_map *map);
>> +#endif /* CONFIG_BPF_SYSCALL */
>> };
>>
>> struct security_hook_heads {
>> @@ -1905,6 +1939,13 @@ struct security_hook_heads {
>> struct list_head audit_rule_match;
>> struct list_head audit_rule_free;
>> #endif /* CONFIG_AUDIT */
>> +#ifdef CONFIG_BPF_SYSCALL
>> + struct list_head bpf_map_create;
>> + struct list_head bpf_map_read;
>> + struct list_head bpf_map_modify;
>> + struct list_head bpf_prog_load;
>> + struct list_head bpf_post_create;
>> +#endif /* CONFIG_BPF_SYSCALL */
>> } __randomize_layout;
>>
>> /*
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 458e24bea2d4..0656a4f74d14 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -31,6 +31,7 @@
>> #include <linux/string.h>
>> #include <linux/mm.h>
>> #include <linux/fs.h>
>> +#include <linux/bpf.h>
>>
>> struct linux_binprm;
>> struct cred;
>> @@ -1735,6 +1736,41 @@ static inline void securityfs_remove(struct
>> dentry *dentry)
>>
>> #endif
>>
>> +#ifdef CONFIG_BPF_SYSCALL
>> +#ifdef CONFIG_SECURITY
>> +int security_map_create(void);
>> +int security_map_modify(struct bpf_map *map);
>> +int security_map_read(struct bpf_map *map);
>> +int security_prog_load(void);
>> +int security_post_create(struct bpf_map *map);
>> +#else
>> +static inline int security_map_create(void)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline int security_map_read(struct bpf_map *map)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline int security_map_modify(struct bpf_map *map)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline int security_prog_load(void)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline int security_post_create(struct bpf_map *map)
>> +{
>> + return 0;
>> +}
>> +#endif /* CONFIG_SECURITY */
>> +#endif /* CONFIG_BPF_SYSCALL */
>
> These should be named consistently with the ones in lsm_hooks.h and
> should unambiguously indicate that these are hooks for bpf
> objects/operations, i.e. security_bpf_map_create(),
> security_bpf_map_read(), etc.
>
Thanks for pointing out, will fix this.
> Do you need this level of granularity?
>
The cover letter of this patch series described a possible use cases of
these lsm hooks and this level of granularity would be ideal to reach that
goal. We can also implement two hooks such as bpf_obj_create and
bpf_obj_use to restrict the creation and using when get the bpf fd from
kernel. But that will be less powerful and flexible.
> Could you coalesce the map_create() and post_map_create() hooks into
> one hook and just unwind the create in that case?
>
Okay, I will take a look on how to fix this.
> Why do you label bpf maps but not bpf progs? Should we be controlling
> the ability to attach/detach a bpf prog (partly controlled by
> CAP_NET_ADMIN, but also somewhat broad in scope and doesn't allow
> control based on who created the prog)?
>
> Should there be a top-level security_bpf_use() hook and permission
> check that limits ability to use bpf() at all?
>
This could be useful but having additional lsm hooks check when reading
and write to eBPF maps may cause performance issue. Instead maybe we
could have a hook for creating eBPF object and retrieve object fd to restrict
the access.
>> +
>> #ifdef CONFIG_SECURITY
>>
>> static inline char *alloc_secdata(void)
>> diff --git a/security/security.c b/security/security.c
>> index 55b5997e4b72..02272f93a89e 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -12,6 +12,7 @@
>> * (at your option) any later version.
>> */
>>
>> +#include <linux/bpf.h>
>> #include <linux/capability.h>
>> #include <linux/dcache.h>
>> #include <linux/module.h>
>> @@ -1708,3 +1709,30 @@ int security_audit_rule_match(u32 secid, u32
>> field, u32 op, void *lsmrule,
>> actx);
>> }
>> #endif /* CONFIG_AUDIT */
>> +
>> +#ifdef CONFIG_BPF_SYSCALL
>> +int security_map_create(void)
>> +{
>> + return call_int_hook(bpf_map_create, 0);
>> +}
>> +
>> +int security_map_modify(struct bpf_map *map)
>> +{
>> + return call_int_hook(bpf_map_modify, 0, map);
>> +}
>> +
>> +int security_map_read(struct bpf_map *map)
>> +{
>> + return call_int_hook(bpf_map_read, 0, map);
>> +}
>> +
>> +int security_prog_load(void)
>> +{
>> + return call_int_hook(bpf_prog_load, 0);
>> +}
>> +
>> +int security_post_create(struct bpf_map *map)
>> +{
>> + return call_int_hook(bpf_post_create, 0, map);
>> +}
>> +#endif /* CONFIG_BPF_SYSCALL */
^ permalink raw reply
* Re: [RFC net-next] net: sch_clsact: add support for global per-netns classifier mode
From: Jamal Hadi Salim @ 2017-09-05 22:25 UTC (permalink / raw)
To: Roopa Prabhu, Cong Wang
Cc: Nikolay Aleksandrov, Linux Kernel Network Developers, David Ahern,
Jiri Pirko
In-Reply-To: <CAJieiUj_-o-Cg2AzN5b0vrdnBD69ryMmpu4fcC64-MHEmQ-u4g@mail.gmail.com>
On 17-09-05 06:01 PM, Roopa Prabhu wrote:
>
> yes, like Nikolay says we have been discussing this as well. Nikolay's
> patch is a cleaver and most importantly non-invasive
> way today given the anchor point for tc rules is a netdev. we have
> also considered a separate implicit tc anchor device.
> lo seemed like a good fit for all global rules given it is already
> available. And it is not uncommon to hang off global
> network config on the loopback interface.
>
IMO, Jiri has done all the necessary work already with the concept of
blocks. We dont really need the netdev to be the attachment point.
You can add to a block in many locations in the kernel by
constructing the proper "coordinates" in the tcmsg.
i.e this:
tcmsg {
unsigned char tcm_family;
unsigned char tcm__pad1;
unsigned short tcm__pad2;
int tcm_ifindex;
__u32 tcm_handle;
__u32 tcm_parent;
}
If you were to set tcm_ifindex to -1 (since that is not a legit
ifindex) then all we need to do is define a parent for a
different location. Current locations tied to netdevs are:
-----
#define TC_H_ROOT (0xFFFFFFFFU)
#define TC_H_INGRESS (0xFFFFFFF1U)
#define TC_H_CLSACT TC_H_INGRESS
#define TC_H_MIN_INGRESS 0xFFF2U
#define TC_H_MIN_EGRESS 0xFFF3U
-----
You should be able to say add a location which maps to a pre-routing
or post-routing etc; and this would work as well...
cheers,
jamal
^ permalink raw reply
* Re: [PATCH net-next] bpf: fix numa_node validation
From: Martin KaFai Lau @ 2017-09-05 22:30 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <1504590062.15310.36.camel@edumazet-glaptop3.roam.corp.google.com>
On Mon, Sep 04, 2017 at 10:41:02PM -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> syzkaller reported crashes in bpf map creation or map update [1]
>
> Problem is that nr_node_ids is a signed integer,
> NUMA_NO_NODE is also an integer, so it is very tempting
> to declare numa_node as a signed integer.
>
> This means the typical test to validate a user provided value :
>
> if (numa_node != NUMA_NO_NODE &&
> (numa_node >= nr_node_ids ||
> !node_online(numa_node)))
>
> must be written :
>
> if (numa_node != NUMA_NO_NODE &&
> ((unsigned int)numa_node >= nr_node_ids ||
Thanks for fixing it.
> !node_online(numa_node)))
>
>
> [1]
> kernel BUG at mm/slab.c:3256!
> invalid opcode: 0000 [#1] SMP KASAN
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 0 PID: 2946 Comm: syzkaller916108 Not tainted 4.13.0-rc7+ #35
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> task: ffff8801d2bc60c0 task.stack: ffff8801c0c90000
> RIP: 0010:____cache_alloc_node+0x1d4/0x1e0 mm/slab.c:3292
> RSP: 0018:ffff8801c0c97638 EFLAGS: 00010096
> RAX: ffffffffffff8b7b RBX: 0000000001080220 RCX: 0000000000000000
> RDX: 00000000ffff8b7b RSI: 0000000001080220 RDI: ffff8801dac00040
> RBP: ffff8801c0c976c0 R08: 0000000000000000 R09: 0000000000000000
> R10: ffff8801c0c97620 R11: 0000000000000001 R12: ffff8801dac00040
> R13: ffff8801dac00040 R14: 0000000000000000 R15: 00000000ffff8b7b
> FS: 0000000002119940(0000) GS:ffff8801db200000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000020001fec CR3: 00000001d2980000 CR4: 00000000001406f0
> Call Trace:
> __do_kmalloc_node mm/slab.c:3688 [inline]
> __kmalloc_node+0x33/0x70 mm/slab.c:3696
> kmalloc_node include/linux/slab.h:535 [inline]
> alloc_htab_elem+0x2a8/0x480 kernel/bpf/hashtab.c:740
> htab_map_update_elem+0x740/0xb80 kernel/bpf/hashtab.c:820
> map_update_elem kernel/bpf/syscall.c:587 [inline]
> SYSC_bpf kernel/bpf/syscall.c:1468 [inline]
> SyS_bpf+0x20c5/0x4c40 kernel/bpf/syscall.c:1443
> entry_SYSCALL_64_fastpath+0x1f/0xbe
> RIP: 0033:0x440409
> RSP: 002b:00007ffd1f1792b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
> RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440409
> RDX: 0000000000000020 RSI: 0000000020006000 RDI: 0000000000000002
> RBP: 0000000000000086 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000401d70
> R13: 0000000000401e00 R14: 0000000000000000 R15: 0000000000000000
> Code: 83 c2 01 89 50 18 4c 03 70 08 e8 38 f4 ff ff 4d 85 f6 0f 85 3e ff ff ff 44 89 fe 4c 89 ef e8 94 fb ff ff 49 89 c6 e9 2b ff ff ff <0f> 0b 0f 0b 0f 0b 66 0f 1f 44 00 00 55 48 89 e5 41 57 41 56 41
> RIP: ____cache_alloc_node+0x1d4/0x1e0 mm/slab.c:3292 RSP: ffff8801c0c97638
> ---[ end trace d745f355da2e33ce ]---
> Kernel panic - not syncing: Fatal exception
>
> Fixes: 96eabe7a40aa ("bpf: Allow selecting numa node during map creation")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Martin KaFai Lau <kafai@fb.com>
> Cc: Alexei Starovoitov <ast@fb.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> ---
> kernel/bpf/syscall.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 021a05d9d80095303bdfed51ee85bd9067582774..70ad8e220343c7825c8e331f19c1f65c78fdb796 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -323,7 +323,8 @@ static int map_create(union bpf_attr *attr)
> return -EINVAL;
>
> if (numa_node != NUMA_NO_NODE &&
> - (numa_node >= nr_node_ids || !node_online(numa_node)))
> + ((unsigned int)numa_node >= nr_node_ids ||
> + !node_online(numa_node)))
> return -EINVAL;
>
> /* find map type and init map: hashtable vs rbtree vs bloom vs ... */
>
>
^ permalink raw reply
* [PATCH net-next RFC 0/2] Improve code coverage of syzkaller
From: Petar Penkov @ 2017-09-05 22:35 UTC (permalink / raw)
To: netdev; +Cc: Petar Penkov
This patch series is intended to improve code coverage of syzkaller on
the early receive path, specifically including flow dissector, GRO,
and GRO with frags parts of the networking stack. Syzkaller exercises
the stack through the TUN driver and this is therefore where changes
reside. Current coverage through netif_receive_skb() is limited as it
does not touch on any of the aforementioned code paths. Furthermore,
for full coverage, it is necessary to have more flexibility over the
linear and non-linear data of the skbs.
The following patches address this by providing the user(syzkaller)
with the ability to send via napi_gro_receive() and napi_gro_frags().
Additionally, syzkaller can specify how many fragments there are and
how much data per fragment there is. This is done by exploiting the
convenient structure of iovecs. Finally, this patch series adds
support for exercising the flow dissector during fuzzing.
The code path including napi_gro_receive() can be enabled via the
CONFIG_TUN_NAPI compile-time flag, and can be used by users other than
syzkaller. The remainder of the changes in this patch series give the
user significantly more control over packets entering the kernel. To
avoid potential security vulnerabilities, hide the ability to send
custom skbs and the flow dissector code paths behind a run-time flag
IFF_NAPI_FRAGS that is advertised and accepted only if CONFIG_TUN_NAPI
is enabled.
The patch series will be followed with changes to packetdrill, where
these additions to the TUN driver are exercised and demonstrated.
This will give the ability to write regression tests for specific
parts of the early networking stack.
Patch 1/ Add NAPI struct per receive queue, enable NAPI, and use
napi_gro_receive()
Patch 2/ Use NAPI skb and napi_gro_frags(), exercise flow dissector,
and allow custom skbs.
Petar Penkov (2):
tun: enable NAPI for TUN/TAP driver
tun: enable napi_gro_frags() for TUN/TAP driver
drivers/net/Kconfig | 8 ++
drivers/net/tun.c | 251 +++++++++++++++++++++++++++++++++++++++++---
include/uapi/linux/if_tun.h | 1 +
3 files changed, 246 insertions(+), 14 deletions(-)
--
2.14.1.581.gf28d330327-goog
^ permalink raw reply
* [PATCH net-next RFC 1/2] tun: enable NAPI for TUN/TAP driver
From: Petar Penkov @ 2017-09-05 22:35 UTC (permalink / raw)
To: netdev
Cc: Petar Penkov, Eric Dumazet, Mahesh Bandewar, Willem de Bruijn,
davem, ppenkov
In-Reply-To: <20170905223551.27925-1-ppenkov@google.com>
Changes TUN driver to use napi_gro_receive() upon receiving packets
rather than netif_rx_ni(). Adds flag CONFIG_TUN_NAPI that enables
these changes and operation is not affected if the flag is disabled.
SKBs are constructed upon packet arrival and are queued to be
processed later.
The new path was evaluated with a benchmark with the following setup:
Open two tap devices and a receiver thread that reads in a loop for
each device. Start one sender thread and pin all threads to different
CPUs. Send 1M minimum UDP packets to each device and measure sending
time for each of the sending methods:
napi_gro_receive(): 4.90s
netif_rx_ni(): 4.90s
netif_receive_skb(): 7.20s
Signed-off-by: Petar Penkov <ppenkov@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Mahesh Bandewar <maheshb@google.com>
Cc: Willem de Bruijn <willemb@google.com>
Cc: davem@davemloft.net
Cc: ppenkov@stanford.edu
---
drivers/net/Kconfig | 8 ++++
drivers/net/tun.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 118 insertions(+), 10 deletions(-)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 83a1616903f8..34850d71ddd1 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -307,6 +307,14 @@ config TAP
This option is selected by any driver implementing tap user space
interface for a virtual interface to re-use core tap functionality.
+config TUN_NAPI
+ bool "NAPI support on tx path for TUN/TAP driver"
+ default n
+ depends on TUN
+ ---help---
+ This option allows the TUN/TAP driver to use NAPI to pass packets to
+ the kernel when receiving packets from user space via write()/send().
+
config TUN_VNET_CROSS_LE
bool "Support for cross-endian vnet headers on little-endian kernels"
default n
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 06e8f0bb2dab..d5c824e3ec42 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -172,6 +172,7 @@ struct tun_file {
u16 queue_index;
unsigned int ifindex;
};
+ struct napi_struct napi;
struct list_head next;
struct tun_struct *detached;
struct skb_array tx_array;
@@ -229,6 +230,67 @@ struct tun_struct {
struct bpf_prog __rcu *xdp_prog;
};
+static int tun_napi_receive(struct napi_struct *napi, int budget)
+{
+ struct tun_file *tfile = container_of(napi, struct tun_file, napi);
+ struct sk_buff_head *queue = &tfile->sk.sk_write_queue;
+ struct sk_buff_head process_queue;
+ struct sk_buff *skb;
+ int received = 0;
+
+ __skb_queue_head_init(&process_queue);
+
+ spin_lock(&queue->lock);
+ skb_queue_splice_tail_init(queue, &process_queue);
+ spin_unlock(&queue->lock);
+
+ while (received < budget && (skb = __skb_dequeue(&process_queue))) {
+ napi_gro_receive(napi, skb);
+ ++received;
+ }
+
+ if (!skb_queue_empty(&process_queue)) {
+ spin_lock(&queue->lock);
+ skb_queue_splice(&process_queue, queue);
+ spin_unlock(&queue->lock);
+ }
+
+ return received;
+}
+
+static int tun_napi_poll(struct napi_struct *napi, int budget)
+{
+ unsigned int received;
+
+ received = tun_napi_receive(napi, budget);
+
+ if (received < budget)
+ napi_complete_done(napi, received);
+
+ return received;
+}
+
+static void tun_napi_init(struct tun_struct *tun, struct tun_file *tfile)
+{
+ if (IS_ENABLED(CONFIG_TUN_NAPI)) {
+ netif_napi_add(tun->dev, &tfile->napi, tun_napi_poll,
+ NAPI_POLL_WEIGHT);
+ napi_enable(&tfile->napi);
+ }
+}
+
+static void tun_napi_disable(struct tun_file *tfile)
+{
+ if (IS_ENABLED(CONFIG_TUN_NAPI))
+ napi_disable(&tfile->napi);
+}
+
+static void tun_napi_del(struct tun_file *tfile)
+{
+ if (IS_ENABLED(CONFIG_TUN_NAPI))
+ netif_napi_del(&tfile->napi);
+}
+
#ifdef CONFIG_TUN_VNET_CROSS_LE
static inline bool tun_legacy_is_little_endian(struct tun_struct *tun)
{
@@ -541,6 +603,11 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
tun = rtnl_dereference(tfile->tun);
+ if (tun && clean) {
+ tun_napi_disable(tfile);
+ tun_napi_del(tfile);
+ }
+
if (tun && !tfile->detached) {
u16 index = tfile->queue_index;
BUG_ON(index >= tun->numqueues);
@@ -598,6 +665,7 @@ static void tun_detach_all(struct net_device *dev)
for (i = 0; i < n; i++) {
tfile = rtnl_dereference(tun->tfiles[i]);
BUG_ON(!tfile);
+ tun_napi_disable(tfile);
tfile->socket.sk->sk_shutdown = RCV_SHUTDOWN;
tfile->socket.sk->sk_data_ready(tfile->socket.sk);
RCU_INIT_POINTER(tfile->tun, NULL);
@@ -613,6 +681,7 @@ static void tun_detach_all(struct net_device *dev)
synchronize_net();
for (i = 0; i < n; i++) {
tfile = rtnl_dereference(tun->tfiles[i]);
+ tun_napi_del(tfile);
/* Drop read queue */
tun_queue_purge(tfile);
sock_put(&tfile->sk);
@@ -677,10 +746,12 @@ static int tun_attach(struct tun_struct *tun, struct file *file, bool skip_filte
rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile);
tun->numqueues++;
- if (tfile->detached)
+ if (tfile->detached) {
tun_enable_queue(tfile);
- else
+ } else {
sock_hold(&tfile->sk);
+ tun_napi_init(tun, tfile);
+ }
tun_set_real_num_queues(tun);
@@ -956,13 +1027,28 @@ static void tun_poll_controller(struct net_device *dev)
* Tun only receives frames when:
* 1) the char device endpoint gets data from user space
* 2) the tun socket gets a sendmsg call from user space
- * Since both of those are synchronous operations, we are guaranteed
- * never to have pending data when we poll for it
- * so there is nothing to do here but return.
+ * If NAPI is not enabled, since both of those are synchronous
+ * operations, we are guaranteed never to have pending data when we poll
+ * for it so there is nothing to do here but return.
* We need this though so netpoll recognizes us as an interface that
* supports polling, which enables bridge devices in virt setups to
* still use netconsole
+ * If NAPI is enabled, however, we need to schedule polling for all
+ * queues.
*/
+
+ if (IS_ENABLED(CONFIG_TUN_NAPI)) {
+ struct tun_struct *tun = netdev_priv(dev);
+ struct tun_file *tfile;
+ int i;
+
+ rcu_read_lock();
+ for (i = 0; i < tun->numqueues; i++) {
+ tfile = rcu_dereference(tun->tfiles[i]);
+ napi_schedule(&tfile->napi);
+ }
+ rcu_read_unlock();
+ }
return;
}
#endif
@@ -1535,11 +1621,25 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
}
rxhash = __skb_get_hash_symmetric(skb);
-#ifndef CONFIG_4KSTACKS
- tun_rx_batched(tun, tfile, skb, more);
-#else
- netif_rx_ni(skb);
-#endif
+
+ if (IS_ENABLED(CONFIG_TUN_NAPI)) {
+ struct sk_buff_head *queue = &tfile->sk.sk_write_queue;
+ int queue_len;
+
+ spin_lock_bh(&queue->lock);
+ __skb_queue_tail(queue, skb);
+ queue_len = skb_queue_len(queue);
+ spin_unlock(&queue->lock);
+
+ if (!more || queue_len > NAPI_POLL_WEIGHT)
+ napi_schedule(&tfile->napi);
+
+ local_bh_enable();
+ } else if (!IS_ENABLED(CONFIG_4KSTACKS)) {
+ tun_rx_batched(tun, tfile, skb, more);
+ } else {
+ netif_rx_ni(skb);
+ }
stats = get_cpu_ptr(tun->pcpu_stats);
u64_stats_update_begin(&stats->syncp);
--
2.14.1.581.gf28d330327-goog
^ permalink raw reply related
* [PATCH net-next RFC 2/2] tun: enable napi_gro_frags() for TUN/TAP driver
From: Petar Penkov @ 2017-09-05 22:35 UTC (permalink / raw)
To: netdev
Cc: Petar Penkov, Eric Dumazet, Mahesh Bandewar, Willem de Bruijn,
davem, ppenkov
In-Reply-To: <20170905223551.27925-1-ppenkov@google.com>
Add a TUN/TAP receive mode that exercises the napi_gro_frags()
interface. This mode is available only in TAP mode, as the interface
expects packets with Ethernet headers.
Furthermore, packets follow the layout of the iovec_iter that was
received. The first iovec is the linear data, and every one after the
first is a fragment. If there are more fragments than the max number,
drop the packet. Additionally, invoke eth_get_headlen() to exercise flow
dissector code and to verify that the header resides in the linear data.
The napi_gro_frags() mode requires setting the IFF_NAPI_FRAGS option.
This is imposed because this mode is intended for testing via tools like
syzkaller and packetdrill, and the increased flexibility it provides can
introduce security vulnerabilities.
Signed-off-by: Petar Penkov <ppenkov@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Mahesh Bandewar <maheshb@google.com>
Cc: Willem de Bruijn <willemb@google.com>
Cc: davem@davemloft.net
Cc: ppenkov@stanford.edu
---
drivers/net/tun.c | 135 ++++++++++++++++++++++++++++++++++++++++++--
include/uapi/linux/if_tun.h | 1 +
2 files changed, 130 insertions(+), 6 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index d5c824e3ec42..2ba9809ab6cd 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -75,6 +75,7 @@
#include <linux/skb_array.h>
#include <linux/bpf.h>
#include <linux/bpf_trace.h>
+#include <linux/mutex.h>
#include <linux/uaccess.h>
@@ -120,8 +121,15 @@ do { \
#define TUN_VNET_LE 0x80000000
#define TUN_VNET_BE 0x40000000
+#if IS_ENABLED(CONFIG_TUN_NAPI)
+#define TUN_FEATURES_EXTRA IFF_NAPI_FRAGS
+#else
+#define TUN_FEATURES_EXTRA 0
+#endif
+
#define TUN_FEATURES (IFF_NO_PI | IFF_ONE_QUEUE | IFF_VNET_HDR | \
- IFF_MULTI_QUEUE)
+ IFF_MULTI_QUEUE | TUN_FEATURES_EXTRA)
+
#define GOODCOPY_LEN 128
#define FLT_EXACT_COUNT 8
@@ -173,6 +181,7 @@ struct tun_file {
unsigned int ifindex;
};
struct napi_struct napi;
+ struct mutex napi_mutex; /* Protects access to the above napi */
struct list_head next;
struct tun_struct *detached;
struct skb_array tx_array;
@@ -276,6 +285,7 @@ static void tun_napi_init(struct tun_struct *tun, struct tun_file *tfile)
netif_napi_add(tun->dev, &tfile->napi, tun_napi_poll,
NAPI_POLL_WEIGHT);
napi_enable(&tfile->napi);
+ mutex_init(&tfile->napi_mutex);
}
}
@@ -291,6 +301,11 @@ static void tun_napi_del(struct tun_file *tfile)
netif_napi_del(&tfile->napi);
}
+static bool tun_napi_frags_enabled(const struct tun_struct *tun)
+{
+ return READ_ONCE(tun->flags) & IFF_NAPI_FRAGS;
+}
+
#ifdef CONFIG_TUN_VNET_CROSS_LE
static inline bool tun_legacy_is_little_endian(struct tun_struct *tun)
{
@@ -1034,7 +1049,8 @@ static void tun_poll_controller(struct net_device *dev)
* supports polling, which enables bridge devices in virt setups to
* still use netconsole
* If NAPI is enabled, however, we need to schedule polling for all
- * queues.
+ * queues unless we are using napi_gro_frags(), which we call in
+ * process context and not in NAPI context.
*/
if (IS_ENABLED(CONFIG_TUN_NAPI)) {
@@ -1042,6 +1058,9 @@ static void tun_poll_controller(struct net_device *dev)
struct tun_file *tfile;
int i;
+ if (tun_napi_frags_enabled(tun))
+ return;
+
rcu_read_lock();
for (i = 0; i < tun->numqueues; i++) {
tfile = rcu_dereference(tun->tfiles[i]);
@@ -1264,6 +1283,64 @@ static unsigned int tun_chr_poll(struct file *file, poll_table *wait)
return mask;
}
+static struct sk_buff *tun_napi_alloc_frags(struct tun_file *tfile,
+ size_t len,
+ const struct iov_iter *it)
+{
+ struct sk_buff *skb;
+ size_t linear;
+ int err;
+ int i;
+
+ if (it->nr_segs > MAX_SKB_FRAGS + 1)
+ return ERR_PTR(-ENOMEM);
+
+ local_bh_disable();
+ skb = napi_get_frags(&tfile->napi);
+ local_bh_enable();
+ if (!skb)
+ return ERR_PTR(-ENOMEM);
+
+ linear = iov_iter_single_seg_count(it);
+ err = __skb_grow(skb, linear);
+ if (err)
+ goto free;
+
+ skb->len = len;
+ skb->data_len = len - linear;
+ skb->truesize += skb->data_len;
+
+ for (i = 1; i < it->nr_segs; i++) {
+ size_t fragsz = it->iov[i].iov_len;
+ unsigned long offset;
+ struct page *page;
+ void *data;
+
+ if (fragsz == 0 || fragsz > PAGE_SIZE) {
+ err = -EINVAL;
+ goto free;
+ }
+
+ local_bh_disable();
+ data = napi_alloc_frag(fragsz);
+ local_bh_enable();
+ if (!data) {
+ err = -ENOMEM;
+ goto free;
+ }
+
+ page = virt_to_page(data);
+ offset = offset_in_page(data);
+ skb_fill_page_desc(skb, i - 1, page, offset, fragsz);
+ }
+
+ return skb;
+free:
+ /* frees skb and all frags allocated with napi_alloc_frag() */
+ napi_free_frags(&tfile->napi);
+ return ERR_PTR(err);
+}
+
/* prepad is the amount to reserve at front. len is length after that.
* linear is a hint as to how much to copy (usually headers). */
static struct sk_buff *tun_alloc_skb(struct tun_file *tfile,
@@ -1466,6 +1543,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
int err;
u32 rxhash;
int generic_xdp = 1;
+ bool frags = tun_napi_frags_enabled(tun);
if (!(tun->dev->flags & IFF_UP))
return -EIO;
@@ -1523,7 +1601,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
zerocopy = true;
}
- if (tun_can_build_skb(tun, tfile, len, noblock, zerocopy)) {
+ if (!frags && tun_can_build_skb(tun, tfile, len, noblock, zerocopy)) {
skb = tun_build_skb(tun, tfile, from, &gso, len, &generic_xdp);
if (IS_ERR(skb)) {
this_cpu_inc(tun->pcpu_stats->rx_dropped);
@@ -1540,10 +1618,24 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
linear = tun16_to_cpu(tun, gso.hdr_len);
}
- skb = tun_alloc_skb(tfile, align, copylen, linear, noblock);
+ if (frags) {
+ mutex_lock(&tfile->napi_mutex);
+ skb = tun_napi_alloc_frags(tfile, copylen, from);
+ /* tun_napi_alloc_frags() enforces a layout for the skb.
+ * If zerocopy is enabled, then this layout will be
+ * overwritten by zerocopy_sg_from_iter().
+ */
+ zerocopy = false;
+ } else {
+ skb = tun_alloc_skb(tfile, align, copylen, linear,
+ noblock);
+ }
+
if (IS_ERR(skb)) {
if (PTR_ERR(skb) != -EAGAIN)
this_cpu_inc(tun->pcpu_stats->rx_dropped);
+ if (frags)
+ mutex_unlock(&tfile->napi_mutex);
return PTR_ERR(skb);
}
@@ -1555,6 +1647,11 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
if (err) {
this_cpu_inc(tun->pcpu_stats->rx_dropped);
kfree_skb(skb);
+ if (frags) {
+ tfile->napi.skb = NULL;
+ mutex_unlock(&tfile->napi_mutex);
+ }
+
return -EFAULT;
}
}
@@ -1562,6 +1659,11 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
if (virtio_net_hdr_to_skb(skb, &gso, tun_is_little_endian(tun))) {
this_cpu_inc(tun->pcpu_stats->rx_frame_errors);
kfree_skb(skb);
+ if (frags) {
+ tfile->napi.skb = NULL;
+ mutex_unlock(&tfile->napi_mutex);
+ }
+
return -EINVAL;
}
@@ -1587,7 +1689,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
skb->dev = tun->dev;
break;
case IFF_TAP:
- skb->protocol = eth_type_trans(skb, tun->dev);
+ if (!frags)
+ skb->protocol = eth_type_trans(skb, tun->dev);
break;
}
@@ -1622,7 +1725,23 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
rxhash = __skb_get_hash_symmetric(skb);
- if (IS_ENABLED(CONFIG_TUN_NAPI)) {
+ if (frags) {
+ /* Exercise flow dissector code path. */
+ u32 headlen = eth_get_headlen(skb->data, skb_headlen(skb));
+
+ if (headlen > skb_headlen(skb) || headlen < ETH_HLEN) {
+ this_cpu_inc(tun->pcpu_stats->rx_dropped);
+ napi_free_frags(&tfile->napi);
+ mutex_unlock(&tfile->napi_mutex);
+ WARN_ON(1);
+ return -ENOMEM;
+ }
+
+ local_bh_disable();
+ napi_gro_frags(&tfile->napi);
+ local_bh_enable();
+ mutex_unlock(&tfile->napi_mutex);
+ } else if (IS_ENABLED(CONFIG_TUN_NAPI)) {
struct sk_buff_head *queue = &tfile->sk.sk_write_queue;
int queue_len;
@@ -2168,6 +2287,10 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
tun->flags = (tun->flags & ~TUN_FEATURES) |
(ifr->ifr_flags & TUN_FEATURES);
+ if (!IS_ENABLED(CONFIG_TUN_NAPI) ||
+ (tun->flags & TUN_TYPE_MASK) != IFF_TAP)
+ tun->flags = tun->flags & ~IFF_NAPI_FRAGS;
+
/* Make sure persistent devices do not get stuck in
* xoff state.
*/
diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
index 3cb5e1d85ddd..1eb1eb42f151 100644
--- a/include/uapi/linux/if_tun.h
+++ b/include/uapi/linux/if_tun.h
@@ -60,6 +60,7 @@
/* TUNSETIFF ifr flags */
#define IFF_TUN 0x0001
#define IFF_TAP 0x0002
+#define IFF_NAPI_FRAGS 0x0010
#define IFF_NO_PI 0x1000
/* This flag has no real effect */
#define IFF_ONE_QUEUE 0x2000
--
2.14.1.581.gf28d330327-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox