* Re: [PATCH net] udp: Allow kernel service to avoid udp socket rx queue
From: Paolo Abeni @ 2018-10-04 9:08 UTC (permalink / raw)
To: David Howells, netdev; +Cc: linux-afs, linux-kernel
In-Reply-To: <153859250219.15389.11970533498295122206.stgit@warthog.procyon.org.uk>
Hi,
On Wed, 2018-10-03 at 19:48 +0100, David Howells wrote:
> There's a problem somewhere skb_recv_udp() that doesn't decrease the
> sk_rmem_alloc counter when a packet is extracted from the receive queue by
> a kernel service.
If this is the case, it's really bad and need an explicit fix. However
it looks like sk_rmem_alloc is reclaimed by skb_recv_udp(), as it ends-
up calling udp_rmem_release() on succesfull dequeue.
udp_rmem_release() can delay sk_rmem_alloc reclaiming is the rx queue
is almost empty, due to commit
6b229cf77d683f634f0edd876c6d1015402303ad. Anyhow I don't see either as
that may affect this scenario: if only 1/4 of the maxium receive buffer
size is used, the next packet should always be able to land into the rx
queue, with the default sk_rcvbuf and every reasonable truesize.
> Further, there doesn't seem any point in having the socket buffer being
> added to the UDP socket's rx queue since the rxrpc's data_ready handler
> takes it straight back out again (more or less, there seem to be occasional
> hold ups there).
I really would really try to avoid adding another indirect call in the
data-path, unless strictly needed (to avoid more RETPOLINE overhead for
all other use-case). If skipping altogether the enqueuing makes sense
(I guess so, mostily for performance reasons), I *think* you can use
the already existing encap_rcv hook, initializing it to the rxrpc input
function, and updating such function to pull the udp header and ev.
initializing the pktinfo, if needed. Please see e.g. l2tp usage.
> Putting in some tracepoints show a significant delay occurring between packets
> coming in and thence being delivered to rxrpc:
>
> <idle>-0 [001] ..s2 67.631844: net_rtl8169_napi_rx: enp3s0 skb=07db0a32
> ...
> <idle>-0 [001] ..s4 68.292778: rxrpc_rx_packet: d5ce8d37:bdb93c60:00000002:09c7 00000006 00000000 02 20 ACK 660967981 skb=07db0a32
>
> The "660967981" is the time difference in nanoseconds between the sk_buff
> timestamp and the current time. It seems to match the time elapsed between
> the two trace lines reasonably well. I've seen anything up to about 4s.
Can you please provide more data? specifically can you please add:
* a perf probe in rxrpc_data_ready() just after skb_recv_udp()
reporting the sk->sk_rmem_alloc and skb->truesize
* a perf probe in __udp_enqueue_schedule_skb() just before the 'if
(rmem > sk->sk_rcvbuf)' test reporting again sk->sk_rmem_alloc, skb-
>truesize and sk->sk_rcvbuf
And then provide the perf record -g -e ... /perf script output?
Thanks,
Paolo
^ permalink raw reply
* Re: [PATCH] net: wireless: iwlegacy: Fix possible data races in il4965_send_rxon_assoc()
From: Jia-Ju Bai @ 2018-10-04 8:52 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: kvalo, davem, linux-wireless, netdev, linux-kernel
In-Reply-To: <20181004075914.GB20523@redhat.com>
Thanks for your reply :)
On 2018/10/4 15:59, Stanislaw Gruszka wrote:
> On Wed, Oct 03, 2018 at 10:07:45PM +0800, Jia-Ju Bai wrote:
>> These possible races are detected by a runtime testing.
>> To fix these races, the mutex lock is used in il4965_send_rxon_assoc()
>> to protect the data.
> Really ? I'm surprised by that, see below.
My runtime testing shows that il4965_send_rxon_assoc() and
il4965_configure_filter() are concurrently executed.
But after seeing your reply, I need to carefully check whether my
runtime testing is right, because I think you are right.
In fact, I only monitored the iwl4965 driver, but did not monitor the
iwlegacy driver, so I will do the testing again with monitoring the
lwlegacy driver.
>
>> @@ -1297,6 +1297,7 @@ il4965_send_rxon_assoc(struct il_priv *il)
>> const struct il_rxon_cmd *rxon1 = &il->staging;
>> const struct il_rxon_cmd *rxon2 = &il->active;
>>
>> + mutex_lock(&il->mutex);
>> if (rxon1->flags == rxon2->flags &&
> For 4965 driver il4965_send_rxon_assoc() is only called by
> il_mac_bss_info_changed() and il4965_commit_rxon().
>
> il_mac_bss_info_changed() acquire il->mutex and
> callers of il4965_commit_rxon() acquire il->mutex
> (but I did not check all of them).
>
> So I wonder how this patch did not cause the deadlock ?
Oh, sorry, anyway, my patch will cause double locks...
>
> Anyway what can be done is adding:
>
> lockdep_assert_held(&il->mutex);
>
> il4965_commit_rxon() to check if we hold the mutex.
I agree.
Best wishes,
Jia-Ju Bai
^ permalink raw reply
* KASAN: use-after-free Read in sctp_id2assoc
From: syzbot @ 2018-10-04 8:48 UTC (permalink / raw)
To: davem, linux-kernel, linux-sctp, marcelo.leitner, netdev, nhorman,
syzkaller-bugs, vyasevich
Hello,
syzbot found the following crash on:
HEAD commit: 4e6d47206c32 tls: Add support for inplace records encryption
git tree: net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=13834b81400000
kernel config: https://syzkaller.appspot.com/x/.config?x=e569aa5632ebd436
dashboard link: https://syzkaller.appspot.com/bug?extid=c7dd55d7aec49d48e49a
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+c7dd55d7aec49d48e49a@syzkaller.appspotmail.com
netlink: 'syz-executor1': attribute type 1 has an invalid length.
==================================================================
BUG: KASAN: use-after-free in sctp_id2assoc+0x3a7/0x3e0
net/sctp/socket.c:276
Read of size 8 at addr ffff880195b3eb20 by task syz-executor2/15454
CPU: 1 PID: 15454 Comm: syz-executor2 Not tainted 4.19.0-rc5+ #242
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113
print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412
__asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
sctp_id2assoc+0x3a7/0x3e0 net/sctp/socket.c:276
sctp_do_peeloff+0x88/0x530 net/sctp/socket.c:5327
sctp_getsockopt_peeloff_common.isra.24+0xb5/0x2f0 net/sctp/socket.c:5376
sctp_getsockopt_peeloff_flags net/sctp/socket.c:5452 [inline]
sctp_getsockopt+0x17f1/0x7c78 net/sctp/socket.c:7474
sock_common_getsockopt+0x9a/0xe0 net/core/sock.c:2997
__sys_getsockopt+0x1ad/0x390 net/socket.c:1939
__do_sys_getsockopt net/socket.c:1950 [inline]
__se_sys_getsockopt net/socket.c:1947 [inline]
__x64_sys_getsockopt+0xbe/0x150 net/socket.c:1947
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x457579
Code: 1d b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 eb b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f86d7f8bc78 EFLAGS: 00000246 ORIG_RAX: 0000000000000037
RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 0000000000457579
RDX: 000000000000007a RSI: 0000000000000084 RDI: 0000000000000007
RBP: 000000000072c040 R08: 00000000200001c0 R09: 0000000000000000
R10: 0000000020000080 R11: 0000000000000246 R12: 00007f86d7f8c6d4
R13: 00000000004c79b8 R14: 00000000004cdb70 R15: 00000000ffffffff
Allocated by task 15404:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553
kmem_cache_alloc_trace+0x152/0x750 mm/slab.c:3620
kmalloc include/linux/slab.h:513 [inline]
kzalloc include/linux/slab.h:707 [inline]
sctp_association_new+0x12b/0x22a0 net/sctp/associola.c:311
__sctp_connect+0x6ad/0xda0 net/sctp/socket.c:1209
__sctp_setsockopt_connectx+0x134/0x190 net/sctp/socket.c:1368
sctp_setsockopt_connectx net/sctp/socket.c:1400 [inline]
sctp_setsockopt+0x18e2/0x6da0 net/sctp/socket.c:4359
sock_common_setsockopt+0x9a/0xe0 net/core/sock.c:3038
__sys_setsockopt+0x1ba/0x3c0 net/socket.c:1902
__do_sys_setsockopt net/socket.c:1913 [inline]
__se_sys_setsockopt net/socket.c:1910 [inline]
__x64_sys_setsockopt+0xbe/0x150 net/socket.c:1910
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 15404:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
__kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521
kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
__cache_free mm/slab.c:3498 [inline]
kfree+0xcf/0x230 mm/slab.c:3813
sctp_association_destroy net/sctp/associola.c:437 [inline]
sctp_association_put+0x264/0x350 net/sctp/associola.c:885
sctp_wait_for_connect+0x444/0x640 net/sctp/socket.c:8677
__sctp_connect+0xbb3/0xda0 net/sctp/socket.c:1260
__sctp_setsockopt_connectx+0x134/0x190 net/sctp/socket.c:1368
sctp_setsockopt_connectx net/sctp/socket.c:1400 [inline]
sctp_setsockopt+0x18e2/0x6da0 net/sctp/socket.c:4359
sock_common_setsockopt+0x9a/0xe0 net/core/sock.c:3038
__sys_setsockopt+0x1ba/0x3c0 net/socket.c:1902
__do_sys_setsockopt net/socket.c:1913 [inline]
__se_sys_setsockopt net/socket.c:1910 [inline]
__x64_sys_setsockopt+0xbe/0x150 net/socket.c:1910
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
The buggy address belongs to the object at ffff880195b3eb00
which belongs to the cache kmalloc-4096 of size 4096
The buggy address is located 32 bytes inside of
4096-byte region [ffff880195b3eb00, ffff880195b3fb00)
The buggy address belongs to the page:
page:ffffea000656cf80 count:1 mapcount:0 mapping:ffff8801da800dc0 index:0x0
compound_mapcount: 0
flags: 0x2fffc0000008100(slab|head)
raw: 02fffc0000008100 ffffea0006e66188 ffffea00065b5d08 ffff8801da800dc0
raw: 0000000000000000 ffff880195b3eb00 0000000100000001 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff880195b3ea00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff880195b3ea80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff880195b3eb00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff880195b3eb80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff880195b3ec00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
^ permalink raw reply
* [PATCH net-next] r8169: Add some tracepoints for watching rx/tx events
From: David Howells @ 2018-10-04 8:31 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel, nic_swsd
Add some tracepoints into the Realtek r8169 driver to watch events involved
in the reception and transmission of packets, including:
(1) Interrupts occurring. The device status is logged.
(2) NAPI polling. The device status is logged.
(3) Packet received and about to be handed off. The sk_buff address is
logged.
(4) Packet queued for transmission and device poked. The sk_buff address
is logged along with the revised Tx ring indices.
(5) Packet removed from Tx ring after transmission. The sk_buff address
is logged along with the Tx ring slot number.
Signed-off-by: David Howells <dhowells@redhat.com>
---
drivers/net/ethernet/realtek/r8169.c | 9 ++
include/trace/events/net_rtl8169.h | 125 ++++++++++++++++++++++++++++++++++
2 files changed, 134 insertions(+)
create mode 100644 include/trace/events/net_rtl8169.h
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 9a5e2969df61..1d4d3d953628 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -31,6 +31,9 @@
#include <linux/ipv6.h>
#include <net/ip6_checksum.h>
+#define CREATE_TRACE_POINTS
+#include <trace/events/net_rtl8169.h>
+
#define MODULENAME "r8169"
#define FIRMWARE_8168D_1 "rtl_nic/rtl8168d-1.fw"
@@ -6192,6 +6195,7 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb,
RTL_W8(tp, TxPoll, NPQ);
mmiowb();
+ trace_net_rtl8169_tx(tp->dev, tp->dirty_tx, tp->cur_tx, skb);
if (!TX_FRAGS_READY_FOR(tp, MAX_SKB_FRAGS)) {
/* Avoid wrongly optimistic queue wake-up: rtl_tx thread must
@@ -6297,6 +6301,7 @@ static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp)
rtl8169_unmap_tx_skb(tp_to_dev(tp), tx_skb,
tp->TxDescArray + entry);
if (status & LastFrag) {
+ trace_net_rtl8169_tx_done(dev, dirty_tx, tx_skb->skb);
u64_stats_update_begin(&tp->tx_stats.syncp);
tp->tx_stats.packets++;
tp->tx_stats.bytes += tx_skb->skb->len;
@@ -6447,6 +6452,7 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp, u32 budget
if (skb->pkt_type == PACKET_MULTICAST)
dev->stats.multicast++;
+ trace_net_rtl8169_rx(dev, skb);
napi_gro_receive(&tp->napi, skb);
u64_stats_update_begin(&tp->rx_stats.syncp);
@@ -6473,6 +6479,8 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
if (status == 0xffff || !(status & (RTL_EVENT_NAPI | tp->event_slow)))
return IRQ_NONE;
+ trace_net_rtl8169_interrupt(tp->napi.dev, status);
+
rtl_irq_disable(tp);
napi_schedule_irqoff(&tp->napi);
@@ -6554,6 +6562,7 @@ static int rtl8169_poll(struct napi_struct *napi, int budget)
status = rtl_get_events(tp);
rtl_ack_events(tp, status & ~tp->event_slow);
+ trace_net_rtl8169_poll(dev, status);
if (status & RTL_EVENT_NAPI_RX)
work_done = rtl_rx(dev, tp, (u32) budget);
diff --git a/include/trace/events/net_rtl8169.h b/include/trace/events/net_rtl8169.h
new file mode 100644
index 000000000000..15b3984c3939
--- /dev/null
+++ b/include/trace/events/net_rtl8169.h
@@ -0,0 +1,125 @@
+/* Realtek RTL8169 tracepoints
+ *
+ * Copyright (C) 2018 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM net_rtl8169
+
+#if !defined(_TRACE_NET_RTL8169_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_NET_RTL8169_H
+
+#include <linux/tracepoint.h>
+#include <linux/errqueue.h>
+
+
+TRACE_EVENT(net_rtl8169_interrupt,
+ TP_PROTO(struct net_device *netdev, u16 status),
+
+ TP_ARGS(netdev, status),
+
+ TP_STRUCT__entry(
+ __field(u16, status )
+ __array(char, name, IFNAMSIZ )
+ ),
+
+ TP_fast_assign(
+ __entry->status = status;
+ memcpy(__entry->name, netdev->name, IFNAMSIZ);
+ ),
+
+ TP_printk("%s st=%x", __entry->name, __entry->status)
+ );
+
+TRACE_EVENT(net_rtl8169_poll,
+ TP_PROTO(struct net_device *netdev, u16 status),
+
+ TP_ARGS(netdev, status),
+
+ TP_STRUCT__entry(
+ __field(u16, status )
+ __array(char, name, IFNAMSIZ )
+ ),
+
+ TP_fast_assign(
+ __entry->status = status;
+ memcpy(__entry->name, netdev->name, IFNAMSIZ);
+ ),
+
+ TP_printk("%s st=%x", __entry->name, __entry->status)
+ );
+
+TRACE_EVENT(net_rtl8169_rx,
+ TP_PROTO(struct net_device *netdev, struct sk_buff *skb),
+
+ TP_ARGS(netdev, skb),
+
+ TP_STRUCT__entry(
+ __field(struct sk_buff *, skb )
+ __array(char, name, IFNAMSIZ )
+ ),
+
+ TP_fast_assign(
+ __entry->skb = skb;
+ memcpy(__entry->name, netdev->name, IFNAMSIZ);
+ ),
+
+ TP_printk("%s skb=%p", __entry->name, __entry->skb)
+ );
+
+TRACE_EVENT(net_rtl8169_tx_done,
+ TP_PROTO(struct net_device *netdev, unsigned int dirty_tx,
+ struct sk_buff *skb),
+
+ TP_ARGS(netdev, dirty_tx, skb),
+
+ TP_STRUCT__entry(
+ __field(struct sk_buff *, skb)
+ __field(unsigned int, dirty_tx)
+ __array(char, name, IFNAMSIZ )
+ ),
+
+ TP_fast_assign(
+ __entry->skb = skb;
+ __entry->dirty_tx = dirty_tx;
+ memcpy(__entry->name, netdev->name, IFNAMSIZ);
+ ),
+
+ TP_printk("%s p=%u skb=%p",
+ __entry->name, __entry->dirty_tx, __entry->skb)
+ );
+
+TRACE_EVENT(net_rtl8169_tx,
+ TP_PROTO(struct net_device *netdev, unsigned int dirty_tx,
+ unsigned int cur_tx, struct sk_buff *skb),
+
+ TP_ARGS(netdev, dirty_tx, cur_tx, skb),
+
+ TP_STRUCT__entry(
+ __field(struct sk_buff *, skb )
+ __field(unsigned int, dirty_tx )
+ __field(unsigned int, cur_tx )
+ __array(char, name, IFNAMSIZ )
+ ),
+
+ TP_fast_assign(
+ __entry->skb = skb;
+ __entry->dirty_tx = dirty_tx;
+ __entry->cur_tx = cur_tx;
+ memcpy(__entry->name, netdev->name, IFNAMSIZ);
+ ),
+
+ TP_printk("%s p=%u-%u skb=%p",
+ __entry->name, __entry->dirty_tx, __entry->cur_tx,
+ __entry->skb)
+ );
+
+#endif /* _TRACE_NET_RTL8169_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
^ permalink raw reply related
* [PATCH net-next] udp: Add tracepoints to monitor skbs going in and out of a UDP socket
From: David Howells @ 2018-10-04 8:30 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
Add three tracepoints to the UDP/UDP6 code to trace three aspects of
sk_buff handling:
(1) UDP packets that are about to be handed off to the IP layer to be
transmitted.
(2) UDP packets that have just arrived at the socket.
(3) UDP packets that are about to be queued on the UDP socket's receive
queue, and displaying the sk_rmem_alloc counter.
Signed-off-by: David Howells <dhowells@redhat.com>
---
include/trace/events/udp.h | 53 ++++++++++++++++++++++++++++++++++++++++++++
net/ipv4/udp.c | 3 ++
net/ipv6/udp.c | 3 ++
3 files changed, 59 insertions(+)
diff --git a/include/trace/events/udp.h b/include/trace/events/udp.h
index 336fe272889f..ff6b3ab5a65e 100644
--- a/include/trace/events/udp.h
+++ b/include/trace/events/udp.h
@@ -27,6 +27,59 @@ TRACE_EVENT(udp_fail_queue_rcv_skb,
TP_printk("rc=%d port=%hu", __entry->rc, __entry->lport)
);
+TRACE_EVENT(udp_tx,
+
+ TP_PROTO(struct sk_buff *skb),
+
+ TP_ARGS(skb),
+
+ TP_STRUCT__entry(
+ __field(struct sk_buff *, skb)
+ ),
+
+ TP_fast_assign(
+ __entry->skb = skb;
+ ),
+
+ TP_printk("skb=%p", __entry->skb)
+);
+
+TRACE_EVENT(udp_rcv,
+
+ TP_PROTO(struct sk_buff *skb),
+
+ TP_ARGS(skb),
+
+ TP_STRUCT__entry(
+ __field(struct sk_buff *, skb)
+ ),
+
+ TP_fast_assign(
+ __entry->skb = skb;
+ ),
+
+ TP_printk("skb=%p", __entry->skb)
+);
+
+TRACE_EVENT(udp_enqueue,
+
+ TP_PROTO(struct sk_buff *skb, int rmem),
+
+ TP_ARGS(skb, rmem),
+
+ TP_STRUCT__entry(
+ __field(struct sk_buff *, skb)
+ __field(int, rmem)
+ ),
+
+ TP_fast_assign(
+ __entry->skb = skb;
+ __entry->rmem = rmem;
+ ),
+
+ TP_printk("skb=%p rm=%d", __entry->skb, __entry->rmem)
+ );
+
#endif /* _TRACE_UDP_H */
/* This part must be outside protection */
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index f93bc444f631..67afbd0ac0a7 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -826,6 +826,7 @@ static int udp_send_skb(struct sk_buff *skb, struct flowi4 *fl4,
uh->check = CSUM_MANGLED_0;
send:
+ trace_udp_tx(skb);
err = ip_send_skb(sock_net(sk), skb);
if (err) {
if (err == -ENOBUFS && !inet->recverr) {
@@ -1363,6 +1364,7 @@ static int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
rmem = atomic_read(&sk->sk_rmem_alloc);
if (rmem > sk->sk_rcvbuf)
goto drop;
+ trace_udp_enqueue(skb, rmem);
/* Under mem pressure, it might be helpful to help udp_recvmsg()
* having linear skbs :
@@ -2395,6 +2397,7 @@ int udp_v4_early_demux(struct sk_buff *skb)
int udp_rcv(struct sk_buff *skb)
{
+ trace_udp_rcv(skb);
return __udp4_lib_rcv(skb, &udp_table, IPPROTO_UDP);
}
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 4421fbf0a7a4..baec8abc1409 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -54,6 +54,7 @@
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
#include <trace/events/skb.h>
+#include <trace/events/udp.h>
#include "udp_impl.h"
static bool udp6_lib_exact_dif_match(struct net *net, struct sk_buff *skb)
@@ -950,6 +951,7 @@ static void udp_v6_early_demux(struct sk_buff *skb)
static __inline__ int udpv6_rcv(struct sk_buff *skb)
{
+ trace_udp_rcv(skb);
return __udp6_lib_rcv(skb, &udp_table, IPPROTO_UDP);
}
@@ -1093,6 +1095,7 @@ static int udp_v6_send_skb(struct sk_buff *skb, struct flowi6 *fl6,
uh->check = CSUM_MANGLED_0;
send:
+ trace_udp_tx(skb);
err = ip6_send_skb(skb);
if (err) {
if (err == -ENOBUFS && !inet6_sk(sk)->recverr) {
^ permalink raw reply related
* RE: [EXT] [PATCH net] net: mvpp2: Extract the correct ethtype from the skb for tx csum offload
From: Yan Markman @ 2018-10-04 8:08 UTC (permalink / raw)
To: Maxime Chevallier, davem@davemloft.net
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Antoine Tenart, thomas.petazzoni@bootlin.com,
gregory.clement@bootlin.com, miquel.raynal@bootlin.com,
Nadav Haklai, Stefan Chulski, mw@semihalf.com
In-Reply-To: <20181004080308.7687-1-maxime.chevallier@bootlin.com>
I'm not sure that kind of ntohs() using is good for the Performance.
htons(ETH_P_IP) is CONSTANT swapped Big/LittleEndian by COMPILER whilst the
ntohs(vlan_get_protocol(skb)) is run-time short/byte-swap for every packet.
Yan Markman
-----Original Message-----
From: Maxime Chevallier [mailto:maxime.chevallier@bootlin.com]
Sent: Thursday, October 04, 2018 11:03 AM
To: davem@davemloft.net
Cc: Maxime Chevallier <maxime.chevallier@bootlin.com>; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Antoine Tenart <antoine.tenart@bootlin.com>; thomas.petazzoni@bootlin.com; gregory.clement@bootlin.com; miquel.raynal@bootlin.com; Nadav Haklai <nadavh@marvell.com>; Stefan Chulski <stefanc@marvell.com>; Yan Markman <ymarkman@marvell.com>; mw@semihalf.com
Subject: [EXT] [PATCH net] net: mvpp2: Extract the correct ethtype from the skb for tx csum offload
External Email
----------------------------------------------------------------------
When offloading the L3 and L4 csum computation on TX, we need to extract the l3_proto from the ethtype, independently of the presence of a vlan tag.
The actual driver uses skb->protocol as-is, resulting in packets with the wrong L4 checksum being sent when there's a vlan tag in the packet header and checksum offloading is enabled.
This commit makes use of vlan_protocol_get() to get the correct ethtype regardless the presence of a vlan tag.
Fixes: 3f51850 ("ethernet: Add new driver for Marvell Armada 375 network unit")
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 38cc01beea79..4bd5b251e3c7 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -1737,7 +1737,7 @@ static u32 mvpp2_txq_desc_csum(int l3_offs, int l3_proto,
command |= (ip_hdr_len << MVPP2_TXD_IP_HLEN_SHIFT);
command |= MVPP2_TXD_IP_CSUM_DISABLE;
- if (l3_proto == htons(ETH_P_IP)) {
+ if (l3_proto == ETH_P_IP) {
command &= ~MVPP2_TXD_IP_CSUM_DISABLE; /* enable IPv4 csum */
command &= ~MVPP2_TXD_L3_IP6; /* enable IPv4 */
} else {
@@ -2600,14 +2600,15 @@ static u32 mvpp2_skb_tx_csum(struct mvpp2_port *port, struct sk_buff *skb)
if (skb->ip_summed == CHECKSUM_PARTIAL) {
int ip_hdr_len = 0;
u8 l4_proto;
+ int l3_proto = ntohs(vlan_get_protocol(skb));
- if (skb->protocol == htons(ETH_P_IP)) {
+ if (l3_proto == ETH_P_IP) {
struct iphdr *ip4h = ip_hdr(skb);
/* Calculate IPv4 checksum and L4 checksum */
ip_hdr_len = ip4h->ihl;
l4_proto = ip4h->protocol;
- } else if (skb->protocol == htons(ETH_P_IPV6)) {
+ } else if (l3_proto == ETH_P_IPV6) {
struct ipv6hdr *ip6h = ipv6_hdr(skb);
/* Read l4_protocol from one of IPv6 extra headers */ @@ -2619,7 +2620,7 @@ static u32 mvpp2_skb_tx_csum(struct mvpp2_port *port, struct sk_buff *skb)
}
return mvpp2_txq_desc_csum(skb_network_offset(skb),
- skb->protocol, ip_hdr_len, l4_proto);
+ l3_proto, ip_hdr_len, l4_proto);
}
return MVPP2_TXD_L4_CSUM_NOT | MVPP2_TXD_IP_CSUM_DISABLE;
--
2.11.0
^ permalink raw reply related
* [PATCH net] net: mvpp2: Extract the correct ethtype from the skb for tx csum offload
From: Maxime Chevallier @ 2018-10-04 8:03 UTC (permalink / raw)
To: davem
Cc: Maxime Chevallier, netdev, linux-kernel, Antoine Tenart,
thomas.petazzoni, gregory.clement, miquel.raynal, nadavh, stefanc,
ymarkman, mw
When offloading the L3 and L4 csum computation on TX, we need to extract
the l3_proto from the ethtype, independently of the presence of a vlan
tag.
The actual driver uses skb->protocol as-is, resulting in packets with
the wrong L4 checksum being sent when there's a vlan tag in the packet
header and checksum offloading is enabled.
This commit makes use of vlan_protocol_get() to get the correct ethtype
regardless the presence of a vlan tag.
Fixes: 3f51850 ("ethernet: Add new driver for Marvell Armada 375 network unit")
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 38cc01beea79..4bd5b251e3c7 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -1737,7 +1737,7 @@ static u32 mvpp2_txq_desc_csum(int l3_offs, int l3_proto,
command |= (ip_hdr_len << MVPP2_TXD_IP_HLEN_SHIFT);
command |= MVPP2_TXD_IP_CSUM_DISABLE;
- if (l3_proto == htons(ETH_P_IP)) {
+ if (l3_proto == ETH_P_IP) {
command &= ~MVPP2_TXD_IP_CSUM_DISABLE; /* enable IPv4 csum */
command &= ~MVPP2_TXD_L3_IP6; /* enable IPv4 */
} else {
@@ -2600,14 +2600,15 @@ static u32 mvpp2_skb_tx_csum(struct mvpp2_port *port, struct sk_buff *skb)
if (skb->ip_summed == CHECKSUM_PARTIAL) {
int ip_hdr_len = 0;
u8 l4_proto;
+ int l3_proto = ntohs(vlan_get_protocol(skb));
- if (skb->protocol == htons(ETH_P_IP)) {
+ if (l3_proto == ETH_P_IP) {
struct iphdr *ip4h = ip_hdr(skb);
/* Calculate IPv4 checksum and L4 checksum */
ip_hdr_len = ip4h->ihl;
l4_proto = ip4h->protocol;
- } else if (skb->protocol == htons(ETH_P_IPV6)) {
+ } else if (l3_proto == ETH_P_IPV6) {
struct ipv6hdr *ip6h = ipv6_hdr(skb);
/* Read l4_protocol from one of IPv6 extra headers */
@@ -2619,7 +2620,7 @@ static u32 mvpp2_skb_tx_csum(struct mvpp2_port *port, struct sk_buff *skb)
}
return mvpp2_txq_desc_csum(skb_network_offset(skb),
- skb->protocol, ip_hdr_len, l4_proto);
+ l3_proto, ip_hdr_len, l4_proto);
}
return MVPP2_TXD_L4_CSUM_NOT | MVPP2_TXD_IP_CSUM_DISABLE;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] net: wireless: iwlegacy: Fix possible data races in il4965_send_rxon_assoc()
From: Stanislaw Gruszka @ 2018-10-04 7:59 UTC (permalink / raw)
To: Jia-Ju Bai; +Cc: kvalo, davem, linux-wireless, netdev, linux-kernel
In-Reply-To: <20181003140745.7650-1-baijiaju1990@gmail.com>
On Wed, Oct 03, 2018 at 10:07:45PM +0800, Jia-Ju Bai wrote:
> These possible races are detected by a runtime testing.
> To fix these races, the mutex lock is used in il4965_send_rxon_assoc()
> to protect the data.
Really ? I'm surprised by that, see below.
> @@ -1297,6 +1297,7 @@ il4965_send_rxon_assoc(struct il_priv *il)
> const struct il_rxon_cmd *rxon1 = &il->staging;
> const struct il_rxon_cmd *rxon2 = &il->active;
>
> + mutex_lock(&il->mutex);
> if (rxon1->flags == rxon2->flags &&
For 4965 driver il4965_send_rxon_assoc() is only called by
il_mac_bss_info_changed() and il4965_commit_rxon().
il_mac_bss_info_changed() acquire il->mutex and
callers of il4965_commit_rxon() acquire il->mutex
(but I did not check all of them).
So I wonder how this patch did not cause the deadlock ?
Anyway what can be done is adding:
lockdep_assert_held(&il->mutex);
il4965_commit_rxon() to check if we hold the mutex.
Thanks
Stanislaw
^ permalink raw reply
* [PATCH] atm: nicstar: Replace spin_is_locked() with spin_trylock()
From: Lance Roy @ 2018-10-04 7:46 UTC (permalink / raw)
To: linux-kernel
Cc: Paul E. McKenney, Lance Roy, Chas Williams, linux-atm-general,
netdev
ns_poll() used spin_is_locked() + spin_lock() to get achieve the same
thing as a spin_trylock(), so simplify it by using that instead. This is
also a step towards possibly removing spin_is_locked().
Signed-off-by: Lance Roy <ldr709@gmail.com>
Cc: Chas Williams <3chas3@gmail.com>
Cc: <linux-atm-general@lists.sourceforge.net>
Cc: <netdev@vger.kernel.org>
---
drivers/atm/nicstar.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/atm/nicstar.c b/drivers/atm/nicstar.c
index cbec9adc01c7..ae4aa02e4dc6 100644
--- a/drivers/atm/nicstar.c
+++ b/drivers/atm/nicstar.c
@@ -2689,11 +2689,10 @@ static void ns_poll(struct timer_list *unused)
PRINTK("nicstar: Entering ns_poll().\n");
for (i = 0; i < num_cards; i++) {
card = cards[i];
- if (spin_is_locked(&card->int_lock)) {
+ if (!spin_trylock_irqsave(&card->int_lock, flags)) {
/* Probably it isn't worth spinning */
continue;
}
- spin_lock_irqsave(&card->int_lock, flags);
stat_w = 0;
stat_r = readl(card->membase + STAT);
--
2.19.0
^ permalink raw reply related
* Clang warning in drivers/net/ethernet/qlogic/qla3xxx.c
From: Nathan Chancellor @ 2018-10-04 7:05 UTC (permalink / raw)
To: Dept-GELinuxNICDev; +Cc: netdev, linux-kernel
Hi all,
In an effort to get the kernel compiling warning free with Clang, I came
the following warning:
drivers/net/ethernet/qlogic/qla3xxx.c:384:24: warning: signed shift result (0xF00000000) requires 37 bits to represent, but 'int' only has 32 bits [-Wshift-overflow]
((ISP_NVRAM_MASK << 16) | qdev->eeprom_cmd_data));
~~~~~~~~~~~~~~ ^ ~~
1 warning generated.
This particular statement has been present since the introduction of the
driver in 2006 so I am unsure if this was intention or if maybe the
statement should have been removed since ISP_NVRAM_MASK is defined as
'0x000F << 16' in drivers/net/ethernet/qlogic/qla3xxx.h and it's the
only enum to be shifted. This is the only location in the driver that
has this warning.
Perhaps this diff would solve the warning and keep the driver
functional? I'll be happy to send a formal patch if that is the case.
diff --git a/drivers/net/ethernet/qlogic/qla3xxx.c b/drivers/net/ethernet/qlogic/qla3xxx.c
index b48f76182049..10b075bc5959 100644
--- a/drivers/net/ethernet/qlogic/qla3xxx.c
+++ b/drivers/net/ethernet/qlogic/qla3xxx.c
@@ -380,8 +380,6 @@ static void fm93c56a_select(struct ql3_adapter *qdev)
qdev->eeprom_cmd_data = AUBURN_EEPROM_CS_1;
ql_write_nvram_reg(qdev, spir, ISP_NVRAM_MASK | qdev->eeprom_cmd_data);
- ql_write_nvram_reg(qdev, spir,
- ((ISP_NVRAM_MASK << 16) | qdev->eeprom_cmd_data));
}
/*
^ permalink raw reply related
* Re: [PATCH 08/16] wireless: Replace spin_is_locked() with lockdep
From: Lance Roy @ 2018-10-04 6:57 UTC (permalink / raw)
To: Kalle Valo
Cc: linux-kernel, Paul E. McKenney, Daniel Drake, Ulrich Kunitz,
David S. Miller, linux-wireless, netdev
In-Reply-To: <877eizjt5j.fsf@kamboji.qca.qualcomm.com>
On Wed, Oct 03, 2018 at 12:06:48PM +0300, Kalle Valo wrote:
> Lance Roy <ldr709@gmail.com> writes:
>
> > lockdep_assert_held() is better suited to checking locking requirements,
> > since it won't get confused when someone else holds the lock. This is
> > also a step towards possibly removing spin_is_locked().
> >
> > Signed-off-by: Lance Roy <ldr709@gmail.com>
> > Cc: Daniel Drake <dsd@gentoo.org>
> > Cc: Ulrich Kunitz <kune@deine-taler.de>
> > Cc: Kalle Valo <kvalo@codeaurora.org>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: <linux-wireless@vger.kernel.org>
> > Cc: <netdev@vger.kernel.org>
> > ---
> > drivers/net/wireless/zydas/zd1211rw/zd_mac.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Should I take this or is it going through some other tree?
Sure you can take this if you like. If not, Paul McKenney will take it in his
tree.
Thanks,
Lance
> If it goes to via some other tree:
>
> Acked-by: Kalle Valo <kvalo@codeaurora.org>
>
> --
> Kalle Valo
^ permalink raw reply
* [PATCH RFC,net-next 3/3] netfilter: nft_tunnel: support for tunnel type
From: Pablo Neira Ayuso @ 2018-10-04 0:03 UTC (permalink / raw)
To: netdev; +Cc: netfilter-devel, roopa, amir, pshelar, u9012063
In-Reply-To: <20181004000345.2380-1-pablo@netfilter.org>
This patch allows you to set an explicit tunnel driver type in the
metadata template. In case of misconfiguration, ie. if the packets ends
up in the wrong tunnel device, the packet is dropped.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/uapi/linux/netfilter/nf_tables.h | 10 ++++++++++
net/netfilter/nft_tunnel.c | 9 ++++++++-
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 5444e76870bb..b36acb52eb50 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -1692,6 +1692,15 @@ enum nft_tunnel_flags {
NFT_TUNNEL_F_DONT_FRAGMENT | \
NFT_TUNNEL_F_SEQ_NUMBER)
+/* 1:1 mapping with the internal enum ip_tunnel_type. */
+enum nft_tunnel_type {
+ NFT_TUNNEL_TYPE_UNSPEC = 0,
+ NFT_TUNNEL_TYPE_GRE,
+ NFT_TUNNEL_TYPE_VXLAN,
+ NFT_TUNNEL_TYPE_GENEVE,
+ NFT_TUNNEL_TYPE_ERSPAN,
+};
+
enum nft_tunnel_key_attributes {
NFTA_TUNNEL_KEY_UNSPEC,
NFTA_TUNNEL_KEY_ID,
@@ -1703,6 +1712,7 @@ enum nft_tunnel_key_attributes {
NFTA_TUNNEL_KEY_SPORT,
NFTA_TUNNEL_KEY_DPORT,
NFTA_TUNNEL_KEY_OPTS,
+ NFTA_TUNNEL_KEY_TYPE,
__NFTA_TUNNEL_KEY_MAX
};
#define NFTA_TUNNEL_KEY_MAX (__NFTA_TUNNEL_KEY_MAX - 1)
diff --git a/net/netfilter/nft_tunnel.c b/net/netfilter/nft_tunnel.c
index 3a15f219e4e7..3cc54bc4ce31 100644
--- a/net/netfilter/nft_tunnel.c
+++ b/net/netfilter/nft_tunnel.c
@@ -305,6 +305,7 @@ static const struct nla_policy nft_tunnel_key_policy[NFTA_TUNNEL_KEY_MAX + 1] =
[NFTA_TUNNEL_KEY_TOS] = { .type = NLA_U8, },
[NFTA_TUNNEL_KEY_TTL] = { .type = NLA_U8, },
[NFTA_TUNNEL_KEY_OPTS] = { .type = NLA_NESTED, },
+ [NFTA_TUNNEL_KEY_TYPE] = { .type = NLA_U32, },
};
static int nft_tunnel_obj_init(const struct nft_ctx *ctx,
@@ -312,6 +313,7 @@ static int nft_tunnel_obj_init(const struct nft_ctx *ctx,
struct nft_object *obj)
{
struct nft_tunnel_obj *priv = nft_obj_data(obj);
+ u32 type = NFT_TUNNEL_TYPE_UNSPEC;
struct ip_tunnel_info info;
struct metadata_dst *md;
int err;
@@ -319,7 +321,11 @@ static int nft_tunnel_obj_init(const struct nft_ctx *ctx,
if (!tb[NFTA_TUNNEL_KEY_ID])
return -EINVAL;
+ if (tb[NFTA_TUNNEL_KEY_TYPE])
+ type = ntohl(nla_get_be32(tb[NFTA_TUNNEL_KEY_TYPE]));
+
memset(&info, 0, sizeof(info));
+ info.type = type;
info.mode = IP_TUNNEL_INFO_TX;
info.key.tun_id = key32_to_tunnel_id(nla_get_be32(tb[NFTA_TUNNEL_KEY_ID]));
info.key.tun_flags = TUNNEL_KEY | TUNNEL_CSUM | TUNNEL_NOCACHE;
@@ -494,7 +500,8 @@ static int nft_tunnel_obj_dump(struct sk_buff *skb,
struct nft_tunnel_obj *priv = nft_obj_data(obj);
struct ip_tunnel_info *info = &priv->md->u.tun_info;
- if (nla_put_be32(skb, NFTA_TUNNEL_KEY_ID,
+ if (nla_put_be32(skb, NFTA_TUNNEL_KEY_TYPE, htonl(info->type)) ||
+ nla_put_be32(skb, NFTA_TUNNEL_KEY_ID,
tunnel_id_to_key32(info->key.tun_id)) ||
nft_tunnel_ip_dump(skb, info) < 0 ||
nft_tunnel_ports_dump(skb, info) < 0 ||
--
2.11.0
^ permalink raw reply related
* [PATCH RFC,net-next 2/3] net: act_tunnel_key: support for tunnel type
From: Pablo Neira Ayuso @ 2018-10-04 0:03 UTC (permalink / raw)
To: netdev; +Cc: netfilter-devel, roopa, amir, pshelar, u9012063
In-Reply-To: <20181004000345.2380-1-pablo@netfilter.org>
This patch allows you to set an explicit tunnel driver type in the
metadata template. In case of misconfiguration, ie. if the packets ends
up in the wrong tunnel device, the packet is dropped.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/uapi/linux/tc_act/tc_tunnel_key.h | 10 ++++++++++
net/sched/act_tunnel_key.c | 9 +++++++++
2 files changed, 19 insertions(+)
diff --git a/include/uapi/linux/tc_act/tc_tunnel_key.h b/include/uapi/linux/tc_act/tc_tunnel_key.h
index be384d63e1b5..f6471549a085 100644
--- a/include/uapi/linux/tc_act/tc_tunnel_key.h
+++ b/include/uapi/linux/tc_act/tc_tunnel_key.h
@@ -41,9 +41,19 @@ enum {
*/
TCA_TUNNEL_KEY_ENC_TOS, /* u8 */
TCA_TUNNEL_KEY_ENC_TTL, /* u8 */
+ TCA_TUNNEL_KEY_ENC_TYPE, /* u32 */
__TCA_TUNNEL_KEY_MAX,
};
+/* 1:1 mapping with the internal enum ip_tunnel_type. */
+enum tc_tunnel_type {
+ TC_TUNNEL_TYPE_UNSPEC = 0,
+ TC_TUNNEL_TYPE_GRE,
+ TC_TUNNEL_TYPE_VXLAN,
+ TC_TUNNEL_TYPE_GENEVE,
+ TC_TUNNEL_TYPE_ERSPAN,
+};
+
#define TCA_TUNNEL_KEY_MAX (__TCA_TUNNEL_KEY_MAX - 1)
enum {
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index 4cca8f274662..e3a1306c7ea8 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -195,6 +195,7 @@ static const struct nla_policy tunnel_key_policy[TCA_TUNNEL_KEY_MAX + 1] = {
[TCA_TUNNEL_KEY_ENC_OPTS] = { .type = NLA_NESTED },
[TCA_TUNNEL_KEY_ENC_TOS] = { .type = NLA_U8 },
[TCA_TUNNEL_KEY_ENC_TTL] = { .type = NLA_U8 },
+ [TCA_TUNNEL_KEY_ENC_TYPE] = { .type = NLA_U32 },
};
static int tunnel_key_init(struct net *net, struct nlattr *nla,
@@ -215,6 +216,7 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
__be16 flags;
u8 tos, ttl;
int ret = 0;
+ u32 type;
int err;
if (!nla) {
@@ -278,6 +280,10 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
if (tb[TCA_TUNNEL_KEY_ENC_TTL])
ttl = nla_get_u8(tb[TCA_TUNNEL_KEY_ENC_TTL]);
+ type = TC_TUNNEL_TYPE_UNSPEC;
+ if (tb[TCA_TUNNEL_KEY_ENC_TYPE])
+ type = nla_get_u32(tb[TCA_TUNNEL_KEY_ENC_TYPE]);
+
if (tb[TCA_TUNNEL_KEY_ENC_IPV4_SRC] &&
tb[TCA_TUNNEL_KEY_ENC_IPV4_DST]) {
__be32 saddr;
@@ -320,6 +326,7 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
goto release_tun_meta;
}
+ metadata->u.tun_info.type = type;
metadata->u.tun_info.mode |= IP_TUNNEL_INFO_TX;
break;
default:
@@ -522,6 +529,8 @@ static int tunnel_key_dump(struct sk_buff *skb, struct tc_action *a,
if (key->ttl && nla_put_u8(skb, TCA_TUNNEL_KEY_ENC_TTL, key->ttl))
goto nla_put_failure;
+ if (nla_put_u32(skb, TCA_TUNNEL_KEY_ENC_TYPE, info->type))
+ goto nla_put_failure;
}
tcf_tm_dump(&tm, &t->tcf_tm);
--
2.11.0
^ permalink raw reply related
* [PATCH RFC,net-next 0/3] ip_tunnel: specify tunnel type via template
From: Pablo Neira Ayuso @ 2018-10-04 0:03 UTC (permalink / raw)
To: netdev; +Cc: netfilter-devel, roopa, amir, pshelar, u9012063
Hi,
The following patchset adds a new field to the tunnel metadata template
to restrict the configuration to a given tunnel driver. Currently, a
misconfiguration may result in packets going to the wrong tunnel driver.
Although we have the tunnel option flags, they are not mandatory for
some tunnel drivers, eg. vxlan, which may use it or not; and gre which
does not use them.
This patch updates tc's tunnel action and netfilter's tunnel extension
to use this new field. OVS netlink interface has been left unset, although they
could be updated to use this.
By extending the existing tc action to support the IP_TUNNEL_INFO_BRIDGE
mode, I think it should be possible to expose IP_TUNNEL_TYPE_VLAN too,
although this patchset doesn't address this scenario.
The field is initialized to zero, which maps to IP_TUNNEL_TYPE_UNSPEC to
retain the existing behaviour, so the existing flexibility is still in
place while this new feature is added.
Cc'ing people that git annotate show as dealing with these bits more
recently.
Compile tested only.
Comments welcome, thanks.
Pablo Neira Ayuso (3):
ip_tunnel: add type field to struct ip_tunnel_info
net: act_tunnel_key: support for tunnel type
netfilter: nft_tunnel: support for tunnel type
drivers/net/geneve.c | 3 ++-
drivers/net/vxlan.c | 13 +++++++------
include/net/dst_metadata.h | 1 +
include/net/ip_tunnels.h | 16 ++++++++++++++++
include/uapi/linux/netfilter/nf_tables.h | 10 ++++++++++
include/uapi/linux/tc_act/tc_tunnel_key.h | 10 ++++++++++
net/ipv4/ip_gre.c | 2 ++
net/ipv6/ip6_gre.c | 2 ++
net/netfilter/nft_tunnel.c | 9 ++++++++-
net/openvswitch/flow_netlink.c | 1 +
net/sched/act_tunnel_key.c | 9 +++++++++
11 files changed, 68 insertions(+), 8 deletions(-)
^ permalink raw reply
* [PATCH RFC,net-next 1/3] ip_tunnel: add type field to struct ip_tunnel_info
From: Pablo Neira Ayuso @ 2018-10-04 0:03 UTC (permalink / raw)
To: netdev; +Cc: netfilter-devel, roopa, amir, pshelar, u9012063
In-Reply-To: <20181004000345.2380-1-pablo@netfilter.org>
This new field allows you to restrict the metadata template for a given
tunnel driver. This is convenient in scenarios that combine different
tunneling drivers, to deal with possible misconfigurations given that
the template can be interpreted by any target tunnel driver. Default
value is IP_TUNNEL_TYPE_UNSPEC, to retain the existing behaviour. This
also implicitly exposes what drivers are currently supported in the
IP_TUNNEL_INFO_TX mode.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
drivers/net/geneve.c | 3 ++-
drivers/net/vxlan.c | 13 +++++++------
include/net/dst_metadata.h | 1 +
include/net/ip_tunnels.h | 16 ++++++++++++++++
net/ipv4/ip_gre.c | 2 ++
net/ipv6/ip6_gre.c | 2 ++
net/openvswitch/flow_netlink.c | 1 +
7 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 6625fabe2c88..c383c394f0d2 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -920,7 +920,8 @@ static netdev_tx_t geneve_xmit(struct sk_buff *skb, struct net_device *dev)
if (geneve->collect_md) {
info = skb_tunnel_info(skb);
- if (unlikely(!info || !(info->mode & IP_TUNNEL_INFO_TX))) {
+ if (unlikely(!info || !(info->mode & IP_TUNNEL_INFO_TX) ||
+ !ip_tunnel_type(info, IP_TUNNEL_TYPE_GENEVE))) {
err = -EINVAL;
netdev_dbg(dev, "no tunnel metadata\n");
goto tx_error;
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index e5d236595206..8cca91b572bd 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2296,14 +2296,15 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
skb_reset_mac_header(skb);
if (vxlan->cfg.flags & VXLAN_F_COLLECT_METADATA) {
- if (info && info->mode & IP_TUNNEL_INFO_BRIDGE &&
- info->mode & IP_TUNNEL_INFO_TX) {
+ if (unlikely(!info || !(info->mode & IP_TUNNEL_INFO_TX) ||
+ !ip_tunnel_type(info, IP_TUNNEL_TYPE_VXLAN))) {
+ kfree_skb(skb);
+ return NETDEV_TX_OK;
+ }
+ if (info->mode & IP_TUNNEL_INFO_BRIDGE) {
vni = tunnel_id_to_key32(info->key.tun_id);
} else {
- if (info && info->mode & IP_TUNNEL_INFO_TX)
- vxlan_xmit_one(skb, dev, vni, NULL, false);
- else
- kfree_skb(skb);
+ vxlan_xmit_one(skb, dev, vni, NULL, false);
return NETDEV_TX_OK;
}
}
diff --git a/include/net/dst_metadata.h b/include/net/dst_metadata.h
index 56cb3c38569a..674116f7fa4a 100644
--- a/include/net/dst_metadata.h
+++ b/include/net/dst_metadata.h
@@ -100,6 +100,7 @@ static inline struct metadata_dst *tun_rx_dst(int md_size)
if (!tun_dst)
return NULL;
+ tun_dst->u.tun_info.type = IP_TUNNEL_TYPE_UNSPEC;
tun_dst->u.tun_info.options_len = 0;
tun_dst->u.tun_info.mode = 0;
return tun_dst;
diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index b0d022ff6ea1..985d24b6a102 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -66,7 +66,16 @@ struct ip_tunnel_key {
GENMASK((FIELD_SIZEOF(struct ip_tunnel_info, \
options_len) * BITS_PER_BYTE) - 1, 0)
+enum ip_tunnel_type {
+ IP_TUNNEL_TYPE_UNSPEC = 0,
+ IP_TUNNEL_TYPE_GRE,
+ IP_TUNNEL_TYPE_VXLAN,
+ IP_TUNNEL_TYPE_GENEVE,
+ IP_TUNNEL_TYPE_ERSPAN,
+};
+
struct ip_tunnel_info {
+ enum ip_tunnel_type type;
struct ip_tunnel_key key;
#ifdef CONFIG_DST_CACHE
struct dst_cache dst_cache;
@@ -75,6 +84,13 @@ struct ip_tunnel_info {
u8 mode;
};
+static inline bool ip_tunnel_type(const struct ip_tunnel_info *tun_info,
+ enum ip_tunnel_type type)
+{
+ return tun_info->type == IP_TUNNEL_TYPE_UNSPEC ||
+ tun_info->type == type;
+}
+
/* 6rd prefix/relay information */
#ifdef CONFIG_IPV6_SIT_6RD
struct ip_tunnel_6rd_parm {
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index c3385a84f8ff..3ad12135c3d3 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -534,6 +534,7 @@ static void gre_fb_xmit(struct sk_buff *skb, struct net_device *dev,
tun_info = skb_tunnel_info(skb);
if (unlikely(!tun_info || !(tun_info->mode & IP_TUNNEL_INFO_TX) ||
+ !ip_tunnel_type(tun_info, IP_TUNNEL_TYPE_GRE) ||
ip_tunnel_info_af(tun_info) != AF_INET))
goto err_free_skb;
@@ -585,6 +586,7 @@ static void erspan_fb_xmit(struct sk_buff *skb, struct net_device *dev,
tun_info = skb_tunnel_info(skb);
if (unlikely(!tun_info || !(tun_info->mode & IP_TUNNEL_INFO_TX) ||
+ !ip_tunnel_type(tun_info, IP_TUNNEL_TYPE_ERSPAN) ||
ip_tunnel_info_af(tun_info) != AF_INET))
goto err_free_skb;
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 515adbdba1d2..675f373809ee 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -732,6 +732,7 @@ static netdev_tx_t __gre6_xmit(struct sk_buff *skb,
tun_info = skb_tunnel_info(skb);
if (unlikely(!tun_info ||
!(tun_info->mode & IP_TUNNEL_INFO_TX) ||
+ !ip_tunnel_type(tun_info, IP_TUNNEL_TYPE_GRE) ||
ip_tunnel_info_af(tun_info) != AF_INET6))
return -EINVAL;
@@ -960,6 +961,7 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff *skb,
tun_info = skb_tunnel_info(skb);
if (unlikely(!tun_info ||
!(tun_info->mode & IP_TUNNEL_INFO_TX) ||
+ !ip_tunnel_type(tun_info, IP_TUNNEL_TYPE_ERSPAN) ||
ip_tunnel_info_af(tun_info) != AF_INET6))
return -EINVAL;
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index a70097ecf33c..1ee2509534df 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -2602,6 +2602,7 @@ static int validate_and_copy_set_tun(const struct nlattr *attr,
ovs_tun->tun_dst = tun_dst;
tun_info = &tun_dst->u.tun_info;
+ tun_info->type = IP_TUNNEL_TYPE_UNSPEC;
tun_info->mode = IP_TUNNEL_INFO_TX;
if (key.tun_proto == AF_INET6)
tun_info->mode |= IP_TUNNEL_INFO_IPV6;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH iproute2] lib/libnetlink: fix response seq check
From: Stephen Hemminger @ 2018-10-03 23:50 UTC (permalink / raw)
To: Vlad Dumitrescu; +Cc: chrism, Mahesh Bandewar, netdev, Vlad Dumitrescu
In-Reply-To: <CALpBo+WyoSH5o+Vg1j_jjA9e_UeVB=9x7q+szPW330J=zdJWqg@mail.gmail.com>
On Wed, 3 Oct 2018 16:01:40 -0700
Vlad Dumitrescu <vlad@dumitrescu.ro> wrote:
> Hi,
>
> On Fri, Sep 28, 2018 at 10:14 AM <vlad@dumitrescu.ro> wrote:
> >
> > From: Vlad Dumitrescu <vladum@google.com>
> >
> > Taking a one-iovec example, with rtnl->seq at 42. iovlen == 1, seq
> > becomes 43 on line 604, and a message is sent with nlmsg_seq == 43. If
> > a response with nlmsg_seq of 42 is received, the condition being fixed
> > in this patch would incorrectly accept it.
> >
> > Fixes: 72a2ff3916e5 ("lib/libnetlink: Add a new function rtnl_talk_iov")
> > Signed-off-by: Vlad Dumitrescu <vladum@google.com>
> > ---
> > lib/libnetlink.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/libnetlink.c b/lib/libnetlink.c
> > index f18dceac..4d2416bf 100644
> > --- a/lib/libnetlink.c
> > +++ b/lib/libnetlink.c
> > @@ -647,7 +647,7 @@ static int __rtnl_talk_iov(struct rtnl_handle *rtnl, struct iovec *iov,
> >
> > if (nladdr.nl_pid != 0 ||
> > h->nlmsg_pid != rtnl->local.nl_pid ||
> > - h->nlmsg_seq > seq || h->nlmsg_seq < seq - iovlen) {
> > + h->nlmsg_seq > seq || h->nlmsg_seq < seq - iovlen + 1) {
> > /* Don't forget to skip that message. */
> > status -= NLMSG_ALIGN(len);
> > h = (struct nlmsghdr *)((char *)h + NLMSG_ALIGN(len));
> > --
> > 2.19.0.605.g01d371f741-goog
>
> Did anybody get a chance to review this? I'm not 100% sure I'm fixing
> the right thing.
>
> Thanks,
> Vlad
Could you give an example where this failed.
Better yet one of the tests.
^ permalink raw reply
* Re: Kernel oops with mlx5 and dual XDP redirect programs
From: Saeed Mahameed @ 2018-10-03 23:44 UTC (permalink / raw)
To: toke@toke.dk, netdev@vger.kernel.org
Cc: Eran Ben Elisha, Tariq Toukan, brouer@redhat.com
In-Reply-To: <877eize5ro.fsf@toke.dk>
On Wed, 2018-10-03 at 11:30 +0200, Toke Høiland-Jørgensen wrote:
> Hi Saeed
>
> I can reliably oops the kernel with the mlx5 driver, by installing
> XDP_REDIRECT programs on two devices so they redirect to each other,
> and then remove them while there is traffic on the interface.
>
> Steps to reproduce:
>
> # cd ~/build/linux/samples/bpf
> # ./xdp_redirect_map $(</sys/class/net/ens1f1/ifindex)
> $(</sys/class/net/ens1f0/ifindex)
> # ./xdp_redirect_map $(</sys/class/net/ens1f0/ifindex)
> $(</sys/class/net/ens1f1/ifindex)
>
> Now, run some traffic (e.g., using pktgen) across the interfaces, and
> while the traffic is running, interrupt one of the xdp_redirect_map
> commands (thus unloading the eBPF program). This results in a kernel
> oops with the backtrace below. I get no crash if there's only a
> single
> XDP program.
Hi Toke,
What looks like happening is that while the traffic is being redirected
to the other device, the driver is trying to unload the program and
restarting the rings from below call trace we can see:
[ 1400.972318] RIP: 0010:mlx5e_xdp_xmit+0x7b/0x2a0 [mlx5_core]
[ 1401.077409] bq_xmit_all+0x5e/0x160
[ 1401.080897] dev_map_enqueue+0x12e/0x140
[ 1401.084823] xdp_do_redirect+0x1a9/0x2a0
[ 1401.088756] mlx5e_xdp_handle+0x24f/0x2b0 [mlx5_core]
and
[ 1401.154559] RIP: 0010:mlx5e_open_channels+0x65e/0x1390 [mlx5_core]
[ 1401.222834] ? mlx5e_open_channels+0x5e1/0x1390 [mlx5_core]
[ 1401.228404] ? rcu_exp_wait_wake+0x550/0x550
[ 1401.232674] ? free_one_page+0x68/0x370
[ 1401.236519] mlx5e_open_locked+0x28/0xa0 [mlx5_core]
[ 1401.241491] mlx5e_xdp+0x2b2/0x300 [mlx5_core]
[ 1401.245936] dev_xdp_install+0x4c/0x70
[ 1401.249686] do_setlink+0xcdb/0xd10
I think that the mlx5 driver doesn't know how to tell the other device
to stop transmitting to it while it is resetting.. Maybe tariq or
Jesper know more about this ?
I will look at this tomorrow after noon and will try to repro...
what is interesting is that @ mlx5e_open_channels stage all previous
TX queues must be still active and not destroyed only later on when we
switch to the new channels we stop and destroy older TX/RX queues, the
question is how much this call trace is reliable ?
Thanks for the report.
>
> Is this something you could look into, please? :)
>
> -Toke
>
>
> [ 1400.937870] BUG: unable to handle kernel paging request at
> 0000000000003fa8
> [ 1400.944826] PGD 800000072cc7b067 P4D 800000072cc7b067 PUD
> 72cc7a067 PMD 0
> [ 1400.951693] Oops: 0000 [#1] SMP PTI
> [ 1400.955184] CPU: 5 PID: 10392 Comm: xdp_redirect_ma Not tainted
> 4.19.0-rc5-xdptest-g5be3ebf+ #17
> [ 1400.965344] Hardware name: LENOVO 30B3005DMT/102F, BIOS S00KT56A
> 01/15/2018
> [ 1400.972318] RIP: 0010:mlx5e_xdp_xmit+0x7b/0x2a0 [mlx5_core]
> [ 1400.977889] Code: 8b 0d 29 d9 4f 3f 39 8f 48 39 00 00 b8 fa ff ff
> ff 0f 86 45 01 00 00 48 8b 87 40 39 00 00 48 63 c9 4c 8b 24 c8 b8 9c
> ff ff ff <49> 8b 8c 24 a8 3f 00 00 4d 8d bc 24 c0 3c 00 00 83 e1 01
> 0f 84 19
> [ 1400.996624] RSP: 0018:ffff90209fb43bb0 EFLAGS: 00010202
> [ 1401.002001] RAX: 00000000ffffff9c RBX: 0000000000000000 RCX:
> 0000000000000005
> [ 1401.009122] RDX: ffffc7627fd75190 RSI: 0000000000000010 RDI:
> ffff902084580000
> [ 1401.016250] RBP: ffffc7627fd75190 R08: ffff901f9821c100 R09:
> ffffc7627fd75210
> [ 1401.023379] R10: 00000000000005dc R11: 0000000000000000 R12:
> 0000000000000000
> [ 1401.030500] R13: ffff902081580000 R14: 0000000000000001 R15:
> ffffc7627fd75190
> [ 1401.037645] FS: 00007f460fa96700(0000) GS:ffff90209fb40000(0000)
> knlGS:0000000000000000
> [ 1401.045718] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1401.051452] CR2: 0000000000003fa8 CR3: 000000076c3b6006 CR4:
> 00000000003606e0
> [ 1401.058573] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [ 1401.065823] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
> 0000000000000400
> [ 1401.072943] Call Trace:
> [ 1401.075390] <IRQ>
> [ 1401.077409] bq_xmit_all+0x5e/0x160
> [ 1401.080897] dev_map_enqueue+0x12e/0x140
> [ 1401.084823] xdp_do_redirect+0x1a9/0x2a0
> [ 1401.088756] mlx5e_xdp_handle+0x24f/0x2b0 [mlx5_core]
> [ 1401.093821] ? resched_cpu+0x5f/0x70
> [ 1401.097399] ? __xdp_return+0x189/0x400
> [ 1401.101242] mlx5e_skb_from_cqe_linear+0xdd/0x180 [mlx5_core]
> [ 1401.106987] mlx5e_handle_rx_cqe+0x43/0xe0 [mlx5_core]
> [ 1401.112130] mlx5e_poll_rx_cq+0xcb/0x940 [mlx5_core]
> [ 1401.117094] mlx5e_napi_poll+0xa6/0xc90 [mlx5_core]
> [ 1401.121966] ? smp_reschedule_interrupt+0x16/0xd0
> [ 1401.126789] ? reschedule_interrupt+0xf/0x20
> [ 1401.131057] ? reschedule_interrupt+0xa/0x20
> [ 1401.135321] net_rx_action+0x279/0x3d0
> [ 1401.139071] __do_softirq+0xf2/0x28e
> [ 1401.142651] irq_exit+0xb6/0xc0
> [ 1401.145792] do_IRQ+0x52/0xd0
> [ 1401.148785] common_interrupt+0xf/0xf
> [ 1401.152445] </IRQ>
> [ 1401.154559] RIP: 0010:mlx5e_open_channels+0x65e/0x1390 [mlx5_core]
> [ 1401.160734] Code: 8b 00 48 05 a8 00 00 00 48 89 85 78 3c 00 00 48
> 8b 83 f8 8d 01 00 48 89 85 80 3c 00 00 48 8b 83 f0 8d 01 00 8b 80 a8
> fb 03 00 <0f> c8 89 85 88 3c 00 00 41 0f b6 45 16 88 85 8c 3c 00 00
> 49 83 bd
> [ 1401.179463] RSP: 0018:ffffa7628dd43808 EFLAGS: 00000282 ORIG_RAX:
> ffffffffffffffd4
> [ 1401.187024] RAX: 0000000000080000 RBX: ffff9020845808c0 RCX:
> 0000000000000000
> [ 1401.194325] RDX: ffffa7628dd43894 RSI: 0000000000000000 RDI:
> ffff901f8a0e0000
> [ 1401.201463] RBP: ffff901f8a0d8000 R08: ffffe1799d283800 R09:
> 0000000000000008
> [ 1401.208582] R10: 0000000000000000 R11: 0000000000000002 R12:
> 0000000000000000
> [ 1401.215702] R13: ffff902084583940 R14: 0000000000000000 R15:
> 0000000000000000
> [ 1401.222834] ? mlx5e_open_channels+0x5e1/0x1390 [mlx5_core]
> [ 1401.228404] ? rcu_exp_wait_wake+0x550/0x550
> [ 1401.232674] ? free_one_page+0x68/0x370
> [ 1401.236519] mlx5e_open_locked+0x28/0xa0 [mlx5_core]
> [ 1401.241491] mlx5e_xdp+0x2b2/0x300 [mlx5_core]
> [ 1401.245936] dev_xdp_install+0x4c/0x70
> [ 1401.249686] do_setlink+0xcdb/0xd10
> [ 1401.253300] ? flat_send_IPI_allbutself+0x6c/0xa0
> [ 1401.258003] ? __update_load_avg_se+0x20c/0x290
> [ 1401.262530] rtnl_setlink+0x104/0x140
> [ 1401.266189] rtnetlink_rcv_msg+0x269/0x310
> [ 1401.270283] ? _cond_resched+0x16/0x40
> [ 1401.274029] ? __kmalloc_node_track_caller+0x1dd/0x2a0
> [ 1401.279162] ? rtnl_calcit.isra.32+0x110/0x110
> [ 1401.283601] netlink_rcv_skb+0xdb/0x110
> [ 1401.287437] netlink_unicast+0x18b/0x250
> [ 1401.291359] netlink_sendmsg+0x2c7/0x3b0
> [ 1401.295287] sock_sendmsg+0x30/0x40
> [ 1401.298776] __sys_sendto+0xd8/0x150
> [ 1401.302351] ? __sys_getsockname+0xac/0xc0
> [ 1401.306448] ? netlink_setsockopt+0x2e/0x2b0
> [ 1401.310718] ? __sys_setsockopt+0x7c/0xe0
> [ 1401.314867] __x64_sys_sendto+0x24/0x30
> [ 1401.318709] do_syscall_64+0x4f/0x100
> [ 1401.322372] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [ 1401.327420] RIP: 0033:0x7f460f3a83dd
> [ 1401.330997] Code: 89 01 48 83 c8 ff c3 66 0f 1f 44 00 00 8b 05 7a
> 13 2c 00 85 c0 75 3e 45 31 c9 45 31 c0 4c 63 d1 48 63 ff b8 2c 00 00
> 00 0f 05 <48> 3d 00 f0 ff ff 77 0b c3 66 2e 0f 1f 84 00 00 00 00 00
> 48 8b 15
> [ 1401.349733] RSP: 002b:00007ffd28d23138 EFLAGS: 00000246 ORIG_RAX:
> 000000000000002c
> [ 1401.357293] RAX: ffffffffffffffda RBX: ffffffffffffff90 RCX:
> 00007f460f3a83dd
> [ 1401.364413] RDX: 000000000000002c RSI: 00007ffd28d23170 RDI:
> 0000000000000003
> [ 1401.371533] RBP: 00007ffd28d231e0 R08: 0000000000000000 R09:
> 0000000000000000
> [ 1401.378767] R10: 0000000000000000 R11: 0000000000000246 R12:
> 0000000000000006
> [ 1401.385895] R13: 00007ffd28d237f0 R14: 00007ffd28d23830 R15:
> 00007ffd28d2388c
> [ 1401.393016] Modules linked in: rpcrdma ib_umad sunrpc ib_ipoib
> rdma_ucm mlx5_ib binfmt_misc ib_uverbs snd_hda_codec_hdmi intel_rapl
> sb_edac x86_pkg_temp_thermal intel_powerclamp coretemp mlx5_core
> kvm_intel snd_hda_codec_realtek snd_hda_codec_generic kvm
> snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_pcm e1000e uas
> irqbypass snd_timer crct10dif_pclmul snd mei_me usb_storage
> crc32_pclmul ghash_clmulni_intel wmi_bmof mei lpc_ich soundcore mlxfw
> pata_acpi mac_hid ib_iser rdma_cm iw_cm ib_cm ib_core configfs
> iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi autofs4 raid10
> raid456 libcrc32c async_raid6_recov async_memcpy async_pq async_xor
> xor async_tx raid6_pq raid1 raid0 multipath linear nouveau video
> i2c_algo_bit ttm drm_kms_helper syscopyarea sysfillrect sysimgblt
> fb_sys_fops drm mxm_wmi
> [ 1401.463638] aesni_intel aes_x86_64 crypto_simd cryptd glue_helper
> ahci libahci wmi
> [ 1401.471289] CR2: 0000000000003fa8
> [ 1401.474617] ---[ end trace 1a0d8962c7db30ed ]---
> [ 1401.528487] RIP: 0010:mlx5e_xdp_xmit+0x7b/0x2a0 [mlx5_core]
> [ 1401.534058] Code: 8b 0d 29 d9 4f 3f 39 8f 48 39 00 00 b8 fa ff ff
> ff 0f 86 45 01 00 00 48 8b 87 40 39 00 00 48 63 c9 4c 8b 24 c8 b8 9c
> ff ff ff <49> 8b 8c 24 a8 3f 00 00 4d 8d bc 24 c0 3c 00 00 83 e1 01
> 0f 84 19
> [ 1401.552789] RSP: 0018:ffff90209fb43bb0 EFLAGS: 00010202
> [ 1401.558012] RAX: 00000000ffffff9c RBX: 0000000000000000 RCX:
> 0000000000000005
> [ 1401.565132] RDX: ffffc7627fd75190 RSI: 0000000000000010 RDI:
> ffff902084580000
> [ 1401.572252] RBP: ffffc7627fd75190 R08: ffff901f9821c100 R09:
> ffffc7627fd75210
> [ 1401.579371] R10: 00000000000005dc R11: 0000000000000000 R12:
> 0000000000000000
> [ 1401.586493] R13: ffff902081580000 R14: 0000000000000001 R15:
> ffffc7627fd75190
> [ 1401.593726] FS: 00007f460fa96700(0000) GS:ffff90209fb40000(0000)
> knlGS:0000000000000000
> [ 1401.601797] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1401.607533] CR2: 0000000000003fa8 CR3: 000000076c3b6006 CR4:
> 00000000003606e0
> [ 1401.614653] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [ 1401.621772] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
> 0000000000000400
> [ 1401.628895] Kernel panic - not syncing: Fatal exception in
> interrupt
> [ 1401.635280] Kernel Offset: 0x5000000 from 0xffffffff81000000
> (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
> [ 1401.694263] Rebooting in 5 seconds..
^ permalink raw reply
* [PATCH v3] net/ncsi: Add NCSI OEM command support
From: Vijay Khemka @ 2018-10-03 23:32 UTC (permalink / raw)
To: Justin . Lee1 @ Dell . com, sam @ mendozajonas . com,
joel @ jms . id . au, linux-aspeed @ lists . ozlabs . org,
openbmc @ lists . ozlabs . org, Sai Dasari,
netdev @ vger . kernel . org, christian @ cmd . nu
Cc: Vijay Khemka
This patch adds OEM commands and response handling. It also defines OEM
command and response structure as per NCSI specification along with its
handlers.
ncsi_cmd_handler_oem: This is a generic command request handler for OEM
commands
ncsi_rsp_handler_oem: This is a generic response handler for OEM commands
Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
net/ncsi/internal.h | 5 +++++
net/ncsi/ncsi-cmd.c | 30 +++++++++++++++++++++++++++---
net/ncsi/ncsi-pkt.h | 14 ++++++++++++++
net/ncsi/ncsi-rsp.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 88 insertions(+), 4 deletions(-)
diff --git a/net/ncsi/internal.h b/net/ncsi/internal.h
index 8055e3965cef..3d0a33b874f5 100644
--- a/net/ncsi/internal.h
+++ b/net/ncsi/internal.h
@@ -68,6 +68,10 @@ enum {
NCSI_MODE_MAX
};
+/* OEM Vendor Manufacture ID */
+#define NCSI_OEM_MFR_MLX_ID 0x8119
+#define NCSI_OEM_MFR_BCM_ID 0x113d
+
struct ncsi_channel_version {
u32 version; /* Supported BCD encoded NCSI version */
u32 alpha2; /* Supported BCD encoded NCSI version */
@@ -305,6 +309,7 @@ struct ncsi_cmd_arg {
unsigned short words[8];
unsigned int dwords[4];
};
+ unsigned char *data; /* NCSI OEM data */
};
extern struct list_head ncsi_dev_list;
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index 7567ca63aae2..82b7d9201db8 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -211,6 +211,25 @@ static int ncsi_cmd_handler_snfc(struct sk_buff *skb,
return 0;
}
+static int ncsi_cmd_handler_oem(struct sk_buff *skb,
+ struct ncsi_cmd_arg *nca)
+{
+ struct ncsi_cmd_oem_pkt *cmd;
+ unsigned int len;
+
+ len = sizeof(struct ncsi_cmd_pkt_hdr) + 4;
+ if (nca->payload < 26)
+ len += 26;
+ else
+ len += nca->payload;
+
+ cmd = skb_put_zero(skb, len);
+ memcpy(&cmd->mfr_id, nca->data, nca->payload);
+ ncsi_cmd_build_header(&cmd->cmd.common, nca);
+
+ return 0;
+}
+
static struct ncsi_cmd_handler {
unsigned char type;
int payload;
@@ -244,7 +263,7 @@ static struct ncsi_cmd_handler {
{ NCSI_PKT_CMD_GNS, 0, ncsi_cmd_handler_default },
{ NCSI_PKT_CMD_GNPTS, 0, ncsi_cmd_handler_default },
{ NCSI_PKT_CMD_GPS, 0, ncsi_cmd_handler_default },
- { NCSI_PKT_CMD_OEM, 0, NULL },
+ { NCSI_PKT_CMD_OEM, -1, ncsi_cmd_handler_oem },
{ NCSI_PKT_CMD_PLDM, 0, NULL },
{ NCSI_PKT_CMD_GPUUID, 0, ncsi_cmd_handler_default }
};
@@ -316,8 +335,13 @@ int ncsi_xmit_cmd(struct ncsi_cmd_arg *nca)
return -ENOENT;
}
- /* Get packet payload length and allocate the request */
- nca->payload = nch->payload;
+ /* Get packet payload length and allocate the request
+ * It is expected that if length set as negative in
+ * handler structure means caller is initializing it
+ * and setting length in nca before calling xmit function
+ */
+ if (nch->payload >= 0)
+ nca->payload = nch->payload;
nr = ncsi_alloc_command(nca);
if (!nr)
return -ENOMEM;
diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h
index 91b4b66438df..0f2087c8d42a 100644
--- a/net/ncsi/ncsi-pkt.h
+++ b/net/ncsi/ncsi-pkt.h
@@ -151,6 +151,20 @@ struct ncsi_cmd_snfc_pkt {
unsigned char pad[22];
};
+/* OEM Request Command as per NCSI Specification */
+struct ncsi_cmd_oem_pkt {
+ struct ncsi_cmd_pkt_hdr cmd; /* Command header */
+ __be32 mfr_id; /* Manufacture ID */
+ unsigned char data[]; /* OEM Payload Data */
+};
+
+/* OEM Response Packet as per NCSI Specification */
+struct ncsi_rsp_oem_pkt {
+ struct ncsi_rsp_pkt_hdr rsp; /* Command header */
+ __be32 mfr_id; /* Manufacture ID */
+ unsigned char data[]; /* Payload data */
+};
+
/* Get Link Status */
struct ncsi_rsp_gls_pkt {
struct ncsi_rsp_pkt_hdr rsp; /* Response header */
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 930c1d3796f0..d66b34749027 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -596,6 +596,47 @@ static int ncsi_rsp_handler_snfc(struct ncsi_request *nr)
return 0;
}
+static struct ncsi_rsp_oem_handler {
+ unsigned int mfr_id;
+ int (*handler)(struct ncsi_request *nr);
+} ncsi_rsp_oem_handlers[] = {
+ { NCSI_OEM_MFR_MLX_ID, NULL },
+ { NCSI_OEM_MFR_BCM_ID, NULL }
+};
+
+/* Response handler for OEM command */
+static int ncsi_rsp_handler_oem(struct ncsi_request *nr)
+{
+ struct ncsi_rsp_oem_pkt *rsp;
+ struct ncsi_rsp_oem_handler *nrh = NULL;
+ unsigned int mfr_id, i;
+
+ /* Get the response header */
+ rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
+ mfr_id = ntohl(rsp->mfr_id);
+
+ /* Check for manufacturer id and Find the handler */
+ for (i = 0; i < ARRAY_SIZE(ncsi_rsp_oem_handlers); i++) {
+ if (ncsi_rsp_oem_handlers[i].mfr_id == mfr_id) {
+ if (ncsi_rsp_oem_handlers[i].handler)
+ nrh = &ncsi_rsp_oem_handlers[i];
+ else
+ nrh = NULL;
+
+ break;
+ }
+ }
+
+ if (!nrh) {
+ netdev_err(nr->ndp->ndev.dev, "Received unrecognized OEM packet with MFR-ID (0x%x)\n",
+ mfr_id);
+ return -ENOENT;
+ }
+
+ /* Process the packet */
+ return nrh->handler(nr);
+}
+
static int ncsi_rsp_handler_gvi(struct ncsi_request *nr)
{
struct ncsi_rsp_gvi_pkt *rsp;
@@ -932,7 +973,7 @@ static struct ncsi_rsp_handler {
{ NCSI_PKT_RSP_GNS, 172, ncsi_rsp_handler_gns },
{ NCSI_PKT_RSP_GNPTS, 172, ncsi_rsp_handler_gnpts },
{ NCSI_PKT_RSP_GPS, 8, ncsi_rsp_handler_gps },
- { NCSI_PKT_RSP_OEM, 0, NULL },
+ { NCSI_PKT_RSP_OEM, -1, ncsi_rsp_handler_oem },
{ NCSI_PKT_RSP_PLDM, 0, NULL },
{ NCSI_PKT_RSP_GPUUID, 20, ncsi_rsp_handler_gpuuid }
};
--
2.17.1
^ permalink raw reply related
* Re: [PATCH iproute2] lib/libnetlink: fix response seq check
From: Vlad Dumitrescu @ 2018-10-03 23:01 UTC (permalink / raw)
To: stephen, chrism, Mahesh Bandewar; +Cc: netdev, Vlad Dumitrescu
In-Reply-To: <20180928171339.4332-1-vlad@dumitrescu.ro>
Hi,
On Fri, Sep 28, 2018 at 10:14 AM <vlad@dumitrescu.ro> wrote:
>
> From: Vlad Dumitrescu <vladum@google.com>
>
> Taking a one-iovec example, with rtnl->seq at 42. iovlen == 1, seq
> becomes 43 on line 604, and a message is sent with nlmsg_seq == 43. If
> a response with nlmsg_seq of 42 is received, the condition being fixed
> in this patch would incorrectly accept it.
>
> Fixes: 72a2ff3916e5 ("lib/libnetlink: Add a new function rtnl_talk_iov")
> Signed-off-by: Vlad Dumitrescu <vladum@google.com>
> ---
> lib/libnetlink.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/libnetlink.c b/lib/libnetlink.c
> index f18dceac..4d2416bf 100644
> --- a/lib/libnetlink.c
> +++ b/lib/libnetlink.c
> @@ -647,7 +647,7 @@ static int __rtnl_talk_iov(struct rtnl_handle *rtnl, struct iovec *iov,
>
> if (nladdr.nl_pid != 0 ||
> h->nlmsg_pid != rtnl->local.nl_pid ||
> - h->nlmsg_seq > seq || h->nlmsg_seq < seq - iovlen) {
> + h->nlmsg_seq > seq || h->nlmsg_seq < seq - iovlen + 1) {
> /* Don't forget to skip that message. */
> status -= NLMSG_ALIGN(len);
> h = (struct nlmsghdr *)((char *)h + NLMSG_ALIGN(len));
> --
> 2.19.0.605.g01d371f741-goog
Did anybody get a chance to review this? I'm not 100% sure I'm fixing
the right thing.
Thanks,
Vlad
^ permalink raw reply
* Re: Bad MAINTAINERS pattern in section 'NETWORKING [IPSEC]'
From: Steffen Klassert @ 2018-10-04 5:28 UTC (permalink / raw)
To: Joe Perches
Cc: linux-kernel, Herbert Xu, David S . Miller, netdev,
Florian Westphal
In-Reply-To: <20180928220123.30999-1-joe@perches.com>
On Fri, Sep 28, 2018 at 03:01:23PM -0700, Joe Perches wrote:
> Please fix this defect appropriately.
>
> linux-next MAINTAINERS section:
>
> 10233 NETWORKING [IPSEC]
> 10234 M: Steffen Klassert <steffen.klassert@secunet.com>
> 10235 M: Herbert Xu <herbert@gondor.apana.org.au>
> 10236 M: "David S. Miller" <davem@davemloft.net>
> 10237 L: netdev@vger.kernel.org
> 10238 T: git git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git
> 10239 T: git git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git
> 10240 S: Maintained
> --> 10241 F: net/core/flow.c
This file went away with the flowcache removal.
I plan to apply the patch below to the ipsec tree:
Subject: [PATCH] MAINTAINERS: Remove net/core/flow.c
net/core/flow.c does not exist anymore, so remove it
from the IPSEC NETWORKING section of the MAINTAINERS
file.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
MAINTAINERS | 1 -
1 file changed, 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index dcb0191c4f54..4ff21dac9b45 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10130,7 +10130,6 @@ L: netdev@vger.kernel.org
T: git git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git
T: git git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git
S: Maintained
-F: net/core/flow.c
F: net/xfrm/
F: net/key/
F: net/ipv4/xfrm*
--
2.17.1
^ permalink raw reply related
* [PATCH net-next] net/neigh: Extend dump filter to proxy neighbor dumps
From: David Ahern @ 2018-10-03 22:33 UTC (permalink / raw)
To: netdev; +Cc: David Ahern
From: David Ahern <dsahern@gmail.com>
Move the attribute parsing from neigh_dump_table to neigh_dump_info, and
pass the filter arguments down to neigh_dump_table in a new struct. Add
the filter option to proxy neigh dumps as well to make them consistent.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/core/neighbour.c | 72 ++++++++++++++++++++++++++++++----------------------
1 file changed, 42 insertions(+), 30 deletions(-)
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 20e0d3308148..fb023df48b83 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2329,35 +2329,24 @@ static bool neigh_ifindex_filtered(struct net_device *dev, int filter_idx)
return false;
}
+struct neigh_dump_filter {
+ int master_idx;
+ int dev_idx;
+};
+
static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
- struct netlink_callback *cb)
+ struct netlink_callback *cb,
+ struct neigh_dump_filter *filter)
{
struct net *net = sock_net(skb->sk);
- const struct nlmsghdr *nlh = cb->nlh;
- struct nlattr *tb[NDA_MAX + 1];
struct neighbour *n;
int rc, h, s_h = cb->args[1];
int idx, s_idx = idx = cb->args[2];
struct neigh_hash_table *nht;
- int filter_master_idx = 0, filter_idx = 0;
unsigned int flags = NLM_F_MULTI;
- int err;
- err = nlmsg_parse(nlh, sizeof(struct ndmsg), tb, NDA_MAX, NULL, NULL);
- if (!err) {
- if (tb[NDA_IFINDEX]) {
- if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32))
- return -EINVAL;
- filter_idx = nla_get_u32(tb[NDA_IFINDEX]);
- }
- if (tb[NDA_MASTER]) {
- if (nla_len(tb[NDA_MASTER]) != sizeof(u32))
- return -EINVAL;
- filter_master_idx = nla_get_u32(tb[NDA_MASTER]);
- }
- if (filter_idx || filter_master_idx)
- flags |= NLM_F_DUMP_FILTERED;
- }
+ if (filter->dev_idx || filter->master_idx)
+ flags |= NLM_F_DUMP_FILTERED;
rcu_read_lock_bh();
nht = rcu_dereference_bh(tbl->nht);
@@ -2370,8 +2359,8 @@ static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
n = rcu_dereference_bh(n->next)) {
if (idx < s_idx || !net_eq(dev_net(n->dev), net))
goto next;
- if (neigh_ifindex_filtered(n->dev, filter_idx) ||
- neigh_master_filtered(n->dev, filter_master_idx))
+ if (neigh_ifindex_filtered(n->dev, filter->dev_idx) ||
+ neigh_master_filtered(n->dev, filter->master_idx))
goto next;
if (neigh_fill_info(skb, n, NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq,
@@ -2393,12 +2382,17 @@ static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
}
static int pneigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
- struct netlink_callback *cb)
+ struct netlink_callback *cb,
+ struct neigh_dump_filter *filter)
{
struct pneigh_entry *n;
struct net *net = sock_net(skb->sk);
int rc, h, s_h = cb->args[3];
int idx, s_idx = idx = cb->args[4];
+ unsigned int flags = NLM_F_MULTI;
+
+ if (filter->dev_idx || filter->master_idx)
+ flags |= NLM_F_DUMP_FILTERED;
read_lock_bh(&tbl->lock);
@@ -2408,10 +2402,12 @@ static int pneigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
for (n = tbl->phash_buckets[h], idx = 0; n; n = n->next) {
if (idx < s_idx || pneigh_net(n) != net)
goto next;
+ if (neigh_ifindex_filtered(n->dev, filter->dev_idx) ||
+ neigh_master_filtered(n->dev, filter->master_idx))
+ goto next;
if (pneigh_fill_info(skb, n, NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq,
- RTM_NEWNEIGH,
- NLM_F_MULTI, tbl) < 0) {
+ RTM_NEWNEIGH, flags, tbl) < 0) {
read_unlock_bh(&tbl->lock);
rc = -1;
goto out;
@@ -2432,20 +2428,36 @@ static int pneigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
static int neigh_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
{
+ const struct nlmsghdr *nlh = cb->nlh;
+ struct neigh_dump_filter filter = {};
+ struct nlattr *tb[NDA_MAX + 1];
struct neigh_table *tbl;
int t, family, s_t;
int proxy = 0;
int err;
- family = ((struct rtgenmsg *) nlmsg_data(cb->nlh))->rtgen_family;
+ family = ((struct rtgenmsg *)nlmsg_data(nlh))->rtgen_family;
/* check for full ndmsg structure presence, family member is
* the same for both structures
*/
- if (nlmsg_len(cb->nlh) >= sizeof(struct ndmsg) &&
- ((struct ndmsg *) nlmsg_data(cb->nlh))->ndm_flags == NTF_PROXY)
+ if (nlmsg_len(nlh) >= sizeof(struct ndmsg) &&
+ ((struct ndmsg *)nlmsg_data(nlh))->ndm_flags == NTF_PROXY)
proxy = 1;
+ err = nlmsg_parse(nlh, sizeof(struct ndmsg), tb, NDA_MAX, NULL, NULL);
+ if (!err) {
+ if (tb[NDA_IFINDEX]) {
+ if (nla_len(tb[NDA_IFINDEX]) != sizeof(u32))
+ return -EINVAL;
+ filter.dev_idx = nla_get_u32(tb[NDA_IFINDEX]);
+ }
+ if (tb[NDA_MASTER]) {
+ if (nla_len(tb[NDA_MASTER]) != sizeof(u32))
+ return -EINVAL;
+ filter.master_idx = nla_get_u32(tb[NDA_MASTER]);
+ }
+ }
s_t = cb->args[0];
for (t = 0; t < NEIGH_NR_TABLES; t++) {
@@ -2459,9 +2471,9 @@ static int neigh_dump_info(struct sk_buff *skb, struct netlink_callback *cb)
memset(&cb->args[1], 0, sizeof(cb->args) -
sizeof(cb->args[0]));
if (proxy)
- err = pneigh_dump_table(tbl, skb, cb);
+ err = pneigh_dump_table(tbl, skb, cb, &filter);
else
- err = neigh_dump_table(tbl, skb, cb);
+ err = neigh_dump_table(tbl, skb, cb, &filter);
if (err < 0)
break;
}
--
2.11.0
^ permalink raw reply related
* [PATCH bpf-next 3/6] libbpf: Consistent prefixes for interfaces in nlattr.h.
From: Andrey Ignatov @ 2018-10-03 22:26 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, kernel-team
In-Reply-To: <cover.1538605140.git.rdna@fb.com>
libbpf is used more and more outside kernel tree. That means the library
should follow good practices in library design and implementation to
play well with third party code that uses it.
One of such practices is to have a common prefix (or a few) for every
interface, function or data structure, library provides. I helps to
avoid name conflicts with other libraries and keeps API consistent.
Inconsistent names in libbpf already cause problems in real life. E.g.
an application can't use both libbpf and libnl due to conflicting
symbols.
Having common prefix will help to fix current and avoid future problems.
libbpf already uses the following prefixes for its interfaces:
* bpf_ for bpf system call wrappers, program/map/elf-object
abstractions and a few other things;
* btf_ for BTF related API;
* libbpf_ for everything else.
The patch adds libbpf_ prefix to interfaces in nlattr.h that use none of
mentioned above prefixes and doesn't fit well into the first two
categories.
Since affected part of API is used in bpftool, the patch applies
corresponding change to bpftool as well. Having it in a separate patch
will cause a state of tree where bpftool is broken what may not be a
good idea.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
tools/bpf/bpftool/net.c | 10 +++--
tools/bpf/bpftool/netlink_dumper.c | 32 ++++++++-------
tools/lib/bpf/netlink.c | 10 ++---
tools/lib/bpf/nlattr.c | 64 ++++++++++++++++--------------
tools/lib/bpf/nlattr.h | 59 +++++++++++++--------------
5 files changed, 94 insertions(+), 81 deletions(-)
diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
index ef83e8a08490..d441bb7035ca 100644
--- a/tools/bpf/bpftool/net.c
+++ b/tools/bpf/bpftool/net.c
@@ -69,7 +69,9 @@ static int dump_link_nlmsg(void *cookie, void *msg, struct nlattr **tb)
snprintf(netinfo->devices[netinfo->used_len].devname,
sizeof(netinfo->devices[netinfo->used_len].devname),
"%s",
- tb[IFLA_IFNAME] ? nla_getattr_str(tb[IFLA_IFNAME]) : "");
+ tb[IFLA_IFNAME]
+ ? libbpf_nla_getattr_str(tb[IFLA_IFNAME])
+ : "");
netinfo->used_len++;
return do_xdp_dump(ifinfo, tb);
@@ -83,7 +85,7 @@ static int dump_class_qdisc_nlmsg(void *cookie, void *msg, struct nlattr **tb)
if (tcinfo->is_qdisc) {
/* skip clsact qdisc */
if (tb[TCA_KIND] &&
- strcmp(nla_data(tb[TCA_KIND]), "clsact") == 0)
+ strcmp(libbpf_nla_data(tb[TCA_KIND]), "clsact") == 0)
return 0;
if (info->tcm_handle == 0)
return 0;
@@ -101,7 +103,9 @@ static int dump_class_qdisc_nlmsg(void *cookie, void *msg, struct nlattr **tb)
snprintf(tcinfo->handle_array[tcinfo->used_len].kind,
sizeof(tcinfo->handle_array[tcinfo->used_len].kind),
"%s",
- tb[TCA_KIND] ? nla_getattr_str(tb[TCA_KIND]) : "unknown");
+ tb[TCA_KIND]
+ ? libbpf_nla_getattr_str(tb[TCA_KIND])
+ : "unknown");
tcinfo->used_len++;
return 0;
diff --git a/tools/bpf/bpftool/netlink_dumper.c b/tools/bpf/bpftool/netlink_dumper.c
index 6f5e9cc6836c..4e9f4531269f 100644
--- a/tools/bpf/bpftool/netlink_dumper.c
+++ b/tools/bpf/bpftool/netlink_dumper.c
@@ -21,7 +21,7 @@ static void xdp_dump_prog_id(struct nlattr **tb, int attr,
if (new_json_object)
NET_START_OBJECT
NET_DUMP_STR("mode", " %s", mode);
- NET_DUMP_UINT("id", " id %u", nla_getattr_u32(tb[attr]))
+ NET_DUMP_UINT("id", " id %u", libbpf_nla_getattr_u32(tb[attr]))
if (new_json_object)
NET_END_OBJECT
}
@@ -32,13 +32,13 @@ static int do_xdp_dump_one(struct nlattr *attr, unsigned int ifindex,
struct nlattr *tb[IFLA_XDP_MAX + 1];
unsigned char mode;
- if (nla_parse_nested(tb, IFLA_XDP_MAX, attr, NULL) < 0)
+ if (libbpf_nla_parse_nested(tb, IFLA_XDP_MAX, attr, NULL) < 0)
return -1;
if (!tb[IFLA_XDP_ATTACHED])
return 0;
- mode = nla_getattr_u8(tb[IFLA_XDP_ATTACHED]);
+ mode = libbpf_nla_getattr_u8(tb[IFLA_XDP_ATTACHED]);
if (mode == XDP_ATTACHED_NONE)
return 0;
@@ -75,14 +75,14 @@ int do_xdp_dump(struct ifinfomsg *ifinfo, struct nlattr **tb)
return 0;
return do_xdp_dump_one(tb[IFLA_XDP], ifinfo->ifi_index,
- nla_getattr_str(tb[IFLA_IFNAME]));
+ libbpf_nla_getattr_str(tb[IFLA_IFNAME]));
}
static int do_bpf_dump_one_act(struct nlattr *attr)
{
struct nlattr *tb[TCA_ACT_BPF_MAX + 1];
- if (nla_parse_nested(tb, TCA_ACT_BPF_MAX, attr, NULL) < 0)
+ if (libbpf_nla_parse_nested(tb, TCA_ACT_BPF_MAX, attr, NULL) < 0)
return -LIBBPF_ERRNO__NLPARSE;
if (!tb[TCA_ACT_BPF_PARMS])
@@ -91,10 +91,10 @@ static int do_bpf_dump_one_act(struct nlattr *attr)
NET_START_OBJECT_NESTED2;
if (tb[TCA_ACT_BPF_NAME])
NET_DUMP_STR("name", "%s",
- nla_getattr_str(tb[TCA_ACT_BPF_NAME]));
+ libbpf_nla_getattr_str(tb[TCA_ACT_BPF_NAME]));
if (tb[TCA_ACT_BPF_ID])
NET_DUMP_UINT("id", " id %u",
- nla_getattr_u32(tb[TCA_ACT_BPF_ID]));
+ libbpf_nla_getattr_u32(tb[TCA_ACT_BPF_ID]));
NET_END_OBJECT_NESTED;
return 0;
}
@@ -106,10 +106,11 @@ static int do_dump_one_act(struct nlattr *attr)
if (!attr)
return 0;
- if (nla_parse_nested(tb, TCA_ACT_MAX, attr, NULL) < 0)
+ if (libbpf_nla_parse_nested(tb, TCA_ACT_MAX, attr, NULL) < 0)
return -LIBBPF_ERRNO__NLPARSE;
- if (tb[TCA_ACT_KIND] && strcmp(nla_data(tb[TCA_ACT_KIND]), "bpf") == 0)
+ if (tb[TCA_ACT_KIND] &&
+ strcmp(libbpf_nla_data(tb[TCA_ACT_KIND]), "bpf") == 0)
return do_bpf_dump_one_act(tb[TCA_ACT_OPTIONS]);
return 0;
@@ -120,7 +121,7 @@ static int do_bpf_act_dump(struct nlattr *attr)
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
int act, ret;
- if (nla_parse_nested(tb, TCA_ACT_MAX_PRIO, attr, NULL) < 0)
+ if (libbpf_nla_parse_nested(tb, TCA_ACT_MAX_PRIO, attr, NULL) < 0)
return -LIBBPF_ERRNO__NLPARSE;
NET_START_ARRAY("act", " %s [");
@@ -139,13 +140,15 @@ static int do_bpf_filter_dump(struct nlattr *attr)
struct nlattr *tb[TCA_BPF_MAX + 1];
int ret;
- if (nla_parse_nested(tb, TCA_BPF_MAX, attr, NULL) < 0)
+ if (libbpf_nla_parse_nested(tb, TCA_BPF_MAX, attr, NULL) < 0)
return -LIBBPF_ERRNO__NLPARSE;
if (tb[TCA_BPF_NAME])
- NET_DUMP_STR("name", " %s", nla_getattr_str(tb[TCA_BPF_NAME]));
+ NET_DUMP_STR("name", " %s",
+ libbpf_nla_getattr_str(tb[TCA_BPF_NAME]));
if (tb[TCA_BPF_ID])
- NET_DUMP_UINT("id", " id %u", nla_getattr_u32(tb[TCA_BPF_ID]));
+ NET_DUMP_UINT("id", " id %u",
+ libbpf_nla_getattr_u32(tb[TCA_BPF_ID]));
if (tb[TCA_BPF_ACT]) {
ret = do_bpf_act_dump(tb[TCA_BPF_ACT]);
if (ret)
@@ -160,7 +163,8 @@ int do_filter_dump(struct tcmsg *info, struct nlattr **tb, const char *kind,
{
int ret = 0;
- if (tb[TCA_OPTIONS] && strcmp(nla_data(tb[TCA_KIND]), "bpf") == 0) {
+ if (tb[TCA_OPTIONS] &&
+ strcmp(libbpf_nla_data(tb[TCA_KIND]), "bpf") == 0) {
NET_START_OBJECT;
if (devname[0] != '\0')
NET_DUMP_STR("devname", "%s", devname);
diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c
index 506bdfdbcab0..2d2edbbd8ae8 100644
--- a/tools/lib/bpf/netlink.c
+++ b/tools/lib/bpf/netlink.c
@@ -103,7 +103,7 @@ static int bpf_netlink_recv(int sock, __u32 nl_pid, int seq,
if (!err->error)
continue;
ret = err->error;
- nla_dump_errormsg(nh);
+ libbpf_nla_dump_errormsg(nh);
goto done;
case NLMSG_DONE:
return 0;
@@ -190,7 +190,7 @@ static int __dump_link_nlmsg(struct nlmsghdr *nlh,
len = nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*ifi));
attr = (struct nlattr *) ((void *) ifi + NLMSG_ALIGN(sizeof(*ifi)));
- if (nla_parse(tb, IFLA_MAX, attr, len, NULL) != 0)
+ if (libbpf_nla_parse(tb, IFLA_MAX, attr, len, NULL) != 0)
return -LIBBPF_ERRNO__NLPARSE;
return dump_link_nlmsg(cookie, ifi, tb);
@@ -228,7 +228,7 @@ static int __dump_class_nlmsg(struct nlmsghdr *nlh,
len = nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*t));
attr = (struct nlattr *) ((void *) t + NLMSG_ALIGN(sizeof(*t)));
- if (nla_parse(tb, TCA_MAX, attr, len, NULL) != 0)
+ if (libbpf_nla_parse(tb, TCA_MAX, attr, len, NULL) != 0)
return -LIBBPF_ERRNO__NLPARSE;
return dump_class_nlmsg(cookie, t, tb);
@@ -267,7 +267,7 @@ static int __dump_qdisc_nlmsg(struct nlmsghdr *nlh,
len = nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*t));
attr = (struct nlattr *) ((void *) t + NLMSG_ALIGN(sizeof(*t)));
- if (nla_parse(tb, TCA_MAX, attr, len, NULL) != 0)
+ if (libbpf_nla_parse(tb, TCA_MAX, attr, len, NULL) != 0)
return -LIBBPF_ERRNO__NLPARSE;
return dump_qdisc_nlmsg(cookie, t, tb);
@@ -306,7 +306,7 @@ static int __dump_filter_nlmsg(struct nlmsghdr *nlh,
len = nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*t));
attr = (struct nlattr *) ((void *) t + NLMSG_ALIGN(sizeof(*t)));
- if (nla_parse(tb, TCA_MAX, attr, len, NULL) != 0)
+ if (libbpf_nla_parse(tb, TCA_MAX, attr, len, NULL) != 0)
return -LIBBPF_ERRNO__NLPARSE;
return dump_filter_nlmsg(cookie, t, tb);
diff --git a/tools/lib/bpf/nlattr.c b/tools/lib/bpf/nlattr.c
index 49f514119bdb..e52257a7367a 100644
--- a/tools/lib/bpf/nlattr.c
+++ b/tools/lib/bpf/nlattr.c
@@ -17,13 +17,13 @@
#include <string.h>
#include <stdio.h>
-static uint16_t nla_attr_minlen[NLA_TYPE_MAX+1] = {
- [NLA_U8] = sizeof(uint8_t),
- [NLA_U16] = sizeof(uint16_t),
- [NLA_U32] = sizeof(uint32_t),
- [NLA_U64] = sizeof(uint64_t),
- [NLA_STRING] = 1,
- [NLA_FLAG] = 0,
+static uint16_t nla_attr_minlen[LIBBPF_NLA_TYPE_MAX+1] = {
+ [LIBBPF_NLA_U8] = sizeof(uint8_t),
+ [LIBBPF_NLA_U16] = sizeof(uint16_t),
+ [LIBBPF_NLA_U32] = sizeof(uint32_t),
+ [LIBBPF_NLA_U64] = sizeof(uint64_t),
+ [LIBBPF_NLA_STRING] = 1,
+ [LIBBPF_NLA_FLAG] = 0,
};
static struct nlattr *nla_next(const struct nlattr *nla, int *remaining)
@@ -47,9 +47,9 @@ static int nla_type(const struct nlattr *nla)
}
static int validate_nla(struct nlattr *nla, int maxtype,
- struct nla_policy *policy)
+ struct libbpf_nla_policy *policy)
{
- struct nla_policy *pt;
+ struct libbpf_nla_policy *pt;
unsigned int minlen = 0;
int type = nla_type(nla);
@@ -58,23 +58,24 @@ static int validate_nla(struct nlattr *nla, int maxtype,
pt = &policy[type];
- if (pt->type > NLA_TYPE_MAX)
+ if (pt->type > LIBBPF_NLA_TYPE_MAX)
return 0;
if (pt->minlen)
minlen = pt->minlen;
- else if (pt->type != NLA_UNSPEC)
+ else if (pt->type != LIBBPF_NLA_UNSPEC)
minlen = nla_attr_minlen[pt->type];
- if (nla_len(nla) < minlen)
+ if (libbpf_nla_len(nla) < minlen)
return -1;
- if (pt->maxlen && nla_len(nla) > pt->maxlen)
+ if (pt->maxlen && libbpf_nla_len(nla) > pt->maxlen)
return -1;
- if (pt->type == NLA_STRING) {
- char *data = nla_data(nla);
- if (data[nla_len(nla) - 1] != '\0')
+ if (pt->type == LIBBPF_NLA_STRING) {
+ char *data = libbpf_nla_data(nla);
+
+ if (data[libbpf_nla_len(nla) - 1] != '\0')
return -1;
}
@@ -104,15 +105,15 @@ static inline int nlmsg_len(const struct nlmsghdr *nlh)
* @see nla_validate
* @return 0 on success or a negative error code.
*/
-int nla_parse(struct nlattr *tb[], int maxtype, struct nlattr *head, int len,
- struct nla_policy *policy)
+int libbpf_nla_parse(struct nlattr *tb[], int maxtype, struct nlattr *head,
+ int len, struct libbpf_nla_policy *policy)
{
struct nlattr *nla;
int rem, err;
memset(tb, 0, sizeof(struct nlattr *) * (maxtype + 1));
- nla_for_each_attr(nla, head, len, rem) {
+ libbpf_nla_for_each_attr(nla, head, len, rem) {
int type = nla_type(nla);
if (type > maxtype)
@@ -144,23 +145,25 @@ int nla_parse(struct nlattr *tb[], int maxtype, struct nlattr *head, int len,
* @arg policy Attribute validation policy.
*
* Feeds the stream of attributes nested into the specified attribute
- * to nla_parse().
+ * to libbpf_nla_parse().
*
- * @see nla_parse
+ * @see libbpf_nla_parse
* @return 0 on success or a negative error code.
*/
-int nla_parse_nested(struct nlattr *tb[], int maxtype, struct nlattr *nla,
- struct nla_policy *policy)
+int libbpf_nla_parse_nested(struct nlattr *tb[], int maxtype,
+ struct nlattr *nla,
+ struct libbpf_nla_policy *policy)
{
- return nla_parse(tb, maxtype, nla_data(nla), nla_len(nla), policy);
+ return libbpf_nla_parse(tb, maxtype, libbpf_nla_data(nla),
+ libbpf_nla_len(nla), policy);
}
/* dump netlink extended ack error message */
-int nla_dump_errormsg(struct nlmsghdr *nlh)
+int libbpf_nla_dump_errormsg(struct nlmsghdr *nlh)
{
- struct nla_policy extack_policy[NLMSGERR_ATTR_MAX + 1] = {
- [NLMSGERR_ATTR_MSG] = { .type = NLA_STRING },
- [NLMSGERR_ATTR_OFFS] = { .type = NLA_U32 },
+ struct libbpf_nla_policy extack_policy[NLMSGERR_ATTR_MAX + 1] = {
+ [NLMSGERR_ATTR_MSG] = { .type = LIBBPF_NLA_STRING },
+ [NLMSGERR_ATTR_OFFS] = { .type = LIBBPF_NLA_U32 },
};
struct nlattr *tb[NLMSGERR_ATTR_MAX + 1], *attr;
struct nlmsgerr *err;
@@ -181,14 +184,15 @@ int nla_dump_errormsg(struct nlmsghdr *nlh)
attr = (struct nlattr *) ((void *) err + hlen);
alen = nlh->nlmsg_len - hlen;
- if (nla_parse(tb, NLMSGERR_ATTR_MAX, attr, alen, extack_policy) != 0) {
+ if (libbpf_nla_parse(tb, NLMSGERR_ATTR_MAX, attr, alen,
+ extack_policy) != 0) {
fprintf(stderr,
"Failed to parse extended error attributes\n");
return 0;
}
if (tb[NLMSGERR_ATTR_MSG])
- errmsg = (char *) nla_data(tb[NLMSGERR_ATTR_MSG]);
+ errmsg = (char *) libbpf_nla_data(tb[NLMSGERR_ATTR_MSG]);
fprintf(stderr, "Kernel error message: %s\n", errmsg);
diff --git a/tools/lib/bpf/nlattr.h b/tools/lib/bpf/nlattr.h
index a6e2396bce7c..755a3312c87f 100644
--- a/tools/lib/bpf/nlattr.h
+++ b/tools/lib/bpf/nlattr.h
@@ -23,19 +23,19 @@
* Standard attribute types to specify validation policy
*/
enum {
- NLA_UNSPEC, /**< Unspecified type, binary data chunk */
- NLA_U8, /**< 8 bit integer */
- NLA_U16, /**< 16 bit integer */
- NLA_U32, /**< 32 bit integer */
- NLA_U64, /**< 64 bit integer */
- NLA_STRING, /**< NUL terminated character string */
- NLA_FLAG, /**< Flag */
- NLA_MSECS, /**< Micro seconds (64bit) */
- NLA_NESTED, /**< Nested attributes */
- __NLA_TYPE_MAX,
+ LIBBPF_NLA_UNSPEC, /**< Unspecified type, binary data chunk */
+ LIBBPF_NLA_U8, /**< 8 bit integer */
+ LIBBPF_NLA_U16, /**< 16 bit integer */
+ LIBBPF_NLA_U32, /**< 32 bit integer */
+ LIBBPF_NLA_U64, /**< 64 bit integer */
+ LIBBPF_NLA_STRING, /**< NUL terminated character string */
+ LIBBPF_NLA_FLAG, /**< Flag */
+ LIBBPF_NLA_MSECS, /**< Micro seconds (64bit) */
+ LIBBPF_NLA_NESTED, /**< Nested attributes */
+ __LIBBPF_NLA_TYPE_MAX,
};
-#define NLA_TYPE_MAX (__NLA_TYPE_MAX - 1)
+#define LIBBPF_NLA_TYPE_MAX (__LIBBPF_NLA_TYPE_MAX - 1)
/**
* @ingroup attr
@@ -43,8 +43,8 @@ enum {
*
* See section @core_doc{core_attr_parse,Attribute Parsing} for more details.
*/
-struct nla_policy {
- /** Type of attribute or NLA_UNSPEC */
+struct libbpf_nla_policy {
+ /** Type of attribute or LIBBPF_NLA_UNSPEC */
uint16_t type;
/** Minimal length of payload required */
@@ -62,49 +62,50 @@ struct nla_policy {
* @arg len length of attribute stream
* @arg rem initialized to len, holds bytes currently remaining in stream
*/
-#define nla_for_each_attr(pos, head, len, rem) \
+#define libbpf_nla_for_each_attr(pos, head, len, rem) \
for (pos = head, rem = len; \
nla_ok(pos, rem); \
pos = nla_next(pos, &(rem)))
/**
- * nla_data - head of payload
+ * libbpf_nla_data - head of payload
* @nla: netlink attribute
*/
-static inline void *nla_data(const struct nlattr *nla)
+static inline void *libbpf_nla_data(const struct nlattr *nla)
{
return (char *) nla + NLA_HDRLEN;
}
-static inline uint8_t nla_getattr_u8(const struct nlattr *nla)
+static inline uint8_t libbpf_nla_getattr_u8(const struct nlattr *nla)
{
- return *(uint8_t *)nla_data(nla);
+ return *(uint8_t *)libbpf_nla_data(nla);
}
-static inline uint32_t nla_getattr_u32(const struct nlattr *nla)
+static inline uint32_t libbpf_nla_getattr_u32(const struct nlattr *nla)
{
- return *(uint32_t *)nla_data(nla);
+ return *(uint32_t *)libbpf_nla_data(nla);
}
-static inline const char *nla_getattr_str(const struct nlattr *nla)
+static inline const char *libbpf_nla_getattr_str(const struct nlattr *nla)
{
- return (const char *)nla_data(nla);
+ return (const char *)libbpf_nla_data(nla);
}
/**
- * nla_len - length of payload
+ * libbpf_nla_len - length of payload
* @nla: netlink attribute
*/
-static inline int nla_len(const struct nlattr *nla)
+static inline int libbpf_nla_len(const struct nlattr *nla)
{
return nla->nla_len - NLA_HDRLEN;
}
-int nla_parse(struct nlattr *tb[], int maxtype, struct nlattr *head, int len,
- struct nla_policy *policy);
-int nla_parse_nested(struct nlattr *tb[], int maxtype, struct nlattr *nla,
- struct nla_policy *policy);
+int libbpf_nla_parse(struct nlattr *tb[], int maxtype, struct nlattr *head,
+ int len, struct libbpf_nla_policy *policy);
+int libbpf_nla_parse_nested(struct nlattr *tb[], int maxtype,
+ struct nlattr *nla,
+ struct libbpf_nla_policy *policy);
-int nla_dump_errormsg(struct nlmsghdr *nlh);
+int libbpf_nla_dump_errormsg(struct nlmsghdr *nlh);
#endif /* __NLATTR_H */
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 4/6] libbpf: Consistent prefixes for interfaces in str_error.h.
From: Andrey Ignatov @ 2018-10-03 22:26 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, kernel-team
In-Reply-To: <cover.1538605140.git.rdna@fb.com>
libbpf is used more and more outside kernel tree. That means the library
should follow good practices in library design and implementation to
play well with third party code that uses it.
One of such practices is to have a common prefix (or a few) for every
interface, function or data structure, library provides. I helps to
avoid name conflicts with other libraries and keeps API consistent.
Inconsistent names in libbpf already cause problems in real life. E.g.
an application can't use both libbpf and libnl due to conflicting
symbols.
Having common prefix will help to fix current and avoid future problems.
libbpf already uses the following prefixes for its interfaces:
* bpf_ for bpf system call wrappers, program/map/elf-object
abstractions and a few other things;
* btf_ for BTF related API;
* libbpf_ for everything else.
The patch renames function in str_error.h to have libbpf_ prefix since it
misses one and doesn't fit well into the first two categories.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
tools/lib/bpf/libbpf.c | 20 +++++++++++---------
tools/lib/bpf/str_error.c | 2 +-
tools/lib/bpf/str_error.h | 2 +-
3 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 9e68fd9fcfca..02888d36b805 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -470,7 +470,8 @@ static int bpf_object__elf_init(struct bpf_object *obj)
obj->efile.fd = open(obj->path, O_RDONLY);
if (obj->efile.fd < 0) {
char errmsg[STRERR_BUFSIZE];
- char *cp = str_error(errno, errmsg, sizeof(errmsg));
+ char *cp = libbpf_strerror_r(errno, errmsg,
+ sizeof(errmsg));
pr_warning("failed to open %s: %s\n", obj->path, cp);
return -errno;
@@ -811,7 +812,8 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
data->d_size, name, idx);
if (err) {
char errmsg[STRERR_BUFSIZE];
- char *cp = str_error(-err, errmsg, sizeof(errmsg));
+ char *cp = libbpf_strerror_r(-err, errmsg,
+ sizeof(errmsg));
pr_warning("failed to alloc program %s (%s): %s",
name, obj->path, cp);
@@ -1140,7 +1142,7 @@ bpf_object__create_maps(struct bpf_object *obj)
*pfd = bpf_create_map_xattr(&create_attr);
if (*pfd < 0 && create_attr.btf_key_type_id) {
- cp = str_error(errno, errmsg, sizeof(errmsg));
+ cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
pr_warning("Error in bpf_create_map_xattr(%s):%s(%d). Retrying without BTF.\n",
map->name, cp, errno);
create_attr.btf_fd = 0;
@@ -1155,7 +1157,7 @@ bpf_object__create_maps(struct bpf_object *obj)
size_t j;
err = *pfd;
- cp = str_error(errno, errmsg, sizeof(errmsg));
+ cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
pr_warning("failed to create map (name: '%s'): %s\n",
map->name, cp);
for (j = 0; j < i; j++)
@@ -1339,7 +1341,7 @@ load_program(enum bpf_prog_type type, enum bpf_attach_type expected_attach_type,
}
ret = -LIBBPF_ERRNO__LOAD;
- cp = str_error(errno, errmsg, sizeof(errmsg));
+ cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
pr_warning("load bpf program failed: %s\n", cp);
if (log_buf && log_buf[0] != '\0') {
@@ -1655,7 +1657,7 @@ static int check_path(const char *path)
dir = dirname(dname);
if (statfs(dir, &st_fs)) {
- cp = str_error(errno, errmsg, sizeof(errmsg));
+ cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
pr_warning("failed to statfs %s: %s\n", dir, cp);
err = -errno;
}
@@ -1691,7 +1693,7 @@ int bpf_program__pin_instance(struct bpf_program *prog, const char *path,
}
if (bpf_obj_pin(prog->instances.fds[instance], path)) {
- cp = str_error(errno, errmsg, sizeof(errmsg));
+ cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
pr_warning("failed to pin program: %s\n", cp);
return -errno;
}
@@ -1709,7 +1711,7 @@ static int make_dir(const char *path)
err = -errno;
if (err) {
- cp = str_error(-err, errmsg, sizeof(errmsg));
+ cp = libbpf_strerror_r(-err, errmsg, sizeof(errmsg));
pr_warning("failed to mkdir %s: %s\n", path, cp);
}
return err;
@@ -1771,7 +1773,7 @@ int bpf_map__pin(struct bpf_map *map, const char *path)
}
if (bpf_obj_pin(map->fd, path)) {
- cp = str_error(errno, errmsg, sizeof(errmsg));
+ cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
pr_warning("failed to pin map: %s\n", cp);
return -errno;
}
diff --git a/tools/lib/bpf/str_error.c b/tools/lib/bpf/str_error.c
index b8798114a357..3d211b642cb5 100644
--- a/tools/lib/bpf/str_error.c
+++ b/tools/lib/bpf/str_error.c
@@ -9,7 +9,7 @@
* libc, while checking strerror_r() return to avoid having to check this in
* all places calling it.
*/
-char *str_error(int err, char *dst, int len)
+char *libbpf_strerror_r(int err, char *dst, int len)
{
int ret = strerror_r(err, dst, len);
if (ret)
diff --git a/tools/lib/bpf/str_error.h b/tools/lib/bpf/str_error.h
index 355b1db571d1..998eff7d6710 100644
--- a/tools/lib/bpf/str_error.h
+++ b/tools/lib/bpf/str_error.h
@@ -2,5 +2,5 @@
#ifndef BPF_STR_ERROR
#define BPF_STR_ERROR
-char *str_error(int err, char *dst, int len);
+char *libbpf_strerror_r(int err, char *dst, int len);
#endif // BPF_STR_ERROR
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 2/6] libbpf: Consistent prefixes for interfaces in libbpf.h.
From: Andrey Ignatov @ 2018-10-03 22:26 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, kernel-team
In-Reply-To: <cover.1538605140.git.rdna@fb.com>
libbpf is used more and more outside kernel tree. That means the library
should follow good practices in library design and implementation to
play well with third party code that uses it.
One of such practices is to have a common prefix (or a few) for every
interface, function or data structure, library provides. I helps to
avoid name conflicts with other libraries and keeps API consistent.
Inconsistent names in libbpf already cause problems in real life. E.g.
an application can't use both libbpf and libnl due to conflicting
symbols.
Having common prefix will help to fix current and avoid future problems.
libbpf already uses the following prefixes for its interfaces:
* bpf_ for bpf system call wrappers, program/map/elf-object
abstractions and a few other things;
* btf_ for BTF related API;
* libbpf_ for everything else.
The patch adds libbpf_ prefix to functions and typedef in libbpf.h that
use none of mentioned above prefixes and doesn't fit well into the first
two categories.
Since affected part of API is used in bpftool, the patch applies
corresponding change to bpftool as well. Having it in a separate patch
will cause a state of tree where bpftool is broken what may not be a
good idea.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
tools/bpf/bpftool/net.c | 31 +++++++++++++++----------------
tools/lib/bpf/libbpf.h | 20 ++++++++++----------
tools/lib/bpf/netlink.c | 37 ++++++++++++++++++++-----------------
3 files changed, 45 insertions(+), 43 deletions(-)
diff --git a/tools/bpf/bpftool/net.c b/tools/bpf/bpftool/net.c
index ed205ee57655..ef83e8a08490 100644
--- a/tools/bpf/bpftool/net.c
+++ b/tools/bpf/bpftool/net.c
@@ -127,14 +127,14 @@ static int show_dev_tc_bpf(int sock, unsigned int nl_pid,
tcinfo.array_len = 0;
tcinfo.is_qdisc = false;
- ret = nl_get_class(sock, nl_pid, dev->ifindex, dump_class_qdisc_nlmsg,
- &tcinfo);
+ ret = libbpf_nl_get_class(sock, nl_pid, dev->ifindex,
+ dump_class_qdisc_nlmsg, &tcinfo);
if (ret)
goto out;
tcinfo.is_qdisc = true;
- ret = nl_get_qdisc(sock, nl_pid, dev->ifindex, dump_class_qdisc_nlmsg,
- &tcinfo);
+ ret = libbpf_nl_get_qdisc(sock, nl_pid, dev->ifindex,
+ dump_class_qdisc_nlmsg, &tcinfo);
if (ret)
goto out;
@@ -142,10 +142,9 @@ static int show_dev_tc_bpf(int sock, unsigned int nl_pid,
filter_info.ifindex = dev->ifindex;
for (i = 0; i < tcinfo.used_len; i++) {
filter_info.kind = tcinfo.handle_array[i].kind;
- ret = nl_get_filter(sock, nl_pid, dev->ifindex,
- tcinfo.handle_array[i].handle,
- dump_filter_nlmsg,
- &filter_info);
+ ret = libbpf_nl_get_filter(sock, nl_pid, dev->ifindex,
+ tcinfo.handle_array[i].handle,
+ dump_filter_nlmsg, &filter_info);
if (ret)
goto out;
}
@@ -153,22 +152,22 @@ static int show_dev_tc_bpf(int sock, unsigned int nl_pid,
/* root, ingress and egress handle */
handle = TC_H_ROOT;
filter_info.kind = "root";
- ret = nl_get_filter(sock, nl_pid, dev->ifindex, handle,
- dump_filter_nlmsg, &filter_info);
+ ret = libbpf_nl_get_filter(sock, nl_pid, dev->ifindex, handle,
+ dump_filter_nlmsg, &filter_info);
if (ret)
goto out;
handle = TC_H_MAKE(TC_H_CLSACT, TC_H_MIN_INGRESS);
filter_info.kind = "clsact/ingress";
- ret = nl_get_filter(sock, nl_pid, dev->ifindex, handle,
- dump_filter_nlmsg, &filter_info);
+ ret = libbpf_nl_get_filter(sock, nl_pid, dev->ifindex, handle,
+ dump_filter_nlmsg, &filter_info);
if (ret)
goto out;
handle = TC_H_MAKE(TC_H_CLSACT, TC_H_MIN_EGRESS);
filter_info.kind = "clsact/egress";
- ret = nl_get_filter(sock, nl_pid, dev->ifindex, handle,
- dump_filter_nlmsg, &filter_info);
+ ret = libbpf_nl_get_filter(sock, nl_pid, dev->ifindex, handle,
+ dump_filter_nlmsg, &filter_info);
if (ret)
goto out;
@@ -196,7 +195,7 @@ static int do_show(int argc, char **argv)
usage();
}
- sock = bpf_netlink_open(&nl_pid);
+ sock = libbpf_netlink_open(&nl_pid);
if (sock < 0) {
fprintf(stderr, "failed to open netlink sock\n");
return -1;
@@ -211,7 +210,7 @@ static int do_show(int argc, char **argv)
jsonw_start_array(json_wtr);
NET_START_OBJECT;
NET_START_ARRAY("xdp", "%s:\n");
- ret = nl_get_link(sock, nl_pid, dump_link_nlmsg, &dev_array);
+ ret = libbpf_nl_get_link(sock, nl_pid, dump_link_nlmsg, &dev_array);
NET_END_ARRAY("\n");
if (!ret) {
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 8388be525388..710ff5724980 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -305,14 +305,14 @@ int bpf_perf_event_read_simple(void *mem, unsigned long size,
bpf_perf_event_print_t fn, void *priv);
struct nlattr;
-typedef int (*dump_nlmsg_t)(void *cookie, void *msg, struct nlattr **tb);
-int bpf_netlink_open(unsigned int *nl_pid);
-int nl_get_link(int sock, unsigned int nl_pid, dump_nlmsg_t dump_link_nlmsg,
- void *cookie);
-int nl_get_class(int sock, unsigned int nl_pid, int ifindex,
- dump_nlmsg_t dump_class_nlmsg, void *cookie);
-int nl_get_qdisc(int sock, unsigned int nl_pid, int ifindex,
- dump_nlmsg_t dump_qdisc_nlmsg, void *cookie);
-int nl_get_filter(int sock, unsigned int nl_pid, int ifindex, int handle,
- dump_nlmsg_t dump_filter_nlmsg, void *cookie);
+typedef int (*libbpf_dump_nlmsg_t)(void *cookie, void *msg, struct nlattr **tb);
+int libbpf_netlink_open(unsigned int *nl_pid);
+int libbpf_nl_get_link(int sock, unsigned int nl_pid,
+ libbpf_dump_nlmsg_t dump_link_nlmsg, void *cookie);
+int libbpf_nl_get_class(int sock, unsigned int nl_pid, int ifindex,
+ libbpf_dump_nlmsg_t dump_class_nlmsg, void *cookie);
+int libbpf_nl_get_qdisc(int sock, unsigned int nl_pid, int ifindex,
+ libbpf_dump_nlmsg_t dump_qdisc_nlmsg, void *cookie);
+int libbpf_nl_get_filter(int sock, unsigned int nl_pid, int ifindex, int handle,
+ libbpf_dump_nlmsg_t dump_filter_nlmsg, void *cookie);
#endif
diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c
index da46d9358d9d..506bdfdbcab0 100644
--- a/tools/lib/bpf/netlink.c
+++ b/tools/lib/bpf/netlink.c
@@ -18,10 +18,10 @@
#define SOL_NETLINK 270
#endif
-typedef int (*__dump_nlmsg_t)(struct nlmsghdr *nlmsg, dump_nlmsg_t,
+typedef int (*__dump_nlmsg_t)(struct nlmsghdr *nlmsg, libbpf_dump_nlmsg_t,
void *cookie);
-int bpf_netlink_open(__u32 *nl_pid)
+int libbpf_netlink_open(__u32 *nl_pid)
{
struct sockaddr_nl sa;
socklen_t addrlen;
@@ -65,7 +65,7 @@ int bpf_netlink_open(__u32 *nl_pid)
}
static int bpf_netlink_recv(int sock, __u32 nl_pid, int seq,
- __dump_nlmsg_t _fn, dump_nlmsg_t fn,
+ __dump_nlmsg_t _fn, libbpf_dump_nlmsg_t fn,
void *cookie)
{
bool multipart = true;
@@ -133,7 +133,7 @@ int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags)
} req;
__u32 nl_pid;
- sock = bpf_netlink_open(&nl_pid);
+ sock = libbpf_netlink_open(&nl_pid);
if (sock < 0)
return sock;
@@ -181,8 +181,8 @@ int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags)
return ret;
}
-static int __dump_link_nlmsg(struct nlmsghdr *nlh, dump_nlmsg_t dump_link_nlmsg,
- void *cookie)
+static int __dump_link_nlmsg(struct nlmsghdr *nlh,
+ libbpf_dump_nlmsg_t dump_link_nlmsg, void *cookie)
{
struct nlattr *tb[IFLA_MAX + 1], *attr;
struct ifinfomsg *ifi = NLMSG_DATA(nlh);
@@ -196,8 +196,8 @@ static int __dump_link_nlmsg(struct nlmsghdr *nlh, dump_nlmsg_t dump_link_nlmsg,
return dump_link_nlmsg(cookie, ifi, tb);
}
-int nl_get_link(int sock, unsigned int nl_pid, dump_nlmsg_t dump_link_nlmsg,
- void *cookie)
+int libbpf_nl_get_link(int sock, unsigned int nl_pid,
+ libbpf_dump_nlmsg_t dump_link_nlmsg, void *cookie)
{
struct {
struct nlmsghdr nlh;
@@ -219,7 +219,8 @@ int nl_get_link(int sock, unsigned int nl_pid, dump_nlmsg_t dump_link_nlmsg,
}
static int __dump_class_nlmsg(struct nlmsghdr *nlh,
- dump_nlmsg_t dump_class_nlmsg, void *cookie)
+ libbpf_dump_nlmsg_t dump_class_nlmsg,
+ void *cookie)
{
struct nlattr *tb[TCA_MAX + 1], *attr;
struct tcmsg *t = NLMSG_DATA(nlh);
@@ -233,8 +234,8 @@ static int __dump_class_nlmsg(struct nlmsghdr *nlh,
return dump_class_nlmsg(cookie, t, tb);
}
-int nl_get_class(int sock, unsigned int nl_pid, int ifindex,
- dump_nlmsg_t dump_class_nlmsg, void *cookie)
+int libbpf_nl_get_class(int sock, unsigned int nl_pid, int ifindex,
+ libbpf_dump_nlmsg_t dump_class_nlmsg, void *cookie)
{
struct {
struct nlmsghdr nlh;
@@ -257,7 +258,8 @@ int nl_get_class(int sock, unsigned int nl_pid, int ifindex,
}
static int __dump_qdisc_nlmsg(struct nlmsghdr *nlh,
- dump_nlmsg_t dump_qdisc_nlmsg, void *cookie)
+ libbpf_dump_nlmsg_t dump_qdisc_nlmsg,
+ void *cookie)
{
struct nlattr *tb[TCA_MAX + 1], *attr;
struct tcmsg *t = NLMSG_DATA(nlh);
@@ -271,8 +273,8 @@ static int __dump_qdisc_nlmsg(struct nlmsghdr *nlh,
return dump_qdisc_nlmsg(cookie, t, tb);
}
-int nl_get_qdisc(int sock, unsigned int nl_pid, int ifindex,
- dump_nlmsg_t dump_qdisc_nlmsg, void *cookie)
+int libbpf_nl_get_qdisc(int sock, unsigned int nl_pid, int ifindex,
+ libbpf_dump_nlmsg_t dump_qdisc_nlmsg, void *cookie)
{
struct {
struct nlmsghdr nlh;
@@ -295,7 +297,8 @@ int nl_get_qdisc(int sock, unsigned int nl_pid, int ifindex,
}
static int __dump_filter_nlmsg(struct nlmsghdr *nlh,
- dump_nlmsg_t dump_filter_nlmsg, void *cookie)
+ libbpf_dump_nlmsg_t dump_filter_nlmsg,
+ void *cookie)
{
struct nlattr *tb[TCA_MAX + 1], *attr;
struct tcmsg *t = NLMSG_DATA(nlh);
@@ -309,8 +312,8 @@ static int __dump_filter_nlmsg(struct nlmsghdr *nlh,
return dump_filter_nlmsg(cookie, t, tb);
}
-int nl_get_filter(int sock, unsigned int nl_pid, int ifindex, int handle,
- dump_nlmsg_t dump_filter_nlmsg, void *cookie)
+int libbpf_nl_get_filter(int sock, unsigned int nl_pid, int ifindex, int handle,
+ libbpf_dump_nlmsg_t dump_filter_nlmsg, void *cookie)
{
struct {
struct nlmsghdr nlh;
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 6/6] libbpf: Use __u32 instead of u32 in bpf_program__load
From: Andrey Ignatov @ 2018-10-03 22:26 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, kernel-team
In-Reply-To: <cover.1538605140.git.rdna@fb.com>
Make bpf_program__load consistent with other interfaces: use __u32
instead of u32. That in turn fixes build of samples:
In file included from ./samples/bpf/trace_output_user.c:21:0:
./tools/lib/bpf/libbpf.h:132:9: error: unknown type name ‘u32’
u32 kern_version);
^
Fixes: commit 29cd77f41620d ("libbpf: Support loading individual progs")
Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
tools/lib/bpf/libbpf.c | 2 +-
tools/lib/bpf/libbpf.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 02888d36b805..85de1ebd4cb0 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1379,7 +1379,7 @@ load_program(enum bpf_prog_type type, enum bpf_attach_type expected_attach_type,
int
bpf_program__load(struct bpf_program *prog,
- char *license, u32 kern_version)
+ char *license, __u32 kern_version)
{
int err = 0, fd, i;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 28f83dd6022b..fbfc2aec0f0d 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -129,7 +129,7 @@ void bpf_program__set_ifindex(struct bpf_program *prog, __u32 ifindex);
const char *bpf_program__title(struct bpf_program *prog, bool needs_copy);
int bpf_program__load(struct bpf_program *prog, char *license,
- u32 kern_version);
+ __u32 kern_version);
int bpf_program__fd(struct bpf_program *prog);
int bpf_program__pin_instance(struct bpf_program *prog, const char *path,
int instance);
--
2.17.1
^ 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