* Re: [PATCH v5 net-next 1/6] xdp: allow same allocator usage
From: Ivan Khoronzhuk @ 2019-07-02 14:53 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: grygorii.strashko, hawk, davem, ast, linux-kernel, linux-omap,
xdp-newbies, ilias.apalodimas, netdev, daniel, jakub.kicinski,
john.fastabend
In-Reply-To: <20190702164648.56ff0761@carbon>
On Tue, Jul 02, 2019 at 04:46:48PM +0200, Jesper Dangaard Brouer wrote:
>On Tue, 2 Jul 2019 13:27:07 +0300
>Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>
>> On Mon, Jul 01, 2019 at 01:40:59PM +0200, Jesper Dangaard Brouer wrote:
>> >
>> >I'm very skeptical about this approach.
>> >
>> >On Sun, 30 Jun 2019 20:23:43 +0300
>> >Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>> >
>> >> XDP rxqs can be same for ndevs running under same rx napi softirq.
>> >> But there is no ability to register same allocator for both rxqs,
>> >> by fact it can same rxq but has different ndev as a reference.
>> >
>> >This description is not very clear. It can easily be misunderstood.
>> >
>> >It is an absolute requirement that each RX-queue have their own
>> >page_pool object/allocator. (This where the performance comes from) as
>> >the page_pool have NAPI protected array for alloc and XDP_DROP recycle.
>> >
>> >Your driver/hardware seems to have special case, where a single
>> >RX-queue can receive packets for two different net_device'es.
>> >
>> >Do you violate this XDP devmap redirect assumption[1]?
>> >[1] https://github.com/torvalds/linux/blob/v5.2-rc7/kernel/bpf/devmap.c#L324-L329
>> Seems that yes, but that's used only for trace for now.
>> As it runs under napi and flush clear dev_rx i must do it right in the
>> rx_handler. So next patchset version will have one patch less.
>>
>> Thanks!
>>
>> >
>> >
>> >> Due to last changes allocator destroy can be defered till the moment
>> >> all packets are recycled by destination interface, afterwards it's
>> >> freed. In order to schedule allocator destroy only after all users are
>> >> unregistered, add refcnt to allocator object and schedule to destroy
>> >> only it reaches 0.
>> >
>> >The guiding principles when designing an API, is to make it easy to
>> >use, but also make it hard to misuse.
>> >
>> >Your API change makes it easy to misuse the API. As it make it easy to
>> >(re)use the allocator pointer (likely page_pool) for multiple
>> >xdp_rxq_info structs. It is only valid for your use-case, because you
>> >have hardware where a single RX-queue delivers to two different
>> >net_devices. For other normal use-cases, this will be a violation.
>> >
>> >If I was a user of this API, and saw your xdp_allocator_get(), then I
>> >would assume that this was the normal case. As minimum, we need to add
>> >a comment in the code, about this specific/intended use-case. I
>> >through about detecting the misuse, by adding a queue_index to
>> >xdp_mem_allocator, that can be checked against, when calling
>> >xdp_rxq_info_reg_mem_model() with another xdp_rxq_info struct (to catch
>> >the obvious mistake where queue_index mismatch).
>>
>> I can add, but not sure if it has or can have some conflicts with another
>> memory allocators, now or in future. Main here to not became a cornerstone
>> in some not obvious use-cases.
>>
>> So, for now, let it be in this way:
>>
>> --- a/include/net/xdp_priv.h
>> +++ b/include/net/xdp_priv.h
>> @@ -19,6 +19,7 @@ struct xdp_mem_allocator {
>> struct delayed_work defer_wq;
>> unsigned long defer_warn;
>> unsigned long refcnt;
>> + u32 queue_index;
>> };
>>
>> #endif /* __LINUX_NET_XDP_PRIV_H__ */
>> diff --git a/net/core/xdp.c b/net/core/xdp.c
>> index a44621190fdc..c4bf29810f4d 100644
>> --- a/net/core/xdp.c
>> +++ b/net/core/xdp.c
>> @@ -324,7 +324,7 @@ static bool __is_supported_mem_type(enum xdp_mem_type type)
>> return true;
>> }
>>
>> -static struct xdp_mem_allocator *xdp_allocator_get(void *allocator)
>> +static struct xdp_mem_allocator *xdp_allocator_find(void *allocator)
>> {
>> struct xdp_mem_allocator *xae, *xa = NULL;
>> struct rhashtable_iter iter;
>> @@ -336,7 +336,6 @@ static struct xdp_mem_allocator *xdp_allocator_get(void *allocator)
>>
>> while ((xae = rhashtable_walk_next(&iter)) && !IS_ERR(xae)) {
>> if (xae->allocator == allocator) {
>> - xae->refcnt++;
>> xa = xae;
>> break;
>> }
>> @@ -386,9 +385,13 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
>> }
>> }
>>
>> - xdp_alloc = xdp_allocator_get(allocator);
>> + xdp_alloc = xdp_allocator_find(allocator);
>> if (xdp_alloc) {
>> + if (xdp_alloc->queue_index != xdp_rxq->queue_index)
>> + return -EINVAL;
>> +
>> xdp_rxq->mem.id = xdp_alloc->mem.id;
>> + xdp_alloc->refcnt++;
>
>This is now adjusted outside lock, not good.
In final it serves:
From f43a0b85838f75814cc93e5a724c4c7e5615f936 Mon Sep 17 00:00:00 2001
From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
Date: Fri, 28 Jun 2019 03:17:24 +0300
Subject: [PATCH] xdp: allow same allocator usage
XDP rxqs can be same for ndevs running under same rx napi softirq.
But there is no ability to register same allocator for both rxqs,
by fact it can same rxq but has different ndev as a reference.
Due to last changes allocator destroy can be defered till the moment
all packets are recycled by destination interface, afterwards it's
freed. In order to schedule allocator destroy only after all users are
unregistered, add refcnt to allocator object and schedule to destroy
only it reaches 0.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
include/net/xdp_priv.h | 2 ++
net/core/xdp.c | 52 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 54 insertions(+)
diff --git a/include/net/xdp_priv.h b/include/net/xdp_priv.h
index 6a8cba6ea79a..9858a4057842 100644
--- a/include/net/xdp_priv.h
+++ b/include/net/xdp_priv.h
@@ -18,6 +18,8 @@ struct xdp_mem_allocator {
struct rcu_head rcu;
struct delayed_work defer_wq;
unsigned long defer_warn;
+ unsigned long refcnt;
+ u32 queue_index;
};
#endif /* __LINUX_NET_XDP_PRIV_H__ */
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 829377cc83db..090f26e4f793 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -98,6 +98,18 @@ static bool __mem_id_disconnect(int id, bool force)
WARN(1, "Request remove non-existing id(%d), driver bug?", id);
return true;
}
+
+ /* to avoid calling hash lookup twice, decrement refcnt here till it
+ * reaches zero, then it can be called from workqueue afterwards.
+ */
+ if (xa->refcnt)
+ xa->refcnt--;
+
+ if (xa->refcnt) {
+ mutex_unlock(&mem_id_lock);
+ return true;
+ }
+
xa->disconnect_cnt++;
/* Detects in-flight packet-pages for page_pool */
@@ -312,6 +324,30 @@ static bool __is_supported_mem_type(enum xdp_mem_type type)
return true;
}
+static struct xdp_mem_allocator *xdp_allocator_find(void *allocator)
+{
+ struct xdp_mem_allocator *xae, *xa = NULL;
+ struct rhashtable_iter iter;
+
+ rhashtable_walk_enter(mem_id_ht, &iter);
+ do {
+ rhashtable_walk_start(&iter);
+
+ while ((xae = rhashtable_walk_next(&iter)) && !IS_ERR(xae)) {
+ if (xae->allocator == allocator) {
+ xa = xae;
+ break;
+ }
+ }
+
+ rhashtable_walk_stop(&iter);
+
+ } while (xae == ERR_PTR(-EAGAIN));
+ rhashtable_walk_exit(&iter);
+
+ return xa;
+}
+
int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
enum xdp_mem_type type, void *allocator)
{
@@ -347,6 +383,20 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
}
}
+ mutex_lock(&mem_id_lock);
+ xdp_alloc = xdp_allocator_find(allocator);
+ if (xdp_alloc) {
+ /* One allocator per queue is supposed only */
+ if (xdp_alloc->queue_index != xdp_rxq->queue_index)
+ return -EINVAL;
+
+ xdp_rxq->mem.id = xdp_alloc->mem.id;
+ xdp_alloc->refcnt++;
+ mutex_unlock(&mem_id_lock);
+ return 0;
+ }
+ mutex_unlock(&mem_id_lock);
+
xdp_alloc = kzalloc(sizeof(*xdp_alloc), gfp);
if (!xdp_alloc)
return -ENOMEM;
@@ -360,6 +410,8 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
xdp_rxq->mem.id = id;
xdp_alloc->mem = xdp_rxq->mem;
xdp_alloc->allocator = allocator;
+ xdp_alloc->refcnt = 1;
+ xdp_alloc->queue_index = xdp_rxq->queue_index;
/* Insert allocator into ID lookup table */
ptr = rhashtable_insert_slow(mem_id_ht, &id, &xdp_alloc->node);
>
>> return 0;
>> }
>>
>> @@ -406,6 +409,7 @@ int xdp_rxq_info_reg_mem_model(struct xdp_rxq_info *xdp_rxq,
>> xdp_alloc->mem = xdp_rxq->mem;
>> xdp_alloc->allocator = allocator;
>> xdp_alloc->refcnt = 1;
>> + xdp_alloc->queue_index = xdp_rxq->queue_index;
>>
>> /* Insert allocator into ID lookup table */
>> ptr = rhashtable_insert_slow(mem_id_ht, &id, &xdp_alloc->node);
>>
>> Jesper, are you Ok with this version?
>
>Please see my other patch, this is based on our first refcnt attempt.
>I think that other patch is a better way forward.
XDP patch serves it better and can prevent not only obj deletion but also
pool flush. So I propose use 2 patches.
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply related
* Re: [PATCH net-next v6 01/15] rtnetlink: provide permanent hardware address in RTM_NEWLINK
From: Stephen Hemminger @ 2019-07-02 14:55 UTC (permalink / raw)
To: Michal Kubecek
Cc: David Miller, netdev, Jakub Kicinski, Jiri Pirko, Andrew Lunn,
Florian Fainelli, John Linville, Johannes Berg, linux-kernel
In-Reply-To: <b6e0aefbcb58297b3ec0a12ee4be8e5194eee61a.1562067622.git.mkubecek@suse.cz>
On Tue, 2 Jul 2019 13:49:44 +0200 (CEST)
Michal Kubecek <mkubecek@suse.cz> wrote:
> Permanent hardware address of a network device was traditionally provided
> via ethtool ioctl interface but as Jiri Pirko pointed out in a review of
> ethtool netlink interface, rtnetlink is much more suitable for it so let's
> add it to the RTM_NEWLINK message.
>
> Add IFLA_PERM_ADDRESS attribute to RTM_NEWLINK messages unless the
> permanent address is all zeros (i.e. device driver did not fill it). As
> permanent address is not modifiable, reject userspace requests containing
> IFLA_PERM_ADDRESS attribute.
>
> Note: we already provide permanent hardware address for bond slaves;
> unfortunately we cannot drop that attribute for backward compatibility
> reasons.
>
> v5 -> v6: only add the attribute if permanent address is not zero
>
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
Do you want to make an iproute patch to display this?
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
^ permalink raw reply
* [PATCH net-next] rxrpc: Fix uninitialized error code in rxrpc_send_data_packet()
From: David Howells @ 2019-07-02 14:55 UTC (permalink / raw)
To: netdev; +Cc: Geert Uytterhoeven, dhowells, linux-afs, linux-kernel
With gcc 4.1:
net/rxrpc/output.c: In function ‘rxrpc_send_data_packet’:
net/rxrpc/output.c:338: warning: ‘ret’ may be used uninitialized in this function
Indeed, if the first jump to the send_fragmentable label is made, and
the address family is not handled in the switch() statement, ret will be
used uninitialized.
Fix this by BUG()'ing as is done in other places in rxrpc where internal
support for future address families will need adding. It should not be
possible to reach this normally as the address families are checked
up-front.
Fixes: 5a924b8951f835b5 ("rxrpc: Don't store the rxrpc header in the Tx queue sk_buffs")
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/rxrpc/output.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index a0b6abfbd277..948e3fe249ec 100644
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -519,6 +519,9 @@ int rxrpc_send_data_packet(struct rxrpc_call *call, struct sk_buff *skb,
}
break;
#endif
+
+ default:
+ BUG();
}
if (ret < 0)
^ permalink raw reply related
* Re: [PATCH] net: core: page_pool: add user refcnt and reintroduce page_pool_destroy
From: Ivan Khoronzhuk @ 2019-07-02 14:56 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, Ilias Apalodimas, grygorii.strashko, jakub.kicinski,
daniel, john.fastabend, ast, linux-kernel, linux-omap
In-Reply-To: <20190702165230.6caa36e3@carbon>
On Tue, Jul 02, 2019 at 04:52:30PM +0200, Jesper Dangaard Brouer wrote:
>On Tue, 2 Jul 2019 17:44:27 +0300
>Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>
>> On Tue, Jul 02, 2019 at 04:31:39PM +0200, Jesper Dangaard Brouer wrote:
>> >From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> >
>> >Jesper recently removed page_pool_destroy() (from driver invocation) and
>> >moved shutdown and free of page_pool into xdp_rxq_info_unreg(), in-order to
>> >handle in-flight packets/pages. This created an asymmetry in drivers
>> >create/destroy pairs.
>> >
>> >This patch add page_pool user refcnt and reintroduce page_pool_destroy.
>> >This serves two purposes, (1) simplify drivers error handling as driver now
>> >drivers always calls page_pool_destroy() and don't need to track if
>> >xdp_rxq_info_reg_mem_model() was unsuccessful. (2) allow special cases
>> >where a single RX-queue (with a single page_pool) provides packets for two
>> >net_device'es, and thus needs to register the same page_pool twice with two
>> >xdp_rxq_info structures.
>>
>> As I tend to use xdp level patch there is no more reason to mention (2) case
>> here. XDP patch serves it better and can prevent not only obj deletion but also
>> pool flush, so, this one patch I could better leave only for (1) case.
>
>I don't understand what you are saying.
>
>Do you approve this patch, or do you reject this patch?
>
It's not reject, it's proposition to use both, XDP and page pool patches,
each having its goal.
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* [PATCH net] rxrpc: Fix send on a connected, but unbound socket
From: David Howells @ 2019-07-02 14:59 UTC (permalink / raw)
To: netdev
Cc: syzbot+7966f2a0b2c7da8939b4, Marc Dionne, dhowells, linux-afs,
linux-kernel
If sendmsg() or sendmmsg() is called on a connected socket that hasn't had
bind() called on it, then an oops will occur when the kernel tries to
connect the call because no local endpoint has been allocated.
Fix this by implicitly binding the socket if it is in the
RXRPC_CLIENT_UNBOUND state, just like it does for the RXRPC_UNBOUND state.
Further, the state should be transitioned to RXRPC_CLIENT_BOUND after this
to prevent further attempts to bind it.
This can be tested with:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <linux/rxrpc.h>
static const unsigned char inet6_addr[16] = {
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -1, -1, 0xac, 0x14, 0x14, 0xaa
};
int main(void)
{
struct sockaddr_rxrpc srx;
struct cmsghdr *cm;
struct msghdr msg;
unsigned char control[16];
int fd;
memset(&srx, 0, sizeof(srx));
srx.srx_family = 0x21;
srx.srx_service = 0;
srx.transport_type = AF_INET;
srx.transport_len = 0x1c;
srx.transport.sin6.sin6_family = AF_INET6;
srx.transport.sin6.sin6_port = htons(0x4e22);
srx.transport.sin6.sin6_flowinfo = htons(0x4e22);
srx.transport.sin6.sin6_scope_id = htons(0xaa3b);
memcpy(&srx.transport.sin6.sin6_addr, inet6_addr, 16);
cm = (struct cmsghdr *)control;
cm->cmsg_len = CMSG_LEN(sizeof(unsigned long));
cm->cmsg_level = SOL_RXRPC;
cm->cmsg_type = RXRPC_USER_CALL_ID;
*(unsigned long *)CMSG_DATA(cm) = 0;
msg.msg_name = NULL;
msg.msg_namelen = 0;
msg.msg_iov = NULL;
msg.msg_iovlen = 0;
msg.msg_control = control;
msg.msg_controllen = cm->cmsg_len;
msg.msg_flags = 0;
fd = socket(AF_RXRPC, SOCK_DGRAM, AF_INET);
connect(fd, (struct sockaddr *)&srx, sizeof(srx));
sendmsg(fd, &msg, 0);
return 0;
}
Leading to the following oops:
BUG: kernel NULL pointer dereference, address: 0000000000000018
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
...
RIP: 0010:rxrpc_connect_call+0x42/0xa01
...
Call Trace:
? mark_held_locks+0x47/0x59
? __local_bh_enable_ip+0xb6/0xba
rxrpc_new_client_call+0x3b1/0x762
? rxrpc_do_sendmsg+0x3c0/0x92e
rxrpc_do_sendmsg+0x3c0/0x92e
rxrpc_sendmsg+0x16b/0x1b5
sock_sendmsg+0x2d/0x39
___sys_sendmsg+0x1a4/0x22a
? release_sock+0x19/0x9e
? reacquire_held_locks+0x136/0x160
? release_sock+0x19/0x9e
? find_held_lock+0x2b/0x6e
? __lock_acquire+0x268/0xf73
? rxrpc_connect+0xdd/0xe4
? __local_bh_enable_ip+0xb6/0xba
__sys_sendmsg+0x5e/0x94
do_syscall_64+0x7d/0x1bf
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Fixes: 2341e0775747 ("rxrpc: Simplify connect() implementation and simplify sendmsg() op")
Reported-by: syzbot+7966f2a0b2c7da8939b4@syzkaller.appspotmail.com
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Marc Dionne <marc.dionne@auristor.com>
---
net/rxrpc/af_rxrpc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c
index f9f4721cdfa7..d09eaf153544 100644
--- a/net/rxrpc/af_rxrpc.c
+++ b/net/rxrpc/af_rxrpc.c
@@ -545,6 +545,7 @@ static int rxrpc_sendmsg(struct socket *sock, struct msghdr *m, size_t len)
switch (rx->sk.sk_state) {
case RXRPC_UNBOUND:
+ case RXRPC_CLIENT_UNBOUND:
rx->srx.srx_family = AF_RXRPC;
rx->srx.srx_service = 0;
rx->srx.transport_type = SOCK_DGRAM;
@@ -569,10 +570,9 @@ static int rxrpc_sendmsg(struct socket *sock, struct msghdr *m, size_t len)
}
rx->local = local;
- rx->sk.sk_state = RXRPC_CLIENT_UNBOUND;
+ rx->sk.sk_state = RXRPC_CLIENT_BOUND;
/* Fall through */
- case RXRPC_CLIENT_UNBOUND:
case RXRPC_CLIENT_BOUND:
if (!m->msg_name &&
test_bit(RXRPC_SOCK_CONNECTED, &rx->flags)) {
^ permalink raw reply related
* Re: [PATCH 1/1] mlx5: Fix build when CONFIG_MLX5_EN_RXNFC is disabled
From: Jes Sorensen @ 2019-07-02 15:00 UTC (permalink / raw)
To: Saeed Mahameed
Cc: kernel-team@fb.com, jsorensen@fb.com, netdev@vger.kernel.org
In-Reply-To: <ff76fcde486792ad01be1476a66a726d6e1ab933.camel@mellanox.com>
On 6/25/19 2:00 PM, Saeed Mahameed wrote:
> On Tue, 2019-06-25 at 11:27 -0400, Jes Sorensen wrote:
>> From: Jes Sorensen <jsorensen@fb.com>
>>
>> The previous patch broke the build with a static declaration for
>> a public function.
>>
>> Fixes: 8f0916c6dc5c (net/mlx5e: Fix ethtool rxfh commands when
>> CONFIG_MLX5_EN_RXNFC is disabled)
>> Signed-off-by: Jes Sorensen <jsorensen@fb.com>
>> ---
>> drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> index dd764e0471f2..776040d91bd4 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
>> @@ -1905,7 +1905,8 @@ static int mlx5e_flash_device(struct net_device
>> *dev,
>> /* When CONFIG_MLX5_EN_RXNFC=n we only support ETHTOOL_GRXRINGS
>> * otherwise this function will be defined from en_fs_ethtool.c
>> */
>
> As the above comment states, when CONFIG_MLX5_EN_RXNFC is disabled,
> mlx5e_get_rxnfc is only defined, declared and used in this file, so it
> must be static. Otherwise it will be defined in another file which
> provides much much more functionality for ethtool flow steering.
>
> can you please provide more information of what went wrong on your
> build machine ?
Sorry was swamped here!
Looks like you're right, it only triggers in our build due to some
patches we don't have from upstream. I did the patch against upstream
and applied it to our tree, so should have checked further there.
Cheers,
Jes
^ permalink raw reply
* Re: [PATCH bpf] xdp: fix race on generic receive path
From: Magnus Karlsson @ 2019-07-02 15:01 UTC (permalink / raw)
To: Ilya Maximets
Cc: Network Development, linux-kernel, bpf, xdp-newbies,
David S. Miller, Björn Töpel, Magnus Karlsson,
Jonathan Lemon, Jakub Kicinski, Alexei Starovoitov,
Daniel Borkmann
In-Reply-To: <20190702143634.19688-1-i.maximets@samsung.com>
On Tue, Jul 2, 2019 at 4:36 PM Ilya Maximets <i.maximets@samsung.com> wrote:
>
> Unlike driver mode, generic xdp receive could be triggered
> by different threads on different CPU cores at the same time
> leading to the fill and rx queue breakage. For example, this
> could happen while sending packets from two processes to the
> first interface of veth pair while the second part of it is
> open with AF_XDP socket.
>
> Need to take a lock for each generic receive to avoid race.
Thanks for this catch Ilya. Do you have any performance numbers you
could share of the impact of adding this spin lock? The reason I ask
is that if the impact is negligible, then let us just add it. But if
it is too large, we might want to brain storm about some other
possible solutions.
Thanks: Magnus
> Fixes: c497176cb2e4 ("xsk: add Rx receive functions and poll support")
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
> include/net/xdp_sock.h | 2 ++
> net/xdp/xsk.c | 32 +++++++++++++++++++++++---------
> 2 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
> index d074b6d60f8a..ac3c047d058c 100644
> --- a/include/net/xdp_sock.h
> +++ b/include/net/xdp_sock.h
> @@ -67,6 +67,8 @@ struct xdp_sock {
> * in the SKB destructor callback.
> */
> spinlock_t tx_completion_lock;
> + /* Protects generic receive. */
> + spinlock_t rx_lock;
> u64 rx_dropped;
> };
>
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index a14e8864e4fa..19f41d2b670c 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -119,17 +119,22 @@ int xsk_generic_rcv(struct xdp_sock *xs, struct xdp_buff *xdp)
> {
> u32 metalen = xdp->data - xdp->data_meta;
> u32 len = xdp->data_end - xdp->data;
> + unsigned long flags;
> void *buffer;
> u64 addr;
> int err;
>
> - if (xs->dev != xdp->rxq->dev || xs->queue_id != xdp->rxq->queue_index)
> - return -EINVAL;
> + spin_lock_irqsave(&xs->rx_lock, flags);
> +
> + if (xs->dev != xdp->rxq->dev || xs->queue_id != xdp->rxq->queue_index) {
> + err = -EINVAL;
> + goto out_unlock;
> + }
>
> if (!xskq_peek_addr(xs->umem->fq, &addr) ||
> len > xs->umem->chunk_size_nohr - XDP_PACKET_HEADROOM) {
> - xs->rx_dropped++;
> - return -ENOSPC;
> + err = -ENOSPC;
> + goto out_drop;
> }
>
> addr += xs->umem->headroom;
> @@ -138,13 +143,21 @@ int xsk_generic_rcv(struct xdp_sock *xs, struct xdp_buff *xdp)
> memcpy(buffer, xdp->data_meta, len + metalen);
> addr += metalen;
> err = xskq_produce_batch_desc(xs->rx, addr, len);
> - if (!err) {
> - xskq_discard_addr(xs->umem->fq);
> - xsk_flush(xs);
> - return 0;
> - }
> + if (err)
> + goto out_drop;
> +
> + xskq_discard_addr(xs->umem->fq);
> + xskq_produce_flush_desc(xs->rx);
>
> + spin_unlock_irqrestore(&xs->rx_lock, flags);
> +
> + xs->sk.sk_data_ready(&xs->sk);
> + return 0;
> +
> +out_drop:
> xs->rx_dropped++;
> +out_unlock:
> + spin_unlock_irqrestore(&xs->rx_lock, flags);
> return err;
> }
>
> @@ -765,6 +778,7 @@ static int xsk_create(struct net *net, struct socket *sock, int protocol,
>
> xs = xdp_sk(sk);
> mutex_init(&xs->mutex);
> + spin_lock_init(&xs->rx_lock);
> spin_lock_init(&xs->tx_completion_lock);
>
> mutex_lock(&net->xdp.lock);
> --
> 2.17.1
>
^ permalink raw reply
* re: mlxsw: spectrum: PTP: Support timestamping on Spectrum-1 - potential null ptr dereference
From: Colin Ian King @ 2019-07-02 15:01 UTC (permalink / raw)
To: Petr Machata, Jiri Pirko, Ido Schimmel, David S. Miller, netdev,
linux-kernel@vger.kernel.org
Hi,
Static analysis with Coverity on today's linux-next has found a
potential null pointer dereference bug with the following commit:
commit d92e4e6e33c8b19635be70fb8935b627d2e4f8fe
Author: Petr Machata <petrm@mellanox.com>
Date: Sun Jun 30 09:04:56 2019 +0300
mlxsw: spectrum: PTP: Support timestamping on Spectrum-1
In function: mlxsw_sp1_ptp_packet_finish the offending code is as follows:
/* Between capturing the packet and finishing it, there is a
window of
* opportunity for the originating port to go away (e.g. due to a
* split). Also make sure the SKB device reference is still valid.
*/
mlxsw_sp_port = mlxsw_sp->ports[local_port];
if (!mlxsw_sp_port && (!skb->dev || skb->dev ==
mlxsw_sp_port->dev)) {
dev_kfree_skb_any(skb);
return;
}
If mlxsw_sp_port is null and skb->dev is not-null then the comparison
"skb->dev == mlxsw_sp_port->dev" ends up with a null pointer dereference.
I think the if statement should be:
if (mlxsw_sp_port && (!skb->dev || skb->dev == mlxsw_sp_port->dev))
..but I'm not 100% sure as I may be missing something a bit more subtle
here.
Colin
^ permalink raw reply
* [PATCH net] rxrpc: Fix oops in tracepoint
From: David Howells @ 2019-07-02 15:04 UTC (permalink / raw)
To: netdev; +Cc: Marc Dionne, dhowells, linux-afs, linux-kernel
If the rxrpc_eproto tracepoint is enabled, an oops will be cause by the
trace line that rxrpc_extract_header() tries to emit when a protocol error
occurs (typically because the packet is short) because the call argument is
NULL.
Fix this by using ?: to assume 0 as the debug_id if call is NULL.
This can then be induced by:
echo -e '\0\0\0\0\0\0\0\0' | ncat -4u --send-only <addr> 20001
where addr has the following program running on it:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <linux/rxrpc.h>
int main(void)
{
struct sockaddr_rxrpc srx;
int fd;
memset(&srx, 0, sizeof(srx));
srx.srx_family = AF_RXRPC;
srx.srx_service = 0;
srx.transport_type = AF_INET;
srx.transport_len = sizeof(srx.transport.sin);
srx.transport.sin.sin_family = AF_INET;
srx.transport.sin.sin_port = htons(0x4e21);
fd = socket(AF_RXRPC, SOCK_DGRAM, AF_INET6);
bind(fd, (struct sockaddr *)&srx, sizeof(srx));
sleep(20);
return 0;
}
It results in the following oops.
BUG: kernel NULL pointer dereference, address: 0000000000000340
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
...
RIP: 0010:trace_event_raw_event_rxrpc_rx_eproto+0x47/0xac
...
Call Trace:
<IRQ>
rxrpc_extract_header+0x86/0x171
? rcu_read_lock_sched_held+0x5d/0x63
? rxrpc_new_skb+0xd4/0x109
rxrpc_input_packet+0xef/0x14fc
? rxrpc_input_data+0x986/0x986
udp_queue_rcv_one_skb+0xbf/0x3d0
udp_unicast_rcv_skb.isra.8+0x64/0x71
ip_protocol_deliver_rcu+0xe4/0x1b4
ip_local_deliver+0xf0/0x154
__netif_receive_skb_one_core+0x50/0x6c
netif_receive_skb_internal+0x26b/0x2e9
napi_gro_receive+0xf8/0x1da
rtl8169_poll+0x303/0x4c4
net_rx_action+0x10e/0x333
__do_softirq+0x1a5/0x38f
irq_exit+0x54/0xc4
do_IRQ+0xda/0xf8
common_interrupt+0xf/0xf
</IRQ>
...
? cpuidle_enter_state+0x23c/0x34d
cpuidle_enter+0x2a/0x36
do_idle+0x163/0x1ea
cpu_startup_entry+0x1d/0x1f
start_secondary+0x157/0x172
secondary_startup_64+0xa4/0xb0
Fixes: a25e21f0bcd2 ("rxrpc, afs: Use debug_ids rather than pointers in traces")
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Marc Dionne <marc.dionne@auristor.com>
---
include/trace/events/rxrpc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index d85816878a52..cc1d060cbf13 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -1379,7 +1379,7 @@ TRACE_EVENT(rxrpc_rx_eproto,
),
TP_fast_assign(
- __entry->call = call->debug_id;
+ __entry->call = call ? call->debug_id : 0;
__entry->serial = serial;
__entry->why = why;
),
^ permalink raw reply related
* [PATCH net-next 1/1] qed: Add support for Timestamping the unicast PTP packets.
From: Sudarsana Reddy Kalluru @ 2019-07-02 15:04 UTC (permalink / raw)
To: davem; +Cc: netdev, mkalderon, aelior
This patch masks the lower 4 bits of PARAM-MASK registers as required
by the Hardware to detect/timestamp the unicast PTP packets. The register
definition in the header file captures more details on the individual bits.
Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
drivers/net/ethernet/qlogic/qed/qed_ptp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_ptp.c b/drivers/net/ethernet/qlogic/qed/qed_ptp.c
index f3ebdc5..4a7acfc 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_ptp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_ptp.c
@@ -243,7 +243,7 @@ static int qed_ptp_hw_cfg_filters(struct qed_dev *cdev,
return -EINVAL;
}
- qed_wr(p_hwfn, p_ptt, NIG_REG_LLH_PTP_PARAM_MASK, 0);
+ qed_wr(p_hwfn, p_ptt, NIG_REG_LLH_PTP_PARAM_MASK, 0xF);
qed_wr(p_hwfn, p_ptt, NIG_REG_LLH_PTP_RULE_MASK, rule_mask);
qed_wr(p_hwfn, p_ptt, NIG_REG_RX_PTP_EN, enable_cfg);
@@ -253,7 +253,7 @@ static int qed_ptp_hw_cfg_filters(struct qed_dev *cdev,
qed_wr(p_hwfn, p_ptt, NIG_REG_TX_LLH_PTP_RULE_MASK, 0x3FFF);
} else {
qed_wr(p_hwfn, p_ptt, NIG_REG_TX_PTP_EN, enable_cfg);
- qed_wr(p_hwfn, p_ptt, NIG_REG_TX_LLH_PTP_PARAM_MASK, 0);
+ qed_wr(p_hwfn, p_ptt, NIG_REG_TX_LLH_PTP_PARAM_MASK, 0xF);
qed_wr(p_hwfn, p_ptt, NIG_REG_TX_LLH_PTP_RULE_MASK, rule_mask);
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] net: core: page_pool: add user refcnt and reintroduce page_pool_destroy
From: Jesper Dangaard Brouer @ 2019-07-02 15:10 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: netdev, Ilias Apalodimas, grygorii.strashko, jakub.kicinski,
daniel, john.fastabend, ast, linux-kernel, linux-omap, brouer
In-Reply-To: <20190702145612.GF4510@khorivan>
On Tue, 2 Jul 2019 17:56:13 +0300
Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> On Tue, Jul 02, 2019 at 04:52:30PM +0200, Jesper Dangaard Brouer wrote:
> >On Tue, 2 Jul 2019 17:44:27 +0300
> >Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> >
> >> On Tue, Jul 02, 2019 at 04:31:39PM +0200, Jesper Dangaard Brouer wrote:
> >> >From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
> >> >
> >> >Jesper recently removed page_pool_destroy() (from driver invocation) and
> >> >moved shutdown and free of page_pool into xdp_rxq_info_unreg(), in-order to
> >> >handle in-flight packets/pages. This created an asymmetry in drivers
> >> >create/destroy pairs.
> >> >
> >> >This patch add page_pool user refcnt and reintroduce page_pool_destroy.
> >> >This serves two purposes, (1) simplify drivers error handling as driver now
> >> >drivers always calls page_pool_destroy() and don't need to track if
> >> >xdp_rxq_info_reg_mem_model() was unsuccessful. (2) allow special cases
> >> >where a single RX-queue (with a single page_pool) provides packets for two
> >> >net_device'es, and thus needs to register the same page_pool twice with two
> >> >xdp_rxq_info structures.
> >>
> >> As I tend to use xdp level patch there is no more reason to mention (2) case
> >> here. XDP patch serves it better and can prevent not only obj deletion but also
> >> pool flush, so, this one patch I could better leave only for (1) case.
> >
> >I don't understand what you are saying.
> >
> >Do you approve this patch, or do you reject this patch?
> >
> It's not reject, it's proposition to use both, XDP and page pool patches,
> each having its goal.
Just to be clear, if you want this patch to get accepted you have to
reply with your Signed-off-by (as I wrote).
Maybe we should discuss it in another thread, about why you want two
solutions to the same problem.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH bpf-next] libbpf: fix GCC8 warning for strncpy
From: Andrii Nakryiko @ 2019-07-02 15:14 UTC (permalink / raw)
To: Y Song
Cc: Andrii Nakryiko, bpf, netdev, Alexei Starovoitov, Daniel Borkmann,
Magnus Karlsson
In-Reply-To: <CAH3MdRUv9eJuecKq7weG614+6oEtfLeUHnTxoU19qr39p9-mrQ@mail.gmail.com>
On Mon, Jul 1, 2019 at 11:10 PM Y Song <ys114321@gmail.com> wrote:
>
> On Mon, Jul 1, 2019 at 10:47 PM Andrii Nakryiko <andriin@fb.com> wrote:
> >
> > GCC8 started emitting warning about using strncpy with number of bytes
> > exactly equal destination size, which is generally unsafe, as can lead
> > to non-zero terminated string being copied. Use IFNAMSIZ - 1 as number
> > of bytes to ensure name is always zero-terminated.
> >
> > Cc: Magnus Karlsson <magnus.karlsson@intel.com>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> > tools/lib/bpf/xsk.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> > index bf15a80a37c2..9588e7f87d0b 100644
> > --- a/tools/lib/bpf/xsk.c
> > +++ b/tools/lib/bpf/xsk.c
> > @@ -327,7 +327,7 @@ static int xsk_get_max_queues(struct xsk_socket *xsk)
> >
> > channels.cmd = ETHTOOL_GCHANNELS;
> > ifr.ifr_data = (void *)&channels;
> > - strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ);
> > + strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ - 1);
>
> To accommodate the xsk->ifname string length FNAMSIZ - 1, we need to have
> ifr.ifr_name[FNAMSIZ - 1] = '\0';
> right?
Yes. I somehow misread description of strncpy and assumed it does that
automatically (which would make sense), but it actually doesn't. Only
strlcpy does. v2 with fix is coming.
>
> > err = ioctl(fd, SIOCETHTOOL, &ifr);
> > if (err && errno != EOPNOTSUPP) {
> > ret = -errno;
> > --
> > 2.17.1
> >
^ permalink raw reply
* [PATCH v2 bpf-next] libbpf: fix GCC8 warning for strncpy
From: Andrii Nakryiko @ 2019-07-02 15:16 UTC (permalink / raw)
To: andrii.nakryiko, bpf, netdev, ast, daniel, andriin,
magnus.karlsson
GCC8 started emitting warning about using strncpy with number of bytes
exactly equal destination size, which is generally unsafe, as can lead
to non-zero terminated string being copied. Use IFNAMSIZ - 1 as number
of bytes to ensure name is always zero-terminated.
Cc: Magnus Karlsson <magnus.karlsson@intel.com>
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
tools/lib/bpf/xsk.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
index bf15a80a37c2..b33740221b7e 100644
--- a/tools/lib/bpf/xsk.c
+++ b/tools/lib/bpf/xsk.c
@@ -327,7 +327,8 @@ static int xsk_get_max_queues(struct xsk_socket *xsk)
channels.cmd = ETHTOOL_GCHANNELS;
ifr.ifr_data = (void *)&channels;
- strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ);
+ strncpy(ifr.ifr_name, xsk->ifname, IFNAMSIZ - 1);
+ ifr.ifr_name[IFNAMSIZ - 1] = '\0';
err = ioctl(fd, SIOCETHTOOL, &ifr);
if (err && errno != EOPNOTSUPP) {
ret = -errno;
--
2.17.1
^ permalink raw reply related
* [PATCH net 0/6] gtp: fix several bugs
From: Taehee Yoo @ 2019-07-02 15:20 UTC (permalink / raw)
To: davem, pablo, laforge, osmocom-net-gprs, netdev; +Cc: ap420073
This patch series fixes several bugs in the gtp module.
First patch fixes suspicious RCU usage.
The problem is to use rcu_dereference_sk_user_data() outside of
RCU read critical section.
Second patch fixes use-after-free.
gtp_encap_destroy() is called twice.
gtp_encap_destroy() use both gtp->sk0 and gtp->sk1u.
these pointers can be freed in gtp_encap_destroy().
So, gtp_encap_destroy() should avoid using freed sk pointer.
Third patch removes duplicate code in gtp_dellink().
gtp_dellink() calls gtp_encap_disable() twice.
So, remove one of them.
Fourth patch fixes usage of GFP_KERNEL.
GFP_KERNEL can not be used in RCU read critical section.
This patch make ipv4_pdp_add() to use GFP_ATOMIC instead of GFP_KERNEL.
Fifth patch fixes use-after-free in gtp_newlink().
gtp_newlink() uses gtp_net which would be destroyed by the __exit_net
routine.
So, gtp_newlink should not be called after the __exit_net routine.
Sixth patch adds missing error handling routine in gtp_encap_enable().
gtp_encap_enable() will fail, if invalid role value is sent from
user-space. if so, gtp_encap_enable() should execute error handling
routine.
Taehee Yoo (6):
gtp: fix suspicious RCU usage
gtp: fix use-after-free in gtp_encap_destroy()
gtp: remove duplicate code in gtp_dellink()
gtp: fix Illegal context switch in RCU read-side critical section.
gtp: fix use-after-free in gtp_newlink()
gtp: add missing gtp_encap_disable_sock() in gtp_encap_enable()
drivers/net/gtp.c | 37 +++++++++++++++++++++++++++++--------
1 file changed, 29 insertions(+), 8 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH net 1/6] gtp: fix suspicious RCU usage
From: Taehee Yoo @ 2019-07-02 15:20 UTC (permalink / raw)
To: davem, pablo, laforge, osmocom-net-gprs, netdev; +Cc: ap420073
gtp_encap_enable_socket() and gtp_encap_destroy() are not protected
by rcu_read_lock(). and it's not safe to write sk->sk_user_data.
This patch make these functions to use lock_sock() instead of
rcu_dereference_sk_user_data().
Test commands:
gtp-link add gtp1
Splat looks like:
[ 83.238315] =============================
[ 83.239127] WARNING: suspicious RCU usage
[ 83.239702] 5.2.0-rc6+ #49 Not tainted
[ 83.240268] -----------------------------
[ 83.241205] drivers/net/gtp.c:799 suspicious rcu_dereference_check() usage!
[ 83.243828]
[ 83.243828] other info that might help us debug this:
[ 83.243828]
[ 83.246325]
[ 83.246325] rcu_scheduler_active = 2, debug_locks = 1
[ 83.247314] 1 lock held by gtp-link/1008:
[ 83.248523] #0: 0000000017772c7f (rtnl_mutex){+.+.}, at: __rtnl_newlink+0x5f5/0x11b0
[ 83.251503]
[ 83.251503] stack backtrace:
[ 83.252173] CPU: 0 PID: 1008 Comm: gtp-link Not tainted 5.2.0-rc6+ #49
[ 83.253271] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[ 83.254562] Call Trace:
[ 83.254995] dump_stack+0x7c/0xbb
[ 83.255567] gtp_encap_enable_socket+0x2df/0x360 [gtp]
[ 83.256415] ? gtp_find_dev+0x1a0/0x1a0 [gtp]
[ 83.257161] ? memset+0x1f/0x40
[ 83.257843] gtp_newlink+0x90/0xa21 [gtp]
[ 83.258497] ? __netlink_ns_capable+0xc3/0xf0
[ 83.259260] __rtnl_newlink+0xb9f/0x11b0
[ 83.260022] ? rtnl_link_unregister+0x230/0x230
[ ... ]
Fixes: 1e3a3abd8b28 ("gtp: make GTP sockets in gtp_newlink optional")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
drivers/net/gtp.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index fc45b749db46..939da5442f65 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -289,12 +289,14 @@ static void gtp_encap_destroy(struct sock *sk)
{
struct gtp_dev *gtp;
- gtp = rcu_dereference_sk_user_data(sk);
+ lock_sock(sk);
+ gtp = sk->sk_user_data;
if (gtp) {
udp_sk(sk)->encap_type = 0;
rcu_assign_sk_user_data(sk, NULL);
sock_put(sk);
}
+ release_sock(sk);
}
static void gtp_encap_disable_sock(struct sock *sk)
@@ -796,7 +798,8 @@ static struct sock *gtp_encap_enable_socket(int fd, int type,
goto out_sock;
}
- if (rcu_dereference_sk_user_data(sock->sk)) {
+ lock_sock(sock->sk);
+ if (sock->sk->sk_user_data) {
sk = ERR_PTR(-EBUSY);
goto out_sock;
}
@@ -812,6 +815,7 @@ static struct sock *gtp_encap_enable_socket(int fd, int type,
setup_udp_tunnel_sock(sock_net(sock->sk), sock, &tuncfg);
out_sock:
+ release_sock(sock->sk);
sockfd_put(sock);
return sk;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 1/1] devlink: Add APIs to publish/unpublish the port parameters.
From: Sudarsana Reddy Kalluru @ 2019-07-02 15:20 UTC (permalink / raw)
To: davem; +Cc: netdev, mkalderon, aelior, jiri
The patch adds devlink interfaces for drivers to publish/unpublish the
devlink port parameters.
Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
include/net/devlink.h | 2 ++
net/core/devlink.c | 42 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 44 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 6c51e86..2e2d7fc 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -651,6 +651,8 @@ int devlink_port_params_register(struct devlink_port *devlink_port,
void devlink_port_params_unregister(struct devlink_port *devlink_port,
const struct devlink_param *params,
size_t params_count);
+void devlink_port_params_publish(struct devlink_port *devlink_port);
+void devlink_port_params_unpublish(struct devlink_port *ddevlink_port);
int devlink_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
union devlink_param_value *init_val);
int devlink_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 4baf716..c06c23f 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6378,6 +6378,48 @@ void devlink_port_params_unregister(struct devlink_port *devlink_port,
}
EXPORT_SYMBOL_GPL(devlink_port_params_unregister);
+/**
+ * devlink_port_params_publish - publish port configuration parameters
+ *
+ * @devlink_port: devlink port
+ *
+ * Publish previously registered port configuration parameters.
+ */
+void devlink_port_params_publish(struct devlink_port *devlink_port)
+{
+ struct devlink_param_item *param_item;
+
+ list_for_each_entry(param_item, &devlink_port->param_list, list) {
+ if (param_item->published)
+ continue;
+ param_item->published = true;
+ devlink_param_notify(devlink_port->devlink, devlink_port->index,
+ param_item, DEVLINK_CMD_PORT_PARAM_NEW);
+ }
+}
+EXPORT_SYMBOL_GPL(devlink_port_params_publish);
+
+/**
+ * devlink_port_params_unpublish - unpublish port configuration parameters
+ *
+ * @devlink_port: devlink port
+ *
+ * Unpublish previously registered port configuration parameters.
+ */
+void devlink_port_params_unpublish(struct devlink_port *devlink_port)
+{
+ struct devlink_param_item *param_item;
+
+ list_for_each_entry(param_item, &devlink_port->param_list, list) {
+ if (!param_item->published)
+ continue;
+ param_item->published = false;
+ devlink_param_notify(devlink_port->devlink, devlink_port->index,
+ param_item, DEVLINK_CMD_PORT_PARAM_DEL);
+ }
+}
+EXPORT_SYMBOL_GPL(devlink_port_params_unpublish);
+
static int
__devlink_param_driverinit_value_get(struct list_head *param_list, u32 param_id,
union devlink_param_value *init_val)
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] net: core: page_pool: add user refcnt and reintroduce page_pool_destroy
From: Ivan Khoronzhuk @ 2019-07-02 15:21 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, Ilias Apalodimas, grygorii.strashko, jakub.kicinski,
daniel, john.fastabend, ast, linux-kernel, linux-omap
In-Reply-To: <20190702171029.76c60538@carbon>
On Tue, Jul 02, 2019 at 05:10:29PM +0200, Jesper Dangaard Brouer wrote:
>On Tue, 2 Jul 2019 17:56:13 +0300
>Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>
>> On Tue, Jul 02, 2019 at 04:52:30PM +0200, Jesper Dangaard Brouer wrote:
>> >On Tue, 2 Jul 2019 17:44:27 +0300
>> >Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
>> >
>> >> On Tue, Jul 02, 2019 at 04:31:39PM +0200, Jesper Dangaard Brouer wrote:
>> >> >From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> >> >
>> >> >Jesper recently removed page_pool_destroy() (from driver invocation) and
>> >> >moved shutdown and free of page_pool into xdp_rxq_info_unreg(), in-order to
>> >> >handle in-flight packets/pages. This created an asymmetry in drivers
>> >> >create/destroy pairs.
>> >> >
>> >> >This patch add page_pool user refcnt and reintroduce page_pool_destroy.
>> >> >This serves two purposes, (1) simplify drivers error handling as driver now
>> >> >drivers always calls page_pool_destroy() and don't need to track if
>> >> >xdp_rxq_info_reg_mem_model() was unsuccessful. (2) allow special cases
>> >> >where a single RX-queue (with a single page_pool) provides packets for two
>> >> >net_device'es, and thus needs to register the same page_pool twice with two
>> >> >xdp_rxq_info structures.
>> >>
>> >> As I tend to use xdp level patch there is no more reason to mention (2) case
>> >> here. XDP patch serves it better and can prevent not only obj deletion but also
>> >> pool flush, so, this one patch I could better leave only for (1) case.
>> >
>> >I don't understand what you are saying.
>> >
>> >Do you approve this patch, or do you reject this patch?
>> >
>> It's not reject, it's proposition to use both, XDP and page pool patches,
>> each having its goal.
>
>Just to be clear, if you want this patch to get accepted you have to
>reply with your Signed-off-by (as I wrote).
>
>Maybe we should discuss it in another thread, about why you want two
>solutions to the same problem.
If it solves same problem I propose to reject this one and use this:
https://lkml.org/lkml/2019/7/2/651
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* [PATCH net 2/6] gtp: fix use-after-free in gtp_encap_destroy()
From: Taehee Yoo @ 2019-07-02 15:22 UTC (permalink / raw)
To: davem, pablo, laforge, osmocom-net-gprs, netdev; +Cc: ap420073
gtp_encap_destroy() is called twice.
1. When interface is deleted.
2. When udp socket is destroyed.
either gtp->sk0 or gtp->sk1u could be freed by sock_put() in
gtp_encap_destroy(). so, when gtp_encap_destroy() is called again,
it would uses freed sk pointer.
patch makes gtp_encap_destroy() to set either gtp->sk0 or gtp->sk1u to
null. in addition, both gtp->sk0 and gtp->sk1u pointer are protected
by rtnl_lock. so, rtnl_lock() is added.
Test command:
gtp-link add gtp1 &
killall gtp-link
ip link del gtp1
Splat looks like:
[ 83.182767] BUG: KASAN: use-after-free in __lock_acquire+0x3a20/0x46a0
[ 83.184128] Read of size 8 at addr ffff8880cc7d5360 by task ip/1008
[ 83.185567] CPU: 1 PID: 1008 Comm: ip Not tainted 5.2.0-rc6+ #50
[ 83.188469] Call Trace:
[ ... ]
[ 83.200126] lock_acquire+0x141/0x380
[ 83.200575] ? lock_sock_nested+0x3a/0xf0
[ 83.201069] _raw_spin_lock_bh+0x38/0x70
[ 83.201551] ? lock_sock_nested+0x3a/0xf0
[ 83.202044] lock_sock_nested+0x3a/0xf0
[ 83.202520] gtp_encap_destroy+0x18/0xe0 [gtp]
[ 83.203065] gtp_encap_disable.isra.14+0x13/0x50 [gtp]
[ 83.203687] gtp_dellink+0x56/0x170 [gtp]
[ 83.204190] rtnl_delete_link+0xb4/0x100
[ ... ]
[ 83.236513] Allocated by task 976:
[ 83.236925] save_stack+0x19/0x80
[ 83.237332] __kasan_kmalloc.constprop.3+0xa0/0xd0
[ 83.237894] kmem_cache_alloc+0xd8/0x280
[ 83.238360] sk_prot_alloc.isra.42+0x50/0x200
[ 83.238874] sk_alloc+0x32/0x940
[ 83.239264] inet_create+0x283/0xc20
[ 83.239684] __sock_create+0x2dd/0x540
[ 83.240136] __sys_socket+0xca/0x1a0
[ 83.240550] __x64_sys_socket+0x6f/0xb0
[ 83.240998] do_syscall_64+0x9c/0x450
[ 83.241466] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 83.242061]
[ 83.242249] Freed by task 0:
[ 83.242616] save_stack+0x19/0x80
[ 83.243013] __kasan_slab_free+0x111/0x150
[ 83.243498] kmem_cache_free+0x89/0x250
[ 83.244444] __sk_destruct+0x38f/0x5a0
[ 83.245366] rcu_core+0x7e9/0x1c20
[ 83.245766] __do_softirq+0x213/0x8fa
Fixes: 1e3a3abd8b28 ("gtp: make GTP sockets in gtp_newlink optional")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
drivers/net/gtp.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 939da5442f65..5101f8c3c99c 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -285,13 +285,17 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
return gtp_rx(pctx, skb, hdrlen, gtp->role);
}
-static void gtp_encap_destroy(struct sock *sk)
+static void __gtp_encap_destroy(struct sock *sk)
{
struct gtp_dev *gtp;
lock_sock(sk);
gtp = sk->sk_user_data;
if (gtp) {
+ if (gtp->sk0 == sk)
+ gtp->sk0 = NULL;
+ else
+ gtp->sk1u = NULL;
udp_sk(sk)->encap_type = 0;
rcu_assign_sk_user_data(sk, NULL);
sock_put(sk);
@@ -299,12 +303,19 @@ static void gtp_encap_destroy(struct sock *sk)
release_sock(sk);
}
+static void gtp_encap_destroy(struct sock *sk)
+{
+ rtnl_lock();
+ __gtp_encap_destroy(sk);
+ rtnl_unlock();
+}
+
static void gtp_encap_disable_sock(struct sock *sk)
{
if (!sk)
return;
- gtp_encap_destroy(sk);
+ __gtp_encap_destroy(sk);
}
static void gtp_encap_disable(struct gtp_dev *gtp)
@@ -1038,6 +1049,7 @@ static int gtp_genl_new_pdp(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;
}
+ rtnl_lock();
rcu_read_lock();
gtp = gtp_find_dev(sock_net(skb->sk), info->attrs);
@@ -1062,6 +1074,7 @@ static int gtp_genl_new_pdp(struct sk_buff *skb, struct genl_info *info)
out_unlock:
rcu_read_unlock();
+ rtnl_unlock();
return err;
}
--
2.17.1
^ permalink raw reply related
* [PATCH net 3/6] gtp: remove duplicate code in gtp_dellink()
From: Taehee Yoo @ 2019-07-02 15:22 UTC (permalink / raw)
To: davem, pablo, laforge, osmocom-net-gprs, netdev; +Cc: ap420073
gtp_encap_disable() in gtp_dellink() is unnecessary because it will be
called by unregister_netdevice().
unregister_netdevice() internally calls gtp_dev_uninit() by ->ndo_uninit().
And gtp_dev_uninit() calls gtp_encap_disable().
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
drivers/net/gtp.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 5101f8c3c99c..92ef777a757f 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -694,7 +694,6 @@ static void gtp_dellink(struct net_device *dev, struct list_head *head)
{
struct gtp_dev *gtp = netdev_priv(dev);
- gtp_encap_disable(gtp);
gtp_hashtable_free(gtp);
list_del_rcu(>p->list);
unregister_netdevice_queue(dev, head);
--
2.17.1
^ permalink raw reply related
* [PATCH net 4/6] gtp: fix Illegal context switch in RCU read-side critical section.
From: Taehee Yoo @ 2019-07-02 15:23 UTC (permalink / raw)
To: davem, pablo, laforge, osmocom-net-gprs, netdev; +Cc: ap420073
ipv4_pdp_add() is called in RCU read-side critical section.
So GFP_KERNEL should not be used in the function.
This patch make ipv4_pdp_add() to use GFP_ATOMIC instead of GFP_KERNEL.
Test commands:
gtp-link add gtp1 &
gtp-tunnel add gtp1 v1 100 200 1.1.1.1 2.2.2.2
Splat looks like:
[ 130.618881] =============================
[ 130.626382] WARNING: suspicious RCU usage
[ 130.626994] 5.2.0-rc6+ #50 Not tainted
[ 130.627622] -----------------------------
[ 130.628223] ./include/linux/rcupdate.h:266 Illegal context switch in RCU read-side critical section!
[ 130.629684]
[ 130.629684] other info that might help us debug this:
[ 130.629684]
[ 130.631022]
[ 130.631022] rcu_scheduler_active = 2, debug_locks = 1
[ 130.632136] 4 locks held by gtp-tunnel/1025:
[ 130.632925] #0: 000000002b93c8b7 (cb_lock){++++}, at: genl_rcv+0x15/0x40
[ 130.634159] #1: 00000000f17bc999 (genl_mutex){+.+.}, at: genl_rcv_msg+0xfb/0x130
[ 130.635487] #2: 00000000c644ed8e (rtnl_mutex){+.+.}, at: gtp_genl_new_pdp+0x18c/0x1150 [gtp]
[ 130.636936] #3: 0000000007a1cde7 (rcu_read_lock){....}, at: gtp_genl_new_pdp+0x187/0x1150 [gtp]
[ 130.638348]
[ 130.638348] stack backtrace:
[ 130.639062] CPU: 1 PID: 1025 Comm: gtp-tunnel Not tainted 5.2.0-rc6+ #50
[ 130.641318] Call Trace:
[ 130.641707] dump_stack+0x7c/0xbb
[ 130.642252] ___might_sleep+0x2c0/0x3b0
[ 130.642862] kmem_cache_alloc_trace+0x1cd/0x2b0
[ 130.643591] gtp_genl_new_pdp+0x6c5/0x1150 [gtp]
[ 130.644371] genl_family_rcv_msg+0x63a/0x1030
[ 130.645074] ? mutex_lock_io_nested+0x1090/0x1090
[ 130.645845] ? genl_unregister_family+0x630/0x630
[ 130.646592] ? debug_show_all_locks+0x2d0/0x2d0
[ 130.647293] ? check_flags.part.40+0x440/0x440
[ 130.648099] genl_rcv_msg+0xa3/0x130
[ ... ]
Fixes: 459aa660eb1d ("gtp: add initial driver for datapath of GPRS Tunneling Protocol (GTP-U)")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
drivers/net/gtp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 92ef777a757f..52f35cbeb1dc 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -959,7 +959,7 @@ static int ipv4_pdp_add(struct gtp_dev *gtp, struct sock *sk,
}
- pctx = kmalloc(sizeof(struct pdp_ctx), GFP_KERNEL);
+ pctx = kmalloc(sizeof(*pctx), GFP_ATOMIC);
if (pctx == NULL)
return -ENOMEM;
--
2.17.1
^ permalink raw reply related
* [PATCH net 5/6] gtp: fix use-after-free in gtp_newlink()
From: Taehee Yoo @ 2019-07-02 15:23 UTC (permalink / raw)
To: davem, pablo, laforge, osmocom-net-gprs, netdev; +Cc: ap420073
Current gtp_newlink() could be called after unregister_pernet_subsys().
gtp_newlink() uses gtp_net but it can be destroyed by
unregister_pernet_subsys().
So unregister_pernet_subsys() should be called after
rtnl_link_unregister().
Test commands:
#SHELL 1
while :
do
for i in {1..5}
do
./gtp-link add gtp$i &
done
killall gtp-link
done
#SHELL 2
while :
do
modprobe -rv gtp
done
Splat looks like:
[ 753.176631] BUG: KASAN: use-after-free in gtp_newlink+0x9b4/0xa5c [gtp]
[ 753.177722] Read of size 8 at addr ffff8880d48f2458 by task gtp-link/7126
[ 753.179082] CPU: 0 PID: 7126 Comm: gtp-link Tainted: G W 5.2.0-rc6+ #50
[ 753.185801] Call Trace:
[ 753.186264] dump_stack+0x7c/0xbb
[ 753.186863] ? gtp_newlink+0x9b4/0xa5c [gtp]
[ 753.187583] print_address_description+0xc7/0x240
[ 753.188382] ? gtp_newlink+0x9b4/0xa5c [gtp]
[ 753.189097] ? gtp_newlink+0x9b4/0xa5c [gtp]
[ 753.189846] __kasan_report+0x12a/0x16f
[ 753.190542] ? gtp_newlink+0x9b4/0xa5c [gtp]
[ 753.191298] kasan_report+0xe/0x20
[ 753.191893] gtp_newlink+0x9b4/0xa5c [gtp]
[ 753.192580] ? __netlink_ns_capable+0xc3/0xf0
[ 753.193370] __rtnl_newlink+0xb9f/0x11b0
[ ... ]
[ 753.241201] Allocated by task 7186:
[ 753.241844] save_stack+0x19/0x80
[ 753.242399] __kasan_kmalloc.constprop.3+0xa0/0xd0
[ 753.243192] __kmalloc+0x13e/0x300
[ 753.243764] ops_init+0xd6/0x350
[ 753.244314] register_pernet_operations+0x249/0x6f0
[ ... ]
[ 753.251770] Freed by task 7178:
[ 753.252288] save_stack+0x19/0x80
[ 753.252833] __kasan_slab_free+0x111/0x150
[ 753.253962] kfree+0xc7/0x280
[ 753.254509] ops_free_list.part.11+0x1c4/0x2d0
[ 753.255241] unregister_pernet_operations+0x262/0x390
[ ... ]
[ 753.285883] list_add corruption. next->prev should be prev (ffff8880d48f2458), but was ffff8880d497d878. (next.
[ 753.287241] ------------[ cut here ]------------
[ 753.287794] kernel BUG at lib/list_debug.c:25!
[ 753.288364] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
[ 753.289099] CPU: 0 PID: 7126 Comm: gtp-link Tainted: G B W 5.2.0-rc6+ #50
[ 753.291036] RIP: 0010:__list_add_valid+0x74/0xd0
[ 753.291589] Code: 48 39 da 75 27 48 39 f5 74 36 48 39 dd 74 31 48 83 c4 08 b8 01 00 00 00 5b 5d c3 48 89 d9 48b
[ 753.293779] RSP: 0018:ffff8880cae8f398 EFLAGS: 00010286
[ 753.294401] RAX: 0000000000000075 RBX: ffff8880d497d878 RCX: 0000000000000000
[ 753.296260] RDX: 0000000000000075 RSI: 0000000000000008 RDI: ffffed10195d1e69
[ 753.297070] RBP: ffff8880cd250ae0 R08: ffffed101b4bff21 R09: ffffed101b4bff21
[ 753.297899] R10: 0000000000000001 R11: ffffed101b4bff20 R12: ffff8880d497d878
[ 753.298703] R13: 0000000000000000 R14: ffff8880cd250ae0 R15: ffff8880d48f2458
[ 753.299564] FS: 00007f5f79805740(0000) GS:ffff8880da400000(0000) knlGS:0000000000000000
[ 753.300533] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 753.301231] CR2: 00007fe8c7ef4f10 CR3: 00000000b71a6006 CR4: 00000000000606f0
[ 753.302183] Call Trace:
[ 753.302530] gtp_newlink+0x5f6/0xa5c [gtp]
[ 753.303037] ? __netlink_ns_capable+0xc3/0xf0
[ 753.303576] __rtnl_newlink+0xb9f/0x11b0
[ 753.304092] ? rtnl_link_unregister+0x230/0x230
Fixes: 459aa660eb1d ("gtp: add initial driver for datapath of GPRS Tunneling Protocol (GTP-U)")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
drivers/net/gtp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 52f35cbeb1dc..b3ccac54e204 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -1376,9 +1376,9 @@ late_initcall(gtp_init);
static void __exit gtp_fini(void)
{
- unregister_pernet_subsys(>p_net_ops);
genl_unregister_family(>p_genl_family);
rtnl_link_unregister(>p_link_ops);
+ unregister_pernet_subsys(>p_net_ops);
pr_info("GTP module unloaded\n");
}
--
2.17.1
^ permalink raw reply related
* [PATCH net 6/6] gtp: add missing gtp_encap_disable_sock() in gtp_encap_enable()
From: Taehee Yoo @ 2019-07-02 15:24 UTC (permalink / raw)
To: davem, pablo, laforge, osmocom-net-gprs, netdev; +Cc: ap420073
If an invalid role is sent from user space, gtp_encap_enable() will fail.
Then, it should call gtp_encap_disable_sock() but current code doesn't.
It makes memory leak.
Fixes: 91ed81f9abc7 ("gtp: support SGSN-side tunnels")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
drivers/net/gtp.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index b3ccac54e204..ecfe26215935 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -857,8 +857,13 @@ static int gtp_encap_enable(struct gtp_dev *gtp, struct nlattr *data[])
if (data[IFLA_GTP_ROLE]) {
role = nla_get_u32(data[IFLA_GTP_ROLE]);
- if (role > GTP_ROLE_SGSN)
+ if (role > GTP_ROLE_SGSN) {
+ if (sk0)
+ gtp_encap_disable_sock(sk0);
+ if (sk1u)
+ gtp_encap_disable_sock(sk1u);
return -EINVAL;
+ }
}
gtp->sk0 = sk0;
--
2.17.1
^ permalink raw reply related
* !!!Klientskie bazy. Email: prodawez@armyspy.com Uznajte podrobnee.
From: netdev @ 2019-07-02 3:23 UTC (permalink / raw)
To: netdev
!!!Klientskie bazy. Email: prodawez@armyspy.com Uznajte podrobnee.
^ permalink raw reply
* Re: [PATCH] User mode linux bump maximum MTU tuntap interface [RESAND]
From: Anton Ivanov @ 2019-07-02 14:47 UTC (permalink / raw)
To: Richard Weinberger,
Алексей
Cc: netdev, linux-um
In-Reply-To: <CAFLxGvytDC1TFdT0m9vvijz_93B8TziWURcR-3mskWB-7TzFag@mail.gmail.com>
On 02/07/2019 15:40, Richard Weinberger wrote:
> CC'ing um folks.
>
> On Tue, Jul 2, 2019 at 3:01 PM Алексей <ne-vlezay80@yandex.ru> wrote:
>>
>> Hello, the parameter ETH_MAX_PACKET limited to 1500 bytes is the not
>> support jumbo frames.
>>
>> This patch change ETH_MAX_PACKET the 65535 bytes to jumbo frame support
>> with user mode linux tuntap driver.
>>
>>
>> PATCH:
>>
>> -------------------
>>
>>
>> diff -ruNP ../linux_orig/linux-5.1/arch/um/include/shared/net_user.h
>> ./arch/um/include/shared/net_user.h
>> --- a/arch/um/include/shared/net_user.h 2019-05-06 00:42:58.000000000
>> +0000
>> +++ b/arch/um/include/shared/net_user.h 2019-07-02 07:14:13.593333356
>> +0000
>> @@ -9,7 +9,7 @@
>> #define ETH_ADDR_LEN (6)
>> #define ETH_HEADER_ETHERTAP (16)
>> #define ETH_HEADER_OTHER (26) /* 14 for ethernet + VLAN + MPLS for
>> crazy people */
>> -#define ETH_MAX_PACKET (1500)
>> +#define ETH_MAX_PACKET (65535)
>>
>> #define UML_NET_VERSION (4)
>>
>> -------------------
>>
>>
>
>
The vector version for tap already allows mtu > 1500. It does not have a
check to limit it to 65535 max though and it should.
I will add this one to the queue of stuff for the network drivers. IMHO
we should start migrating some of the older ones to vector IO.
--
Anton R. Ivanov
https://www.kot-begemot.co.uk/
^ permalink raw reply
* Re: [PATCH] bpf, libbpf: Smatch: Fix potential NULL pointer dereference
From: Yonghong Song @ 2019-07-02 15:34 UTC (permalink / raw)
To: Leo Yan, Alexei Starovoitov, Daniel Borkmann, Martin Lau,
Song Liu, netdev@vger.kernel.org, bpf@vger.kernel.org,
linux-kernel@vger.kernel.org, Dan Carpenter
In-Reply-To: <20190702102531.23512-1-leo.yan@linaro.org>
On 7/2/19 3:25 AM, Leo Yan wrote:
> Based on the following report from Smatch, fix the potential
> NULL pointer dereference check.
>
> tools/lib/bpf/libbpf.c:3493
> bpf_prog_load_xattr() warn: variable dereferenced before check 'attr'
> (see line 3483)
>
> 3479 int bpf_prog_load_xattr(const struct bpf_prog_load_attr *attr,
> 3480 struct bpf_object **pobj, int *prog_fd)
> 3481 {
> 3482 struct bpf_object_open_attr open_attr = {
> 3483 .file = attr->file,
> 3484 .prog_type = attr->prog_type,
> ^^^^^^
> 3485 };
>
> At the head of function, it directly access 'attr' without checking if
> it's NULL pointer. This patch moves the values assignment after
> validating 'attr' and 'attr->file'.
>
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply
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