* [RFC PATCH net-next 5/5] virtio-net: support XDP rx handler
From: Jason Wang @ 2018-08-13 3:05 UTC (permalink / raw)
To: netdev, linux-kernel; +Cc: ast, daniel, jbrouer, mst, Jason Wang
In-Reply-To: <1534129513-4845-1-git-send-email-jasowang@redhat.com>
This patch tries to add the support of XDP rx handler to
virtio-net. This is straight-forward, just call xdp_do_pass() and
behave depends on its return value.
Test was done by using XDP_DROP (xdp1) for macvlan on top of
virtio-net. PPS of SKB mode was ~1.2Mpps while PPS of native XDP mode
was ~2.2Mpps. About 83% improvement was measured.
Notes: for RFC, only mergeable buffer case was implemented.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/virtio_net.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 62311dd..1e22ad9 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -777,6 +777,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
rcu_read_lock();
xdp_prog = rcu_dereference(rq->xdp_prog);
if (xdp_prog) {
+ rx_xdp_handler_result_t ret;
struct xdp_frame *xdpf;
struct page *xdp_page;
struct xdp_buff xdp;
@@ -825,6 +826,15 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
switch (act) {
case XDP_PASS:
+ ret = xdp_do_pass(&xdp);
+ if (ret == RX_XDP_HANDLER_DROP)
+ goto drop;
+ if (ret != RX_XDP_HANDLER_FALLBACK) {
+ if (unlikely(xdp_page != page))
+ put_page(page);
+ rcu_read_unlock();
+ goto xdp_xmit;
+ }
/* recalculate offset to account for any header
* adjustments. Note other cases do not build an
* skb and avoid using offset
@@ -881,6 +891,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
case XDP_ABORTED:
trace_xdp_exception(vi->dev, xdp_prog, act);
/* fall through */
+drop:
case XDP_DROP:
if (unlikely(xdp_page != page))
__free_pages(xdp_page, 0);
--
2.7.4
^ permalink raw reply related
* [RFC PATCH net-next 3/5] macvlan: count the number of vlan in source mode
From: Jason Wang @ 2018-08-13 3:05 UTC (permalink / raw)
To: netdev, linux-kernel; +Cc: ast, daniel, jbrouer, mst, Jason Wang
In-Reply-To: <1534129513-4845-1-git-send-email-jasowang@redhat.com>
This patch tries to count the number of vlans in source mode. This
will be used for implementing XDP rx handler for macvlan.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/macvlan.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index cfda146..b7c814d 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -53,6 +53,7 @@ struct macvlan_port {
struct hlist_head vlan_source_hash[MACVLAN_HASH_SIZE];
DECLARE_BITMAP(mc_filter, MACVLAN_MC_FILTER_SZ);
unsigned char perm_addr[ETH_ALEN];
+ unsigned long source_count;
};
struct macvlan_source_entry {
@@ -1433,6 +1434,9 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
if (err)
goto unregister_netdev;
+ if (vlan->mode == MACVLAN_MODE_SOURCE)
+ port->source_count++;
+
list_add_tail_rcu(&vlan->list, &port->vlans);
netif_stacked_transfer_operstate(lowerdev, dev);
linkwatch_fire_event(dev);
@@ -1477,6 +1481,7 @@ static int macvlan_changelink(struct net_device *dev,
struct netlink_ext_ack *extack)
{
struct macvlan_dev *vlan = netdev_priv(dev);
+ struct macvlan_port *port = vlan->port;
enum macvlan_mode mode;
bool set_mode = false;
enum macvlan_macaddr_mode macmode;
@@ -1491,8 +1496,10 @@ static int macvlan_changelink(struct net_device *dev,
(vlan->mode == MACVLAN_MODE_PASSTHRU))
return -EINVAL;
if (vlan->mode == MACVLAN_MODE_SOURCE &&
- vlan->mode != mode)
+ vlan->mode != mode) {
macvlan_flush_sources(vlan->port, vlan);
+ port->source_count--;
+ }
}
if (data && data[IFLA_MACVLAN_FLAGS]) {
@@ -1510,8 +1517,13 @@ static int macvlan_changelink(struct net_device *dev,
}
vlan->flags = flags;
}
- if (set_mode)
+ if (set_mode) {
vlan->mode = mode;
+ if (mode == MACVLAN_MODE_SOURCE &&
+ vlan->mode != mode) {
+ port->source_count++;
+ }
+ }
if (data && data[IFLA_MACVLAN_MACADDR_MODE]) {
if (vlan->mode != MACVLAN_MODE_SOURCE)
return -EINVAL;
--
2.7.4
^ permalink raw reply related
* [RFC PATCH net-next 2/5] net: core: introduce XDP rx handler
From: Jason Wang @ 2018-08-13 3:05 UTC (permalink / raw)
To: netdev, linux-kernel; +Cc: ast, daniel, jbrouer, mst, Jason Wang
In-Reply-To: <1534129513-4845-1-git-send-email-jasowang@redhat.com>
This patch tries to introduce XDP rx handler. This will be used by
stacked device that depends on rx handler for having a fast packet
processing path based on XDP.
This idea is simple, when XDP program returns XDP_PASS, instead of
building skb immediately, driver will call xdp_do_pass() to check
whether or not there's a XDP rx handler, if yes, it will pass XDP
buffer to XDP rx handler first.
There are two main tasks for XDP rx handler, the first is check
whether or not the setup or packet could be processed through XDP buff
directly. The second task is to run XDP program. An XDP rx handler can
return several different results which was defined by enum
rx_xdp_handler_result_t:
RX_XDP_HANDLER_CONSUMED: This means the XDP buff were consumed.
RX_XDP_HANDLER_DROP: This means XDP rx handler ask to drop the packet.
RX_XDP_HANDLER_PASS_FALLBACK: This means XDP rx handler can not
process the packet (e.g cloning), and we need to fall back to normal
skb path to deal with the packet.
Consider we have the following configuration, Level 0 device which has
a rx handler for Level 1 device which has a rx handler for L2 device.
L2 device
|
L1 device
|
L0 device
With the help of XDP rx handler, we can attach XDP program on each of
the layer or even run native XDP handler for L2 without XDP prog
attached to L1 device:
(XDP prog for L2 device)
|
L2 XDP rx handler for L1
|
(XDP prog for L1 device)
|
L1 XDP rx hanlder for L0
|
XDP prog for L0 device
It works like: When the XDP program for L0 device returns XDP_PASS, we
will first try to check and pass XDP buff to its XDP rx handler if
there's one. Then the L1 XDP rx handler will be called and to run XDP
program for L1. When L1 XDP program returns XDP_PASS or there's no XDP
program attached to L1, we will try to call xdp_do_pass() to pass it
to XDP rx hanlder for L1. Then XDP buff will be passed to L2 XDP rx
handler etc. And it will try to run L2 XDP program if any. And if
there's no L2 XDP program or XDP program returns XDP_PASS. The handler
usually will build skb and call netif_rx() for a local receive. If any
of the XDP rx handlers returns XDP_RX_HANDLER_FALLBACK, the code will
return to L0 device and L0 device will try to build skb and go through
normal rx handler path for skb.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
include/linux/filter.h | 1 +
include/linux/netdevice.h | 12 ++++++++++++
net/core/dev.c | 29 +++++++++++++++++++++++++++++
net/core/filter.c | 28 ++++++++++++++++++++++++++++
4 files changed, 70 insertions(+)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index c73dd73..7cc8e69 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -791,6 +791,7 @@ int xdp_do_generic_redirect(struct net_device *dev, struct sk_buff *skb,
int xdp_do_redirect(struct net_device *dev,
struct xdp_buff *xdp,
struct bpf_prog *prog);
+rx_handler_result_t xdp_do_pass(struct xdp_buff *xdp);
void xdp_do_flush_map(void);
void bpf_warn_invalid_xdp_action(u32 act);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 282e2e9..21f0a9e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -421,6 +421,14 @@ enum rx_handler_result {
typedef enum rx_handler_result rx_handler_result_t;
typedef rx_handler_result_t rx_handler_func_t(struct sk_buff **pskb);
+enum rx_xdp_handler_result {
+ RX_XDP_HANDLER_CONSUMED,
+ RX_XDP_HANDLER_DROP,
+ RX_XDP_HANDLER_FALLBACK,
+};
+typedef enum rx_xdp_handler_result rx_xdp_handler_result_t;
+typedef rx_xdp_handler_result_t rx_xdp_handler_func_t(struct net_device *dev,
+ struct xdp_buff *xdp);
void __napi_schedule(struct napi_struct *n);
void __napi_schedule_irqoff(struct napi_struct *n);
@@ -1898,6 +1906,7 @@ struct net_device {
struct bpf_prog __rcu *xdp_prog;
unsigned long gro_flush_timeout;
rx_handler_func_t __rcu *rx_handler;
+ rx_xdp_handler_func_t __rcu *rx_xdp_handler;
void __rcu *rx_handler_data;
#ifdef CONFIG_NET_CLS_ACT
@@ -3530,7 +3539,10 @@ bool netdev_is_rx_handler_busy(struct net_device *dev);
int netdev_rx_handler_register(struct net_device *dev,
rx_handler_func_t *rx_handler,
void *rx_handler_data);
+int netdev_rx_xdp_handler_register(struct net_device *dev,
+ rx_xdp_handler_func_t *rx_xdp_handler);
void netdev_rx_handler_unregister(struct net_device *dev);
+void netdev_rx_xdp_handler_unregister(struct net_device *dev);
bool dev_valid_name(const char *name);
int dev_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr,
diff --git a/net/core/dev.c b/net/core/dev.c
index a77ce08..b4e8949 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4638,6 +4638,12 @@ bool netdev_is_rx_handler_busy(struct net_device *dev)
}
EXPORT_SYMBOL_GPL(netdev_is_rx_handler_busy);
+static bool netdev_is_rx_xdp_handler_busy(struct net_device *dev)
+{
+ ASSERT_RTNL();
+ return dev && rtnl_dereference(dev->rx_xdp_handler);
+}
+
/**
* netdev_rx_handler_register - register receive handler
* @dev: device to register a handler for
@@ -4670,6 +4676,22 @@ int netdev_rx_handler_register(struct net_device *dev,
}
EXPORT_SYMBOL_GPL(netdev_rx_handler_register);
+int netdev_rx_xdp_handler_register(struct net_device *dev,
+ rx_xdp_handler_func_t *rx_xdp_handler)
+{
+ if (netdev_is_rx_xdp_handler_busy(dev))
+ return -EBUSY;
+
+ if (dev->priv_flags & IFF_NO_RX_HANDLER)
+ return -EINVAL;
+
+ rcu_assign_pointer(dev->rx_xdp_handler, rx_xdp_handler);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(netdev_rx_xdp_handler_register);
+
+
/**
* netdev_rx_handler_unregister - unregister receive handler
* @dev: device to unregister a handler from
@@ -4692,6 +4714,13 @@ void netdev_rx_handler_unregister(struct net_device *dev)
}
EXPORT_SYMBOL_GPL(netdev_rx_handler_unregister);
+void netdev_rx_xdp_handler_unregister(struct net_device *dev)
+{
+ ASSERT_RTNL();
+ RCU_INIT_POINTER(dev->rx_xdp_handler, NULL);
+}
+EXPORT_SYMBOL_GPL(netdev_rx_xdp_handler_unregister);
+
/*
* Limit the use of PFMEMALLOC reserves to those protocols that implement
* the special handling of PFMEMALLOC skbs.
diff --git a/net/core/filter.c b/net/core/filter.c
index 587bbfb..9ea3797 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3312,6 +3312,34 @@ int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp,
}
EXPORT_SYMBOL_GPL(xdp_do_redirect);
+rx_handler_result_t xdp_do_pass(struct xdp_buff *xdp)
+{
+ rx_xdp_handler_result_t ret;
+ rx_xdp_handler_func_t *rx_xdp_handler;
+ struct net_device *dev = xdp->rxq->dev;
+
+ ret = RX_XDP_HANDLER_FALLBACK;
+ rx_xdp_handler = rcu_dereference(dev->rx_xdp_handler);
+
+ if (rx_xdp_handler) {
+ ret = rx_xdp_handler(dev, xdp);
+ switch (ret) {
+ case RX_XDP_HANDLER_CONSUMED:
+ /* Fall through */
+ case RX_XDP_HANDLER_DROP:
+ /* Fall through */
+ case RX_XDP_HANDLER_FALLBACK:
+ break;
+ default:
+ BUG();
+ break;
+ }
+ }
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(xdp_do_pass);
+
static int xdp_do_generic_redirect_map(struct net_device *dev,
struct sk_buff *skb,
struct xdp_buff *xdp,
--
2.7.4
^ permalink raw reply related
* [RFC PATCH net-next 1/5] net: core: generic XDP support for stacked device
From: Jason Wang @ 2018-08-13 3:05 UTC (permalink / raw)
To: netdev, linux-kernel; +Cc: ast, daniel, jbrouer, mst, Jason Wang
In-Reply-To: <1534129513-4845-1-git-send-email-jasowang@redhat.com>
Stacked device usually change skb->dev to its own and return
RX_HANDLER_ANOTHER during rx handler processing. But we don't call
generic XDP routine at that time, this means it can't work for stacked
device.
Fixing this by calling netif_do_generic_xdp() if rx handler returns
RX_HANDLER_ANOTHER. This allows us to do generic XDP on stacked device
e.g macvlan.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
net/core/dev.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index 605c66e..a77ce08 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4822,6 +4822,11 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc,
ret = NET_RX_SUCCESS;
goto out;
case RX_HANDLER_ANOTHER:
+ ret = netif_do_generic_xdp(skb);
+ if (ret != XDP_PASS) {
+ ret = NET_RX_SUCCESS;
+ goto out;
+ }
goto another_round;
case RX_HANDLER_EXACT:
deliver_exact = true;
--
2.7.4
^ permalink raw reply related
* Re: [BUG] bpf: syscall: a possible sleep-in-atomic-context bug in map_update_elem()
From: Jia-Ju Bai @ 2018-08-13 2:48 UTC (permalink / raw)
To: Y Song
Cc: Daniel Borkmann, Alexei Starovoitov, netdev,
Linux Kernel Mailing List, Jesper Dangaard Brouer
In-Reply-To: <CAH3MdRWBrEfapBMkK9sOa-e_bBZzdkcxhioEHJoZ31hF6XXuxQ@mail.gmail.com>
On 2018/8/11 13:01, Y Song wrote:
> On Fri, Aug 10, 2018 at 6:57 PM, Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
>>
>> On 2018/8/10 22:22, Daniel Borkmann wrote:
>>> On 08/10/2018 04:07 PM, Jia-Ju Bai wrote:
>>>> The kernel may sleep with holding a rcu read lock.
>>>>
>>>> The function call paths (from bottom to top) in Linux-4.16 are:
>>>>
>>>> [FUNC] kmalloc(GFP_KERNEL)
>>>> kernel/kthread.c, 283: kmalloc in __kthread_create_on_node
>>>> kernel/kthread.c, 365: __kthread_create_on_node in kthread_create_on_node
>>>> kernel/bpf/cpumap.c, 368: kthread_create_on_node in __cpu_map_entry_alloc
>>>> kernel/bpf/cpumap.c, 490: __cpu_map_entry_alloc in cpu_map_update_elem
>>>> kernel/bpf/syscall.c, 724: [FUNC_PTR]cpu_map_update_elem in
>>>> map_update_elem
>>>> kernel/bpf/syscall.c, 723: rcu_read_lock in map_update_elem
>>>>
>>>> Note that [FUNC_PTR] means a function pointer call is used.
>>>>
>>>> I do not find a good way to fix it, so I only report.
>>>> This is found by my static analysis tool (DSAC).
> Maybe your static analysis tool have false positives in this case?
>
>>> Thanks for the report Jia-Ju! In the map_update_elem() from syscall
>>> path there's a check map->map_type == BPF_MAP_TYPE_CPUMAP, where we
>>> call the cpumap's map->ops->map_update_elem() while /not/ being under
>>> rcu_read_lock() as in other cases, so looks okay to me. Could you point
>>> out the case for being under rcu_read_lock() more specifically which
>>> the tool found?
>>
>> Thanks for your reply :)
>> My tool cannot accurately track the case of map->map_type at present...
>>
>> According to my code review, there is a indeed check on line 697 in
>> Linux-4.16:
>> else if (map->map_type == BPF_MAP_TYPE_CPUMAP) {
>> err = map->ops->map_update_elem(map, key, value, attr->flags);
>> goto out;
>> }
>> But there is a call to map->ops->map_update_elem() that is under
>> rcu_read_lock on line 724:
>> rcu_read_lock();
>> err = map->ops->map_update_elem(map, key, value, attr->flags);
>> rcu_read_unlock();
>>
>> So I think if map->map_type is not equal to BPF_MAP_TYPE_CPUMAP,
>> map->ops->map_update_elem() can still be called under rcu_read_lock, is it
>> right?
> map->ops->map_update_elem() can be called under rcu_read_lock(), but
> since it is not type cpumap, the function should not be cpu_map_update_elem().
> Could you double check your static analysis tool?
Thanks for your reply :)
I have checked the code again, and find that my report is not correct here.
It is because that my tool does not handle the context of function
pointer assignment.
Best wishes,
Jia-Ju Bai
^ permalink raw reply
* bpf-next is CLOSED
From: Daniel Borkmann @ 2018-08-12 23:50 UTC (permalink / raw)
To: netdev; +Cc: ast
Given the merge window is open right now, please only submit bug
fixes to bpf tree at this time, thank you!
^ permalink raw reply
* pull-request: bpf-next 2018-08-13
From: Daniel Borkmann @ 2018-08-12 23:49 UTC (permalink / raw)
To: davem; +Cc: daniel, ast, netdev
Hi David,
The following pull-request contains BPF updates for your *net-next* tree.
The main changes are:
1) Add driver XDP support for veth. This can be used in conjunction with
redirect of another XDP program e.g. sitting on NIC so the xdp_frame
can be forwarded to the peer veth directly without modification,
from Toshiaki.
2) Add a new BPF map type REUSEPORT_SOCKARRAY and prog type SK_REUSEPORT
in order to provide more control and visibility on where a SO_REUSEPORT
sk should be located, and the latter enables to directly select a sk
from the bpf map. This also enables map-in-map for application migration
use cases, from Martin.
3) Add a new BPF helper bpf_skb_ancestor_cgroup_id() that returns the id
of cgroup v2 that is the ancestor of the cgroup associated with the
skb at the ancestor_level, from Andrey.
4) Implement BPF fs map pretty-print support based on BTF data for regular
hash table and LRU map, from Yonghong.
5) Decouple the ability to attach BTF for a map from the key and value
pretty-printer in BPF fs, and enable further support of BTF for maps for
percpu and LPM trie, from Daniel.
6) Implement a better BPF sample of using XDP's CPU redirect feature for
load balancing SKB processing to remote CPU. The sample implements the
same XDP load balancing as Suricata does which is symmetric hash based
on IP and L4 protocol, from Jesper.
7) Revert adding NULL pointer check with WARN_ON_ONCE() in __xdp_return()'s
critical path as it is ensured that the allocator is present, from Björn.
Please consider pulling these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git
Thanks a lot!
----------------------------------------------------------------
The following changes since commit a736e074680745faa5dc6be8dd3c58ad4850aab9:
Merge ra.kernel.org:/pub/scm/linux/kernel/git/davem/net (2018-08-09 11:52:36 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git
for you to fetch changes up to 2ce3206b9eb3943de09f3bf4ec9134568420d8b9:
Merge branch 'bpf-ancestor-cgroup-id' (2018-08-13 01:02:41 +0200)
----------------------------------------------------------------
Andrey Ignatov (4):
bpf: Introduce bpf_skb_ancestor_cgroup_id helper
bpf: Sync bpf.h to tools/
selftests/bpf: Add cgroup id helpers to bpf_helpers.h
selftests/bpf: Selftest for bpf_skb_ancestor_cgroup_id
Björn Töpel (1):
Revert "xdp: add NULL pointer check in __xdp_return()"
Daniel Borkmann (6):
Merge branch 'bpf-sample-cpumap-lb'
Merge branch 'bpf-veth-xdp-support'
Merge branch 'bpf-btf-for-htab-lru'
Merge branch 'bpf-reuseport-map'
bpf: decouple btf from seq bpf fs dump and enable more maps
Merge branch 'bpf-ancestor-cgroup-id'
Jesper Dangaard Brouer (2):
samples/bpf: add Paul Hsieh's (LGPL 2.1) hash function SuperFastHash
samples/bpf: xdp_redirect_cpu load balance like Suricata
Martin KaFai Lau (9):
tcp: Avoid TCP syncookie rejected by SO_REUSEPORT socket
net: Add ID (if needed) to sock_reuseport and expose reuseport_lock
bpf: Introduce BPF_MAP_TYPE_REUSEPORT_SOCKARRAY
bpf: Introduce BPF_PROG_TYPE_SK_REUSEPORT
bpf: Enable BPF_PROG_TYPE_SK_REUSEPORT bpf prog in reuseport selection
bpf: Refactor ARRAY_SIZE macro to bpf_util.h
bpf: Sync bpf.h uapi to tools/
bpf: test BPF_MAP_TYPE_REUSEPORT_SOCKARRAY
bpf: Test BPF_PROG_TYPE_SK_REUSEPORT
Toshiaki Makita (10):
net: Export skb_headers_offset_update
veth: Add driver XDP
veth: Avoid drops by oversized packets when XDP is enabled
xdp: Helper function to clear kernel pointers in xdp_frame
veth: Handle xdp_frames in xdp napi ring
veth: Add ndo_xdp_xmit
bpf: Make redirect_info accessible from modules
xdp: Helpers for disabling napi_direct of xdp_return_frame
veth: Add XDP TX and REDIRECT
veth: Support per queue XDP ring
Yonghong Song (3):
bpf: fix bpffs non-array map seq_show issue
bpf: btf: add pretty print for hash/lru_hash maps
tools/bpf: add bpffs pretty print btf test for hash/lru_hash maps
drivers/net/veth.c | 750 ++++++++++++++++++++-
include/linux/bpf.h | 41 +-
include/linux/bpf_types.h | 6 +
include/linux/cgroup.h | 30 +
include/linux/filter.h | 51 ++
include/linux/skbuff.h | 1 +
include/net/addrconf.h | 1 +
include/net/sock_reuseport.h | 19 +-
include/net/tcp.h | 30 +-
include/net/xdp.h | 7 +
include/uapi/linux/bpf.h | 56 +-
kernel/bpf/Makefile | 3 +
kernel/bpf/arraymap.c | 28 +-
kernel/bpf/cpumap.c | 1 +
kernel/bpf/devmap.c | 1 +
kernel/bpf/hashtab.c | 26 +
kernel/bpf/inode.c | 11 +-
kernel/bpf/local_storage.c | 1 +
kernel/bpf/lpm_trie.c | 12 +
kernel/bpf/reuseport_array.c | 363 ++++++++++
kernel/bpf/sockmap.c | 2 +
kernel/bpf/stackmap.c | 1 +
kernel/bpf/syscall.c | 42 +-
kernel/bpf/verifier.c | 9 +
kernel/bpf/xskmap.c | 3 +-
net/core/filter.c | 411 +++++++++--
net/core/skbuff.c | 3 +-
net/core/sock_reuseport.c | 92 ++-
net/core/xdp.c | 9 +-
net/ipv4/inet_connection_sock.c | 9 +
net/ipv4/inet_hashtables.c | 19 +-
net/ipv4/udp.c | 9 +-
net/ipv6/inet6_hashtables.c | 14 +-
net/ipv6/udp.c | 4 +
samples/bpf/hash_func01.h | 55 ++
samples/bpf/xdp_redirect_cpu_kern.c | 103 +++
samples/bpf/xdp_redirect_cpu_user.c | 4 +-
tools/include/uapi/linux/bpf.h | 56 +-
tools/lib/bpf/bpf.c | 1 +
tools/lib/bpf/bpf.h | 1 +
tools/lib/bpf/libbpf.c | 1 +
tools/testing/selftests/bpf/Makefile | 11 +-
tools/testing/selftests/bpf/bpf_helpers.h | 8 +
tools/testing/selftests/bpf/bpf_util.h | 4 +
tools/testing/selftests/bpf/test_align.c | 5 +-
tools/testing/selftests/bpf/test_btf.c | 92 ++-
tools/testing/selftests/bpf/test_maps.c | 262 ++++++-
.../testing/selftests/bpf/test_select_reuseport.c | 688 +++++++++++++++++++
.../selftests/bpf/test_select_reuseport_common.h | 36 +
.../selftests/bpf/test_select_reuseport_kern.c | 180 +++++
tools/testing/selftests/bpf/test_skb_cgroup_id.sh | 62 ++
.../selftests/bpf/test_skb_cgroup_id_kern.c | 47 ++
.../selftests/bpf/test_skb_cgroup_id_user.c | 187 +++++
tools/testing/selftests/bpf/test_sock.c | 5 +-
tools/testing/selftests/bpf/test_sock_addr.c | 5 +-
tools/testing/selftests/bpf/test_verifier.c | 5 +-
56 files changed, 3707 insertions(+), 176 deletions(-)
create mode 100644 kernel/bpf/reuseport_array.c
create mode 100644 samples/bpf/hash_func01.h
create mode 100644 tools/testing/selftests/bpf/test_select_reuseport.c
create mode 100644 tools/testing/selftests/bpf/test_select_reuseport_common.h
create mode 100644 tools/testing/selftests/bpf/test_select_reuseport_kern.c
create mode 100755 tools/testing/selftests/bpf/test_skb_cgroup_id.sh
create mode 100644 tools/testing/selftests/bpf/test_skb_cgroup_id_kern.c
create mode 100644 tools/testing/selftests/bpf/test_skb_cgroup_id_user.c
^ permalink raw reply
* [PATCH] net: Change the layout of structure trace_event_raw_fib_table_lookup
From: Zong Li @ 2018-08-13 2:26 UTC (permalink / raw)
To: rostedt, mingo, dsahern, netdev, linux-kernel; +Cc: zongbox, greentime, Zong Li
There is an unalignment access about the structure
'trace_event_raw_fib_table_lookup'.
In include/trace/events/fib.h, there is a memory operation which casting
the 'src' data member to a pointer, and then store a value to this
pointer point to.
p32 = (__be32 *) __entry->src;
*p32 = flp->saddr;
The offset of 'src' in structure trace_event_raw_fib_table_lookup is not
four bytes alignment. On some architectures, they don't permit the
unalignment access, it need to pay the price to handle this situation in
exception handler.
Adjust the layout of structure to avoid this case.
Signed-off-by: Zong Li <zong@andestech.com>
---
include/trace/events/fib.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/trace/events/fib.h b/include/trace/events/fib.h
index 9763cddd0594..6271bab63bfb 100644
--- a/include/trace/events/fib.h
+++ b/include/trace/events/fib.h
@@ -22,6 +22,7 @@ TRACE_EVENT(fib_table_lookup,
__field( int, err )
__field( int, oif )
__field( int, iif )
+ __field( u8, proto )
__field( __u8, tos )
__field( __u8, scope )
__field( __u8, flags )
@@ -31,7 +32,6 @@ TRACE_EVENT(fib_table_lookup,
__array( __u8, saddr, 4 )
__field( u16, sport )
__field( u16, dport )
- __field( u8, proto )
__dynamic_array(char, name, IFNAMSIZ )
),
--
2.16.1
^ permalink raw reply related
* Re: [PATCH v2 bpf-next 0/4] bpf_skb_ancestor_cgroup_id helper
From: Daniel Borkmann @ 2018-08-12 23:23 UTC (permalink / raw)
To: Andrey Ignatov, netdev; +Cc: ast, tj, guro, yhs, kernel-team
In-Reply-To: <cover.1534095926.git.rdna@fb.com>
On 08/12/2018 07:49 PM, Andrey Ignatov wrote:
> v1->v2:
> - more reliable check for testing IPv6 to become ready in selftest.
>
> This patch set adds new BPF helper bpf_skb_ancestor_cgroup_id that returns
> id of cgroup v2 that is ancestor of cgroup associated with the skb at the
> ancestor_level.
>
> The helper is useful to implement policies in TC based on cgroups that are
> upper in hierarchy than immediate cgroup associated with skb.
>
> Patch 0001 provides more details and describes use-cases.
> Patch 0002 syncs UAPI changes to tools/.
> Patch 0003 adds skb*cgroup_id helpers to bpf_helper.h header.
> Patch 0004 adds selftest for the new helper and is an example of usage.
>
>
> Andrey Ignatov (4):
> bpf: Introduce bpf_skb_ancestor_cgroup_id helper
> bpf: Sync bpf.h to tools/
> selftests/bpf: Add cgroup id helpers to bpf_helpers.h
> selftests/bpf: Selftest for bpf_skb_ancestor_cgroup_id
Applied to bpf-next, thanks Andrey!
^ permalink raw reply
* Re: [Patch net] ipv6: fix double refcount of fib6_metrics
From: David Ahern @ 2018-08-12 23:08 UTC (permalink / raw)
To: Cong Wang, netdev; +Cc: Sabrina Dubroca
In-Reply-To: <20180803062038.13272-1-xiyou.wangcong@gmail.com>
On 8/3/18 12:20 AM, Cong Wang wrote:
> All the callers of ip6_rt_copy_init()/rt6_set_from() hold refcnt
> of the "from" fib6_info, so there is no need to hold fib6_metrics
> refcnt again, because fib6_metrics refcnt is only released when
> fib6_info is gone, that is, they have the same life time, so the
> whole fib6_metrics refcnt can be removed actually.
>
> This fixes a kmemleak warning reported by Sabrina.
>
> Fixes: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")
> Reported-by: Sabrina Dubroca <sd@queasysnail.net>
> Cc: Sabrina Dubroca <sd@queasysnail.net>
> Cc: David Ahern <dsahern@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
> net/ipv6/route.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index ec18b3ce8b6d..7208c16302f6 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -978,10 +978,6 @@ static void rt6_set_from(struct rt6_info *rt, struct fib6_info *from)
> rt->rt6i_flags &= ~RTF_EXPIRES;
> rcu_assign_pointer(rt->from, from);
> dst_init_metrics(&rt->dst, from->fib6_metrics->metrics, true);
> - if (from->fib6_metrics != &dst_default_metrics) {
> - rt->dst._metrics |= DST_METRICS_REFCOUNTED;
> - refcount_inc(&from->fib6_metrics->refcnt);
> - }
> }
>
> /* Caller must already hold reference to @ort */
>
This seems like a reasonable fix. Thanks for the patch.
^ permalink raw reply
* Re: [V9fs-developer] [PATCH 2/2] 9p: Add refcount to p9_req_t
From: Dominique Martinet @ 2018-08-13 1:48 UTC (permalink / raw)
To: piaojun
Cc: Tomas Bortoli, ericvh, rminnich, lucho, Dominique Martinet,
netdev, linux-kernel, syzkaller, v9fs-developer, davem
In-Reply-To: <5B70E0D1.2080300@huawei.com>
piaojun wrote on Mon, Aug 13, 2018:
> Could you help paste the reason of the crash bug to help others
> understand more clearly? And I have another question below.
The problem for tcp (but other transports have a similar problem) is
that with a malicious server like syzkaller they can try to submit
replies before the request came in.
This leads in the writer thread trying to write a buffer that has
already been freed, and if memory has been reused could potentially leak
some information.
Now, with the previous patches this is based on this would be a slab and
the likeliness of it being sensitive information is rather low (it would
likely be some other packet being sent twice, or a mix and match of two
packets that would have been sent anyway), but it would nevertheless be
a use after free.
There is a second advantage to this reference counting, that is now we
have this system we will be able to implement flush asynchronously.
This will remove the need for the 'goto again' in p9_client_rpc which
was making 9p threads unkillable in practice if the server would not
reply to the flush requests.
Even if the server replies I've always found myself needing to hit ^C
multiple times to exit a process doing I/Os and I think fixing that
behaviour will make 9p more comfortable to use.
> > diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
> > index 20f46f13fe83..686e24e355d0 100644
> > --- a/net/9p/trans_fd.c
> > +++ b/net/9p/trans_fd.c
> > @@ -132,6 +132,7 @@ struct p9_conn {
> > struct list_head req_list;
> > struct list_head unsent_req_list;
> > struct p9_req_t *req;
> > + struct p9_req_t *wreq;
>
> Why adding a wreq for write work? And I wonder we should rename req to
> rreq?
We need to store a pointer to the request for the write thread because
we need to put the reference to it when we're done writing its content.
Previously, the worker would only store the write buffer there but
that's not enough to figure what request to dereference.
I personally don't think renaming req to rreq would bring much but it
could be done in another patch if you think that'd be helpful; I think
it shouldn't be done here at least to make the patch more readable.
--
Dominique
^ permalink raw reply
* Re: [PATCH net] net/xdp: Fix suspicious RCU usage warning
From: Daniel Borkmann @ 2018-08-12 23:04 UTC (permalink / raw)
To: Tariq Toukan, Alexei Starovoitov
Cc: David S. Miller, netdev, Eran Ben Elisha, Jesper Dangaard Brouer
In-Reply-To: <44cb66c6-2542-3f6a-c426-5b1d1665ea61@mellanox.com>
On 08/12/2018 10:45 AM, Tariq Toukan wrote:
>
>
> On 20/07/2018 12:36 AM, Alexei Starovoitov wrote:
>> On Wed, Jul 18, 2018 at 05:13:54PM +0300, Tariq Toukan wrote:
>>>
>>>
>>> On 17/07/2018 10:27 PM, Daniel Borkmann wrote:
>>>> On 07/17/2018 06:47 PM, Alexei Starovoitov wrote:
>>>>> On Tue, Jul 17, 2018 at 06:10:38PM +0300, Tariq Toukan wrote:
>>>>>> Fix the warning below by calling rhashtable_lookup under
>>>>>> RCU read lock.
>>>>>>
>>>
>>> ...
>>>
>>>>>> mutex_lock(&mem_id_lock);
>>>>>> + rcu_read_lock();
>>>>>> xa = rhashtable_lookup(mem_id_ht, &id, mem_id_rht_params);
>>>>>> + rcu_read_unlock();
>>>>>> if (!xa) {
>>>>>
>>>>> if it's an actual bug rcu_read_unlock seems to be misplaced.
>>>>> It silences the warn, but rcu section looks wrong.
>>>>
>>>> I think that whole piece in __xdp_rxq_info_unreg_mem_model() should be:
>>>>
>>>> mutex_lock(&mem_id_lock);
>>>> xa = rhashtable_lookup_fast(mem_id_ht, &id, mem_id_rht_params);
>>>> if (xa && rhashtable_remove_fast(mem_id_ht, &xa->node, mem_id_rht_params) == 0)
>>>> call_rcu(&xa->rcu, __xdp_mem_allocator_rcu_free);
>>>> mutex_unlock(&mem_id_lock);
>>>>
>>>> Technically the RCU read side plus rhashtable_lookup() is the same, but lets
>>>> use proper api. From the doc (https://lwn.net/Articles/751374/) object removal
>>>> is wrapped around the RCU read side additionally, but in our case we're behind
>>>> mem_id_lock for insertion/removal serialization.
>>>>
>>>> Cheers,
>>>> Daniel
>>>>
>>>
>>> Just as Daniel stated, I think there's no actual bug here, but we still want
>>> to silence the RCU warning.
>>>
>>> Alexei, did you mean getting the if statement into the RCU lock critical
>>> section?
>>
>> If what Daniel proposes silences the warn, I'd rather do that.
>> Pattern like:
>> rcu_lock;
>> val = lookup();
>> rcu_unlock;
>> if (val)
>> will cause people to question the quality of the code and whether
>> authors of the code understand rcu.
>> There should be a way to silence the warn without adding
>> "wrong on the first glance" code.
>
> I'm re-spinning this.
> Can it still go to net, or better send it to bpf-next ?
Please rebase against bpf-next and we route it to stable, thanks!
^ permalink raw reply
* Re: [V9fs-developer] [PATCH 2/2] 9p: Add refcount to p9_req_t
From: piaojun @ 2018-08-13 1:37 UTC (permalink / raw)
To: Tomas Bortoli, asmadeus, ericvh, rminnich, lucho
Cc: Dominique Martinet, netdev, linux-kernel, syzkaller,
v9fs-developer, davem
In-Reply-To: <20180811144254.23665-2-tomasbortoli@gmail.com>
Hi Tomas & Dominique,
Could you help paste the reason of the crash bug to help others understand
more clearly? And I have another question below.
On 2018/8/11 22:42, Tomas Bortoli wrote:
> To avoid use-after-free(s), use a refcount to keep track of the
> usable references to any instantiated struct p9_req_t.
>
> This commit adds p9_req_put(), p9_req_get() and p9_req_try_get() as
> wrappers to kref_put(), kref_get() and kref_get_unless_zero().
> These are used by the client and the transports to keep track of
> valid requests' references.
>
> p9_free_req() is added back and used as callback by kref_put().
>
> Add SLAB_TYPESAFE_BY_RCU as it ensures that the memory freed by
> kmem_cache_free() will not be reused for another type until the rcu
> synchronisation period is over, so an address gotten under rcu read
> lock is safe to inc_ref() without corrupting random memory while
> the lock is held.
>
> Co-developed-by: Dominique Martinet <dominique.martinet@cea.fr>
> Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
> Reported-by: syzbot+467050c1ce275af2a5b8@syzkaller.appspotmail.com
> Signed-off-by: Dominique Martinet <dominique.martinet@cea.fr>
> ---
> include/net/9p/client.h | 14 +++++++++++++
> net/9p/client.c | 54 +++++++++++++++++++++++++++++++++++++++++++------
> net/9p/trans_fd.c | 11 +++++++++-
> net/9p/trans_rdma.c | 1 +
> 4 files changed, 73 insertions(+), 7 deletions(-)
>
> diff --git a/include/net/9p/client.h b/include/net/9p/client.h
> index 735f3979d559..947a570307a6 100644
> --- a/include/net/9p/client.h
> +++ b/include/net/9p/client.h
> @@ -94,6 +94,7 @@ enum p9_req_status_t {
> struct p9_req_t {
> int status;
> int t_err;
> + struct kref refcount;
> wait_queue_head_t wq;
> struct p9_fcall tc;
> struct p9_fcall rc;
> @@ -233,6 +234,19 @@ int p9_client_lock_dotl(struct p9_fid *fid, struct p9_flock *flock, u8 *status);
> int p9_client_getlock_dotl(struct p9_fid *fid, struct p9_getlock *fl);
> void p9_fcall_fini(struct p9_fcall *fc);
> struct p9_req_t *p9_tag_lookup(struct p9_client *, u16);
> +
> +static inline void p9_req_get(struct p9_req_t *r)
> +{
> + kref_get(&r->refcount);
> +}
> +
> +static inline int p9_req_try_get(struct p9_req_t *r)
> +{
> + return kref_get_unless_zero(&r->refcount);
> +}
> +
> +int p9_req_put(struct p9_req_t *r);
> +
> void p9_client_cb(struct p9_client *c, struct p9_req_t *req, int status);
>
> int p9_parse_header(struct p9_fcall *, int32_t *, int8_t *, int16_t *, int);
> diff --git a/net/9p/client.c b/net/9p/client.c
> index 7942c0bfcc5b..83f2f0aadc14 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -310,6 +310,18 @@ p9_tag_alloc(struct p9_client *c, int8_t type, unsigned int max_size)
> if (tag < 0)
> goto free;
>
> + /* Init ref to two because in the general case there is one ref
> + * that is put asynchronously by a writer thread, one ref
> + * temporarily given by p9_tag_lookup and put by p9_client_cb
> + * in the recv thread, and one ref put by p9_remove_tag in the
> + * main thread. The only exception is virtio that does not use
> + * p9_tag_lookup but does not have a writer thread either
> + * (the write happens synchronously in the request/zc_request
> + * callback), so p9_client_cb eats the second ref there
> + * as the pointer is duplicated directly by virtqueue_add_sgs()
> + */
> + refcount_set(&req->refcount.refcount, 2);
> +
> return req;
>
> free:
> @@ -333,10 +345,21 @@ struct p9_req_t *p9_tag_lookup(struct p9_client *c, u16 tag)
> struct p9_req_t *req;
>
> rcu_read_lock();
> +again:
> req = idr_find(&c->reqs, tag);
> - /* There's no refcount on the req; a malicious server could cause
> - * us to dereference a NULL pointer
> - */
> + if (req) {
> + /* We have to be careful with the req found under rcu_read_lock
> + * Thanks to SLAB_TYPESAFE_BY_RCU we can safely try to get the
> + * ref again without corrupting other data, then check again
> + * that the tag matches once we have the ref
> + */
> + if (!p9_req_try_get(req))
> + goto again;
> + if (req->tc.tag != tag) {
> + p9_req_put(req);
> + goto again;
> + }
> + }
> rcu_read_unlock();
>
> return req;
> @@ -350,7 +373,7 @@ EXPORT_SYMBOL(p9_tag_lookup);
> *
> * Context: Any context.
> */
> -static void p9_tag_remove(struct p9_client *c, struct p9_req_t *r)
> +static int p9_tag_remove(struct p9_client *c, struct p9_req_t *r)
> {
> unsigned long flags;
> u16 tag = r->tc.tag;
> @@ -359,11 +382,23 @@ static void p9_tag_remove(struct p9_client *c, struct p9_req_t *r)
> spin_lock_irqsave(&c->lock, flags);
> idr_remove(&c->reqs, tag);
> spin_unlock_irqrestore(&c->lock, flags);
> + return p9_req_put(r);
> +}
> +
> +static void p9_req_free(struct kref *ref)
> +{
> + struct p9_req_t *r = container_of(ref, struct p9_req_t, refcount);
> p9_fcall_fini(&r->tc);
> p9_fcall_fini(&r->rc);
> kmem_cache_free(p9_req_cache, r);
> }
>
> +int p9_req_put(struct p9_req_t *r)
> +{
> + return kref_put(&r->refcount, p9_req_free);
> +}
> +EXPORT_SYMBOL(p9_req_put);
> +
> /**
> * p9_tag_cleanup - cleans up tags structure and reclaims resources
> * @c: v9fs client struct
> @@ -379,7 +414,9 @@ static void p9_tag_cleanup(struct p9_client *c)
> rcu_read_lock();
> idr_for_each_entry(&c->reqs, req, id) {
> pr_info("Tag %d still in use\n", id);
> - p9_tag_remove(c, req);
> + if (p9_tag_remove(c, req) == 0)
> + pr_warn("Packet with tag %d has still references",
> + req->tc.tag);
> }
> rcu_read_unlock();
> }
> @@ -403,6 +440,7 @@ void p9_client_cb(struct p9_client *c, struct p9_req_t *req, int status)
>
> wake_up(&req->wq);
> p9_debug(P9_DEBUG_MUX, "wakeup: %d\n", req->tc.tag);
> + p9_req_put(req);
> }
> EXPORT_SYMBOL(p9_client_cb);
>
> @@ -682,6 +720,8 @@ static struct p9_req_t *p9_client_prepare_req(struct p9_client *c,
> return req;
> reterr:
> p9_tag_remove(c, req);
> + /* We have to put also the 2nd reference as it won't be used */
> + p9_req_put(req);
> return ERR_PTR(err);
> }
>
> @@ -716,6 +756,8 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
>
> err = c->trans_mod->request(c, req);
> if (err < 0) {
> + /* write won't happen */
> + p9_req_put(req);
> if (err != -ERESTARTSYS && err != -EFAULT)
> c->status = Disconnected;
> goto recalc_sigpending;
> @@ -2241,7 +2283,7 @@ EXPORT_SYMBOL(p9_client_readlink);
>
> int __init p9_client_init(void)
> {
> - p9_req_cache = KMEM_CACHE(p9_req_t, 0);
> + p9_req_cache = KMEM_CACHE(p9_req_t, SLAB_TYPESAFE_BY_RCU);
> return p9_req_cache ? 0 : -ENOMEM;
> }
>
> diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
> index 20f46f13fe83..686e24e355d0 100644
> --- a/net/9p/trans_fd.c
> +++ b/net/9p/trans_fd.c
> @@ -132,6 +132,7 @@ struct p9_conn {
> struct list_head req_list;
> struct list_head unsent_req_list;
> struct p9_req_t *req;
> + struct p9_req_t *wreq;
Why adding a wreq for write work? And I wonder we should rename req to
rreq?
> char tmp_buf[7];
> struct p9_fcall rc;
> int wpos;
> @@ -383,6 +384,7 @@ static void p9_read_work(struct work_struct *work)
> m->rc.sdata = NULL;
> m->rc.offset = 0;
> m->rc.capacity = 0;
> + p9_req_put(m->req);
> m->req = NULL;
> }
>
> @@ -472,6 +474,8 @@ static void p9_write_work(struct work_struct *work)
> m->wbuf = req->tc.sdata;
> m->wsize = req->tc.size;
> m->wpos = 0;
> + p9_req_get(req);
> + m->wreq = req;
> spin_unlock(&m->client->lock);
> }
>
> @@ -492,8 +496,11 @@ static void p9_write_work(struct work_struct *work)
> }
>
> m->wpos += err;
> - if (m->wpos == m->wsize)
> + if (m->wpos == m->wsize) {
> m->wpos = m->wsize = 0;
> + p9_req_put(m->wreq);
> + m->wreq = NULL;
> + }
>
> end_clear:
> clear_bit(Wworksched, &m->wsched);
> @@ -694,6 +701,7 @@ static int p9_fd_cancel(struct p9_client *client, struct p9_req_t *req)
> if (req->status == REQ_STATUS_UNSENT) {
> list_del(&req->req_list);
> req->status = REQ_STATUS_FLSHD;
> + p9_req_put(req);
> ret = 0;
> }
> spin_unlock(&client->lock);
> @@ -711,6 +719,7 @@ static int p9_fd_cancelled(struct p9_client *client, struct p9_req_t *req)
> spin_lock(&client->lock);
> list_del(&req->req_list);
> spin_unlock(&client->lock);
> + p9_req_put(req);
>
> return 0;
> }
> diff --git a/net/9p/trans_rdma.c b/net/9p/trans_rdma.c
> index c60655c90c9e..8cff368a11e3 100644
> --- a/net/9p/trans_rdma.c
> +++ b/net/9p/trans_rdma.c
> @@ -365,6 +365,7 @@ send_done(struct ib_cq *cq, struct ib_wc *wc)
> c->busa, c->req->tc.size,
> DMA_TO_DEVICE);
> up(&rdma->sq_sem);
> + p9_req_put(c->req);
> kfree(c);
> }
>
>
^ permalink raw reply
* Re: The recvmsg() with IP_PKTINFO for local addresses returns various ipi_ifindex
From: David Ahern @ 2018-08-12 22:34 UTC (permalink / raw)
To: Damir Mansurov, netdev; +Cc: Alexandra N. Kossovsky
In-Reply-To: <e6e875ca-096f-924d-c1c6-f40339ae3a0c@oktetlabs.ru>
On 8/9/18 2:13 AM, Damir Mansurov wrote:
> Greetings,
>
> I use the IP_PKTINFO to detect ipi_ifindex from which the packet was
> arrived, it used to work for local addresses also.
>
> For local addresses ipi_ifindex always returned 1, but starting from
> Linux 4.14 ip_ifindex began to return various values.
>
> Example host configuration:
> 1: lo: 127.0.0.1
> 2: eth0: 192.168.3.45
> 3: eth1: 192.168.4.45
>
> I use sendto() with addresses {127.0.0.1, 192.168.3.45 and
> 192.168.4.45}, call recvmsg() and than use standard procedure to get
> ipi_ifindex, it shows results:
>
> | ipi_ifindex | ipi_ifindex
> sendto(address) | Linux ver < 4.14 | Linux ver >= 4.14
> ---------------------------------------------------------------
> 127.0.0.1 | 1 | 1
> ---------------------------------------------------------------
> 192.168.3.45 | 1 | 2
> ---------------------------------------------------------------
> 192.168.4.45 | 1 | 3
>
>
> It seems that this behavior depends from commit:
> net: ipv4: set orig_oif based on fib result for local traffic
>
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-4.14.y&id=839da4d98960bcc68e6b7b945b33ad3916ec1e92
>
>
> I believe that ipi_ifindex for local addresses should be 1.
> Is there a Bug for Linux >= 4.14 or is this a valid behavior?
The index should be the device with the address. It was reporting
loopback prior to the mentioned commit because loopback is used for
packet Tx which is an implementation detail for sending packets locally.
^ permalink raw reply
* Re: [PATCH iproute2-next 3/3] q_netem: slotting with non-uniform distribution
From: David Ahern @ 2018-08-12 22:18 UTC (permalink / raw)
To: Yousuk Seung, netdev
Cc: Stephen Hemminger, Michael McLennan, Priyaranjan Jha,
Neal Cardwell, Dave Taht
In-Reply-To: <20180806170953.164776-4-ysseung@google.com>
On 8/6/18 11:09 AM, Yousuk Seung wrote:
> @@ -417,21 +421,53 @@ static int netem_parse_opt(struct qdisc_util *qu, int argc, char **argv,
> }
> }
> } else if (matches(*argv, "slot") == 0) {
> - NEXT_ARG();
> - present[TCA_NETEM_SLOT] = 1;
> - if (get_time64(&slot.min_delay, *argv)) {
> - explain1("slot min_delay");
> - return -1;
> - }
> if (NEXT_IS_NUMBER()) {
> NEXT_ARG();
> - if (get_time64(&slot.max_delay, *argv)) {
> - explain1("slot min_delay max_delay");
> + present[TCA_NETEM_SLOT] = 1;
> + if (get_time64(&slot.min_delay, *argv)) {
> + explain1("slot min_delay");
> + return -1;
> + }
> + if (NEXT_IS_NUMBER()) {
> + NEXT_ARG();
> + if (get_time64(&slot.max_delay, *argv)) {
> + explain1("slot min_delay max_delay");
> + return -1;
> + }
> + }
> + if (slot.max_delay < slot.min_delay)
> + slot.max_delay = slot.min_delay;
> + } else {
> + NEXT_ARG();
> + if (strcmp(*argv, "distribution") == 0) {
> + present[TCA_NETEM_SLOT] = 1;
> + NEXT_ARG();
> + slot_dist_data = calloc(sizeof(slot_dist_data[0]), MAX_DIST);
if (!slot_dist_data) ...
^ permalink raw reply
* Re: [PATCH iproute2-next 2/3] q_netem: support delivering packets in delayed time slots
From: David Ahern @ 2018-08-12 22:14 UTC (permalink / raw)
To: Yousuk Seung, netdev
Cc: Stephen Hemminger, Michael McLennan, Priyaranjan Jha, Dave Taht,
Neal Cardwell
In-Reply-To: <20180806170953.164776-3-ysseung@google.com>
On 8/6/18 11:09 AM, Yousuk Seung wrote:
> diff --git a/tc/q_netem.c b/tc/q_netem.c
> index 9f9a9b3df255..f52a36b6c31c 100644
> --- a/tc/q_netem.c
> +++ b/tc/q_netem.c
> @@ -40,7 +40,10 @@ static void explain(void)
> " [ loss gemodel PERCENT [R [1-H [1-K]]]\n" \
> " [ ecn ]\n" \
> " [ reorder PRECENT [CORRELATION] [ gap DISTANCE ]]\n" \
> -" [ rate RATE [PACKETOVERHEAD] [CELLSIZE] [CELLOVERHEAD]]\n");
> +" [ rate RATE [PACKETOVERHEAD] [CELLSIZE] [CELLOVERHEAD]]\n" \
> +" [ slot MIN_DELAY [MAX_DELAY] [packets MAX_PACKETS]" \
> +" [bytes MAX_BYTES]]\n" \
> + );
> }
>
> static void explain1(const char *arg)
> @@ -164,6 +167,7 @@ static int netem_parse_opt(struct qdisc_util *qu, int argc, char **argv,
> struct tc_netem_gimodel gimodel;
> struct tc_netem_gemodel gemodel;
> struct tc_netem_rate rate = {};
> + struct tc_netem_slot slot = {};
> __s16 *dist_data = NULL;
> __u16 loss_type = NETEM_LOSS_UNSPEC;
> int present[__TCA_NETEM_MAX] = {};
> @@ -412,6 +416,44 @@ static int netem_parse_opt(struct qdisc_util *qu, int argc, char **argv,
> return -1;
> }
> }
> + } else if (matches(*argv, "slot") == 0) {
> + NEXT_ARG();
> + present[TCA_NETEM_SLOT] = 1;
> + if (get_time64(&slot.min_delay, *argv)) {
> + explain1("slot min_delay");
> + return -1;
> + }
> + if (NEXT_IS_NUMBER()) {
> + NEXT_ARG();
> + if (get_time64(&slot.max_delay, *argv)) {
> + explain1("slot min_delay max_delay");
> + return -1;
> + }
> + }
> + if (slot.max_delay < slot.min_delay)
If max_delay is user specified, this should generate an error rather
than just overwriting the value.
> + slot.max_delay = slot.min_delay;
> + if (NEXT_ARG_OK() &&
> + matches(*(argv+1), "packets") == 0) {
> + NEXT_ARG();
> + if (!NEXT_ARG_OK() ||
> + get_s32(&slot.max_packets, *(argv+1), 0)) {
> + explain1("slot packets");
> + return -1;
> + }
> + NEXT_ARG();
> + }
> + if (NEXT_ARG_OK() &&
> + matches(*(argv+1), "bytes") == 0) {
> + unsigned int max_bytes;
> + NEXT_ARG();
> + if (!NEXT_ARG_OK() ||
> + get_size(&max_bytes, *(argv+1))) {
> + explain1("slot bytes");
> + return -1;
> + }
> + slot.max_bytes = (int) max_bytes;
> + NEXT_ARG();
> + }
> } else if (strcmp(*argv, "help") == 0) {
> explain();
> return -1;
^ permalink raw reply
* Re: [PATCH iproute2-next 1/3] tc: support conversions to or from 64 bit nanosecond-based time
From: David Ahern @ 2018-08-12 22:09 UTC (permalink / raw)
To: Yousuk Seung, netdev
Cc: Stephen Hemminger, Michael McLennan, Priyaranjan Jha, Dave Taht,
Neal Cardwell
In-Reply-To: <20180806170953.164776-2-ysseung@google.com>
On 8/6/18 11:09 AM, Yousuk Seung wrote:
> diff --git a/tc/tc_core.h b/tc/tc_core.h
> index 1dfa9a4f773b..a0fe0923d171 100644
> --- a/tc/tc_core.h
> +++ b/tc/tc_core.h
> @@ -7,6 +7,10 @@
>
> #define TIME_UNITS_PER_SEC 1000000
>
> +#define NSEC_PER_USEC 1000
> +#define NSEC_PER_MSEC 1000000
> +#define NSEC_PER_SEC 1000000000LL
> +
These are not specific to tc so a header in include is a better location
(utils.h or a new one)
> enum link_layer {
> LINKLAYER_UNSPEC,
> LINKLAYER_ETHERNET,
> diff --git a/tc/tc_util.c b/tc/tc_util.c
> index d7578528a31b..c39c9046dcae 100644
> --- a/tc/tc_util.c
> +++ b/tc/tc_util.c
Similarly for these time functions - not specific to tc so move to
lib/utils.c
> @@ -385,6 +385,61 @@ char *sprint_ticks(__u32 ticks, char *buf)
> return sprint_time(tc_core_tick2time(ticks), buf);
> }
>
> +/* 64 bit times are represented internally in nanoseconds */
> +int get_time64(__s64 *time, const char *str)
__u64 seems more appropriate than __s64
> +{
> + double nsec;
> + char *p;
> +
> + nsec = strtod(str, &p);
> + if (p == str)
> + return -1;
> +
> + if (*p) {
> + if (strcasecmp(p, "s") == 0 ||
> + strcasecmp(p, "sec") == 0 ||
> + strcasecmp(p, "secs") == 0)
> + nsec *= NSEC_PER_SEC;
> + else if (strcasecmp(p, "ms") == 0 ||
> + strcasecmp(p, "msec") == 0 ||
> + strcasecmp(p, "msecs") == 0)
> + nsec *= NSEC_PER_MSEC;
> + else if (strcasecmp(p, "us") == 0 ||
> + strcasecmp(p, "usec") == 0 ||
> + strcasecmp(p, "usecs") == 0)
> + nsec *= NSEC_PER_USEC;
> + else if (strcasecmp(p, "ns") == 0 ||
> + strcasecmp(p, "nsec") == 0 ||
> + strcasecmp(p, "nsecs") == 0)
strncasecmp would be more efficient
> + nsec *= 1;
> + else
> + return -1;
> + }
> +
> + *time = nsec;
> + return 0;
> +}
> +
> +void print_time64(char *buf, int len, __s64 time)
> +{
> + double nsec = time;
> +
> + if (time >= NSEC_PER_SEC)
> + snprintf(buf, len, "%.3fs", nsec/NSEC_PER_SEC);
> + else if (time >= NSEC_PER_MSEC)
> + snprintf(buf, len, "%.3fms", nsec/NSEC_PER_MSEC);
> + else if (time >= NSEC_PER_USEC)
> + snprintf(buf, len, "%.3fus", nsec/NSEC_PER_USEC);
> + else
> + snprintf(buf, len, "%lldns", time);
> +}
> +
> +char *sprint_time64(__s64 time, char *buf)
> +{
> + print_time64(buf, SPRINT_BSIZE-1, time);
> + return buf;
> +}
> +
> int get_size(unsigned int *size, const char *str)
> {
> double sz;
^ permalink raw reply
* Re: [PATCH iproute2-next] Add SKB Priority qdisc support in tc(8)
From: David Ahern @ 2018-08-12 21:45 UTC (permalink / raw)
To: Nishanth Devarajan, stephen; +Cc: netdev, doucette, michel
In-Reply-To: <20180808182440.GA5929@gmail.com>
On 8/8/18 12:24 PM, Nishanth Devarajan wrote:
> sch_skbprio is a qdisc that prioritizes packets according to their skb->priority
> field. Under congestion, it drops already-enqueued lower priority packets to
> make space available for higher priority packets. Skbprio was conceived as a
> solution for denial-of-service defenses that need to route packets with
> different priorities as a means to overcome DoS attacks.
>
> Signed-off-by: Nishanth Devarajan <ndev2021@gmail.com>
> Reviewed-by: Michel Machado <michel@digirati.com.br>
> ---
> include/uapi/linux/pkt_sched.h | 7 ++++
> man/man8/tc-skbprio.8 | 70 ++++++++++++++++++++++++++++++++++++
> tc/Makefile | 1 +
> tc/q_skbprio.c | 81 ++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 159 insertions(+)
> create mode 100644 man/man8/tc-skbprio.8
> create mode 100644 tc/q_skbprio.c
>
Does not apply cleanly to iproute2-next. Also, you have a couple of
lines > 80 columns that can be easily corrected.
^ permalink raw reply
* Re: [PATCHi iproute2-next] ip: show min and max mtu
From: David Ahern @ 2018-08-12 21:25 UTC (permalink / raw)
To: Stephen Hemminger, netdev; +Cc: Stephen Hemminger
In-Reply-To: <20180727204350.21799-1-stephen@networkplumber.org>
On 7/27/18 2:43 PM, Stephen Hemminger wrote:
> From: Stephen Hemminger <sthemmin@microsoft.com>
>
> Add min/max MTU to the link details
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> include/uapi/linux/if_link.h | 2 ++
> ip/ipaddress.c | 10 ++++++++++
> 2 files changed, 12 insertions(+)
>
applied to iproute2-next. Thanks
^ permalink raw reply
* [PATCH net-next] ieee802154: hwsim: using right kind of iteration
From: Alexander Aring @ 2018-08-12 20:24 UTC (permalink / raw)
To: netdev; +Cc: linux-wpan, kernel, Alexander Aring, Stefan Schmidt
This patch fixes the error path to unsubscribe all other phy's from
current phy. The actually code using a wrong kind of list iteration may
copied from the case to unsubscribe the current phy from all other
phy's.
Cc: Stefan Schmidt <stefan@datenfreihafen.org>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb")
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
drivers/net/ieee802154/mac802154_hwsim.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
index 07a493dea11e..bf70ab892e69 100644
--- a/drivers/net/ieee802154/mac802154_hwsim.c
+++ b/drivers/net/ieee802154/mac802154_hwsim.c
@@ -735,10 +735,12 @@ static int hwsim_subscribe_all_others(struct hwsim_phy *phy)
return 0;
me_fail:
- list_for_each_entry(phy, &hwsim_phys, list) {
+ rcu_read_lock();
+ list_for_each_entry_rcu(e, &phy->edges, list) {
list_del_rcu(&e->list);
hwsim_free_edge(e);
}
+ rcu_read_unlock();
sub_fail:
hwsim_edge_unsubscribe_me(phy);
return -ENOMEM;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] drivers/net/usb/r8152: change rtl8152_system_suspend to be void function
From: Dan Carpenter @ 2018-08-12 22:09 UTC (permalink / raw)
To: kbuild, zhong jiang; +Cc: netdev, edumazet, kbuild-all, linux-kernel, davem
In-Reply-To: <1533738407-10495-1-git-send-email-zhongjiang@huawei.com>
Hi zhong,
Thank you for the patch! Perhaps something to improve:
url: https://github.com/0day-ci/linux/commits/zhong-jiang/drivers-net-usb-r8152-change-rtl8152_system_suspend-to-be-void-function/20180809-111737
New smatch warnings:
drivers/net/usb/r8152.c:4447 rtl8152_suspend() error: uninitialized symbol 'ret'.
Old smatch warnings:
drivers/net/usb/r8152.c:4660 rtl8152_get_strings() error: memcpy() '*rtl8152_gstrings' too small (32 vs 416)
# https://github.com/0day-ci/linux/commit/31b61d6a0989c067332fa1ae07055b3e406c17cd
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 31b61d6a0989c067332fa1ae07055b3e406c17cd
vim +/ret +4447 drivers/net/usb/r8152.c
8fb280616 hayeswang 2017-01-10 4432
8fb280616 hayeswang 2017-01-10 4433 static int rtl8152_suspend(struct usb_interface *intf, pm_message_t message)
8fb280616 hayeswang 2017-01-10 4434 {
8fb280616 hayeswang 2017-01-10 4435 struct r8152 *tp = usb_get_intfdata(intf);
8fb280616 hayeswang 2017-01-10 4436 int ret;
8fb280616 hayeswang 2017-01-10 4437
8fb280616 hayeswang 2017-01-10 4438 mutex_lock(&tp->control);
8fb280616 hayeswang 2017-01-10 4439
8fb280616 hayeswang 2017-01-10 4440 if (PMSG_IS_AUTO(message))
a9c54ad2c hayeswang 2017-01-25 4441 ret = rtl8152_runtime_suspend(tp);
8fb280616 hayeswang 2017-01-10 4442 else
31b61d6a0 zhong jiang 2018-08-08 4443 rtl8152_system_suspend(tp);
8fb280616 hayeswang 2017-01-10 4444
b54032736 hayeswang 2014-10-09 4445 mutex_unlock(&tp->control);
b54032736 hayeswang 2014-10-09 4446
6cc69f2a4 hayeswang 2014-10-17 @4447 return ret;
ac718b693 hayeswang 2013-05-02 4448 }
ac718b693 hayeswang 2013-05-02 4449
^ permalink raw reply
* Re: [RFC net-next 00/15] net: A socket API for LoRa
From: Andreas Färber @ 2018-08-12 17:59 UTC (permalink / raw)
To: Jian-Hong Pan, Alan Cox
Cc: netdev, linux-arm-kernel, linux-kernel, Jiri Pirko,
Marcel Holtmann, David S. Miller, Matthias Brugger, Janus Piwek,
Michael Röder, Dollar Chen, Ken Yu, Konstantin Böhm,
Jan Jongboom, Jon Ortego, contact@snootlab.com, Ben Whitten,
Brian Ray, lora, Alexander Graf, Michal Kubeček
In-Reply-To: <CAC=mGzhaLiSj5-+euM-YTRLwNDi8QJY0fAbiZbUadkPhZGRh_Q@mail.gmail.com>
Am 12.08.2018 um 18:37 schrieb Jian-Hong Pan:
> Alan Cox <gnomes@lxorguk.ukuu.org.uk> 於 2018年8月10日 週五 下午11:57寫道：
>>
>>> The sleep/idle/stop mitigate the unconcerned RF signals or messages.
>>
>> At the physical level it's irrelevant. If we are receiving then we might
>> hear more things we later discard. It's not running on a tiny
>> microcontroller so the extra CPU cycles are not going to kill us.
>
> According different power resource, LoRaWAN defines Class A, B and C.
> Class A is the basic and both Class B and C devices must also
> implement the feature of Class A.
[...]
> So, yes! Class C opens the RX windows almost all the time, except the TX time.
> And uses different channel to avoid the reflection noise (*).
>
> However, Class C must also implements Class A and C is more complex than A.
> I think starting from the simpler one and adding more features and
> complexity in the future will be a better practice.
Jian-Hong, you've failed to come up with any practical proposal or patch
how to implement what you are saying LoRaWAN requires. Doing the
impossible is never simpler!
Implementing a simple back-off timer sounds doable by comparison.
May I remind you, LoRa is the simpler step before LoRaWAN - if our
layering is done right, someone else might choose to implement LoRaWAN
in userspace based on PF_LORA. There is absolutely no reason to hardcode
any LoRaWAN settings at device driver level for e.g. SX1276.
>>>> How do you plan to deal with routing if you've got multiple devices ?
>>>
>>> For LoRaWAN, it is a star topology.
>>
>> No the question was much more how you plan to deal with it in the OS. If
>> for example I want to open a LORA connection to something, then there
>> needs to be a proper process to figure out where the target is and how to
>> get traffic to them.
>>
>> I guess it's best phrased as
>>
>> - What does a struct sockaddr_lora look like
>
> According to LoRaWAN spec, the Data Message only has the device's
> DevAddr (the device's address in 4 bytes) field related to "address".
> The device just sends the uplink Data Message through the interface
> and does not know the destination. Then, a LoRaWAN gateway receives
> the uplink Data Message and forwards to the designated network server.
> So, end device does not care about the destination. It only knows
> there is a gateway will forward its message to some where.
> Therefore, only the DevAddr as the source address will be meaningful
> for uplink Data Message.
Note that he was asking about sockaddr_lora, not LoRaWAN.
The simple answer is that, inspired by CAN, it uses an ifindex to select
the interface the user asked to use. That then also answers Alan's next
question: This ifindex determines which interface it goes out to.
sockaddr_lora was in patch 02/15, latest code here:
https://git.kernel.org/pub/scm/linux/kernel/git/afaerber/linux-lora.git/tree/include/uapi/linux/lora.h?h=lora-next
>> - How does the kernel decide which interface it goes out of (if any), and
>> if it loops back
>
> There is the MAC Header in the Data Message which is one byte.
> Bits 5 to 7 indicate which kind of type the message is.
> 000: Join Request
> 001: Join Accept
> 010: Unconfirmed Data Up
> 011: Unconfirmed Data Down
> 100: Confirmed Data Up
> 101: Confirmed Data Down
> 110: RFU
> 111: Proprietary
>
> So, end device only accepts the types of downlink and the matched
> DevAddr (the device's address) in downlink Data Message for RX.
LoRaWAN is an entirely different story, therefore my agreement that we
need a separate PF_LORAWAN and sockaddr_lorawan for EUI addressing.
I still think the user will need to explicitly say which interface they
want to bind their socket to. AFAIU the device EUI is more comparable to
an Ethernet MAC address than to an IP address that the user would
configure routes for. If you have e.g. four SX1301 devices then they may
be configured for different frequencies, bandwidths, etc. but unlikely
for certain destination/source addresses. So either all that data comes
via sockaddr (which I was discussing in the FSK/OOK part of this thread)
or the user needs to make the interface choice for us.
Loopback mode would require a separate virtual device driver such as
fakelr or vlora.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply
* [PATCH v2 bpf-next 1/4] bpf: Introduce bpf_skb_ancestor_cgroup_id helper
From: Andrey Ignatov @ 2018-08-12 17:49 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, tj, guro, yhs, kernel-team
In-Reply-To: <cover.1534095926.git.rdna@fb.com>
== Problem description ==
It's useful to be able to identify cgroup associated with skb in TC so
that a policy can be applied to this skb, and existing bpf_skb_cgroup_id
helper can help with this.
Though in real life cgroup hierarchy and hierarchy to apply a policy to
don't map 1:1.
It's often the case that there is a container and corresponding cgroup,
but there are many more sub-cgroups inside container, e.g. because it's
delegated to containerized application to control resources for its
subsystems, or to separate application inside container from infra that
belongs to containerization system (e.g. sshd).
At the same time it may be useful to apply a policy to container as a
whole.
If multiple containers like this are run on a host (what is often the
case) and many of them have sub-cgroups, it may not be possible to apply
per-container policy in TC with existing helpers such as
bpf_skb_under_cgroup or bpf_skb_cgroup_id:
* bpf_skb_cgroup_id will return id of immediate cgroup associated with
skb, i.e. if it's a sub-cgroup inside container, it can't be used to
identify container's cgroup;
* bpf_skb_under_cgroup can work only with one cgroup and doesn't scale,
i.e. if there are N containers on a host and a policy has to be
applied to M of them (0 <= M <= N), it'd require M calls to
bpf_skb_under_cgroup, and, if M changes, it'd require to rebuild &
load new BPF program.
== Solution ==
The patch introduces new helper bpf_skb_ancestor_cgroup_id that can be
used to get id of cgroup v2 that is an ancestor of cgroup associated
with skb at specified level of cgroup hierarchy.
That way admin can place all containers on one level of cgroup hierarchy
(what is a good practice in general and already used in many
configurations) and identify specific cgroup on this level no matter
what sub-cgroup skb is associated with.
E.g. if there is a cgroup hierarchy:
root/
root/container1/
root/container1/app11/
root/container1/app11/sub-app-a/
root/container1/app12/
root/container2/
root/container2/app21/
root/container2/app22/
root/container2/app22/sub-app-b/
, then having skb associated with root/container1/app11/sub-app-a/ it's
possible to get ancestor at level 1, what is container1 and apply policy
for this container, or apply another policy if it's container2.
Policies can be kept e.g. in a hash map where key is a container cgroup
id and value is an action.
Levels where container cgroups are created are usually known in advance
whether cgroup hierarchy inside container may be hard to predict
especially in case when its creation is delegated to containerized
application.
== Implementation details ==
The helper gets ancestor by walking parents up to specified level.
Another option would be to get different kind of "id" from
cgroup->ancestor_ids[level] and use it with idr_find() to get struct
cgroup for ancestor. But that would require radix lookup what doesn't
seem to be better (at least it's not obviously better).
Format of return value of the new helper is same as that of
bpf_skb_cgroup_id.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
include/linux/cgroup.h | 30 ++++++++++++++++++++++++++++++
include/uapi/linux/bpf.h | 21 ++++++++++++++++++++-
net/core/filter.c | 28 ++++++++++++++++++++++++++++
3 files changed, 78 insertions(+), 1 deletion(-)
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index c9fdf6f57913..32c553556bbd 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -553,6 +553,36 @@ static inline bool cgroup_is_descendant(struct cgroup *cgrp,
return cgrp->ancestor_ids[ancestor->level] == ancestor->id;
}
+/**
+ * cgroup_ancestor - find ancestor of cgroup
+ * @cgrp: cgroup to find ancestor of
+ * @ancestor_level: level of ancestor to find starting from root
+ *
+ * Find ancestor of cgroup at specified level starting from root if it exists
+ * and return pointer to it. Return NULL if @cgrp doesn't have ancestor at
+ * @ancestor_level.
+ *
+ * This function is safe to call as long as @cgrp is accessible.
+ */
+static inline struct cgroup *cgroup_ancestor(struct cgroup *cgrp,
+ int ancestor_level)
+{
+ struct cgroup *ptr;
+
+ if (cgrp->level < ancestor_level)
+ return NULL;
+
+ for (ptr = cgrp;
+ ptr && ptr->level > ancestor_level;
+ ptr = cgroup_parent(ptr))
+ ;
+
+ if (ptr && ptr->level == ancestor_level)
+ return ptr;
+
+ return NULL;
+}
+
/**
* task_under_cgroup_hierarchy - test task's membership of cgroup ancestry
* @task: the task to be tested
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 3102a2a23c31..66917a4eba27 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2093,6 +2093,24 @@ union bpf_attr {
* Return
* The id is returned or 0 in case the id could not be retrieved.
*
+ * u64 bpf_skb_ancestor_cgroup_id(struct sk_buff *skb, int ancestor_level)
+ * Description
+ * Return id of cgroup v2 that is ancestor of cgroup associated
+ * with the *skb* at the *ancestor_level*. The root cgroup is at
+ * *ancestor_level* zero and each step down the hierarchy
+ * increments the level. If *ancestor_level* == level of cgroup
+ * associated with *skb*, then return value will be same as that
+ * of **bpf_skb_cgroup_id**\ ().
+ *
+ * The helper is useful to implement policies based on cgroups
+ * that are upper in hierarchy than immediate cgroup associated
+ * with *skb*.
+ *
+ * The format of returned id and helper limitations are same as in
+ * **bpf_skb_cgroup_id**\ ().
+ * Return
+ * The id is returned or 0 in case the id could not be retrieved.
+ *
* u64 bpf_get_current_cgroup_id(void)
* Return
* A 64-bit integer containing the current cgroup id based
@@ -2207,7 +2225,8 @@ union bpf_attr {
FN(skb_cgroup_id), \
FN(get_current_cgroup_id), \
FN(get_local_storage), \
- FN(sk_select_reuseport),
+ FN(sk_select_reuseport), \
+ FN(skb_ancestor_cgroup_id),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/net/core/filter.c b/net/core/filter.c
index 22906b31d43f..15b9d2df92ca 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3778,6 +3778,32 @@ static const struct bpf_func_proto bpf_skb_cgroup_id_proto = {
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_CTX,
};
+
+BPF_CALL_2(bpf_skb_ancestor_cgroup_id, const struct sk_buff *, skb, int,
+ ancestor_level)
+{
+ struct sock *sk = skb_to_full_sk(skb);
+ struct cgroup *ancestor;
+ struct cgroup *cgrp;
+
+ if (!sk || !sk_fullsock(sk))
+ return 0;
+
+ cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
+ ancestor = cgroup_ancestor(cgrp, ancestor_level);
+ if (!ancestor)
+ return 0;
+
+ return ancestor->kn->id.id;
+}
+
+static const struct bpf_func_proto bpf_skb_ancestor_cgroup_id_proto = {
+ .func = bpf_skb_ancestor_cgroup_id,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_ANYTHING,
+};
#endif
static unsigned long bpf_xdp_copy(void *dst_buff, const void *src_buff,
@@ -4966,6 +4992,8 @@ tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
#ifdef CONFIG_SOCK_CGROUP_DATA
case BPF_FUNC_skb_cgroup_id:
return &bpf_skb_cgroup_id_proto;
+ case BPF_FUNC_skb_ancestor_cgroup_id:
+ return &bpf_skb_ancestor_cgroup_id_proto;
#endif
default:
return bpf_base_func_proto(func_id);
--
2.17.1
^ permalink raw reply related
* [PATCH v2 bpf-next 3/4] selftests/bpf: Add cgroup id helpers to bpf_helpers.h
From: Andrey Ignatov @ 2018-08-12 17:49 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, tj, guro, yhs, kernel-team
In-Reply-To: <cover.1534095926.git.rdna@fb.com>
Add bpf_skb_cgroup_id and bpf_skb_ancestor_cgroup_id helpers to
bpf_helpers.h to use them in tests and samples.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
tools/testing/selftests/bpf/bpf_helpers.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 5c32266c2c38..e4be7730222d 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -139,6 +139,10 @@ static unsigned long long (*bpf_get_current_cgroup_id)(void) =
(void *) BPF_FUNC_get_current_cgroup_id;
static void *(*bpf_get_local_storage)(void *map, unsigned long long flags) =
(void *) BPF_FUNC_get_local_storage;
+static unsigned long long (*bpf_skb_cgroup_id)(void *ctx) =
+ (void *) BPF_FUNC_skb_cgroup_id;
+static unsigned long long (*bpf_skb_ancestor_cgroup_id)(void *ctx, int level) =
+ (void *) BPF_FUNC_skb_ancestor_cgroup_id;
/* llvm builtin functions that eBPF C program may use to
* emit BPF_LD_ABS and BPF_LD_IND instructions
--
2.17.1
^ permalink raw reply related
* [PATCH v2 bpf-next 4/4] selftests/bpf: Selftest for bpf_skb_ancestor_cgroup_id
From: Andrey Ignatov @ 2018-08-12 17:49 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, tj, guro, yhs, kernel-team
In-Reply-To: <cover.1534095926.git.rdna@fb.com>
Add selftests for bpf_skb_ancestor_cgroup_id helper.
test_skb_cgroup_id.sh prepares testing interface and adds tc qdisc and
filter for it using BPF object compiled from test_skb_cgroup_id_kern.c
program.
BPF program in test_skb_cgroup_id_kern.c gets ancestor cgroup id using
the new helper at different levels of cgroup hierarchy that skb belongs
to, including root level and non-existing level, and saves it to the map
where the key is the level of corresponding cgroup and the value is its
id.
To trigger BPF program, user space program test_skb_cgroup_id_user is
run. It adds itself into testing cgroup and sends UDP datagram to
link-local multicast address of testing interface. Then it reads cgroup
ids saved in kernel for different levels from the BPF map and compares
them with those in user space. They must be equal for every level of
ancestry.
Example of run:
# ./test_skb_cgroup_id.sh
Wait for testing link-local IP to become available ... OK
Note: 8 bytes struct bpf_elf_map fixup performed due to size mismatch!
[PASS]
Signed-off-by: Andrey Ignatov <rdna@fb.com>
---
tools/testing/selftests/bpf/Makefile | 9 +-
.../selftests/bpf/test_skb_cgroup_id.sh | 62 ++++++
.../selftests/bpf/test_skb_cgroup_id_kern.c | 47 +++++
.../selftests/bpf/test_skb_cgroup_id_user.c | 187 ++++++++++++++++++
4 files changed, 302 insertions(+), 3 deletions(-)
create mode 100755 tools/testing/selftests/bpf/test_skb_cgroup_id.sh
create mode 100644 tools/testing/selftests/bpf/test_skb_cgroup_id_kern.c
create mode 100644 tools/testing/selftests/bpf/test_skb_cgroup_id_user.c
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index daed162043c2..fff7fb1285fc 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -34,7 +34,8 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test
test_btf_haskv.o test_btf_nokv.o test_sockmap_kern.o test_tunnel_kern.o \
test_get_stack_rawtp.o test_sockmap_kern.o test_sockhash_kern.o \
test_lwt_seg6local.o sendmsg4_prog.o sendmsg6_prog.o test_lirc_mode2_kern.o \
- get_cgroup_id_kern.o socket_cookie_prog.o test_select_reuseport_kern.o
+ get_cgroup_id_kern.o socket_cookie_prog.o test_select_reuseport_kern.o \
+ test_skb_cgroup_id_kern.o
# Order correspond to 'make run_tests' order
TEST_PROGS := test_kmod.sh \
@@ -45,10 +46,11 @@ TEST_PROGS := test_kmod.sh \
test_sock_addr.sh \
test_tunnel.sh \
test_lwt_seg6local.sh \
- test_lirc_mode2.sh
+ test_lirc_mode2.sh \
+ test_skb_cgroup_id.sh
# Compile but not part of 'make run_tests'
-TEST_GEN_PROGS_EXTENDED = test_libbpf_open test_sock_addr
+TEST_GEN_PROGS_EXTENDED = test_libbpf_open test_sock_addr test_skb_cgroup_id_user
include ../lib.mk
@@ -59,6 +61,7 @@ $(TEST_GEN_PROGS): $(BPFOBJ)
$(TEST_GEN_PROGS_EXTENDED): $(OUTPUT)/libbpf.a
$(OUTPUT)/test_dev_cgroup: cgroup_helpers.c
+$(OUTPUT)/test_skb_cgroup_id_user: cgroup_helpers.c
$(OUTPUT)/test_sock: cgroup_helpers.c
$(OUTPUT)/test_sock_addr: cgroup_helpers.c
$(OUTPUT)/test_socket_cookie: cgroup_helpers.c
diff --git a/tools/testing/selftests/bpf/test_skb_cgroup_id.sh b/tools/testing/selftests/bpf/test_skb_cgroup_id.sh
new file mode 100755
index 000000000000..42544a969abc
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_skb_cgroup_id.sh
@@ -0,0 +1,62 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2018 Facebook
+
+set -eu
+
+wait_for_ip()
+{
+ local _i
+ echo -n "Wait for testing link-local IP to become available "
+ for _i in $(seq ${MAX_PING_TRIES}); do
+ echo -n "."
+ if ping -6 -q -c 1 -W 1 ff02::1%${TEST_IF} >/dev/null 2>&1; then
+ echo " OK"
+ return
+ fi
+ sleep 1
+ done
+ echo 1>&2 "ERROR: Timeout waiting for test IP to become available."
+ exit 1
+}
+
+setup()
+{
+ # Create testing interfaces not to interfere with current environment.
+ ip link add dev ${TEST_IF} type veth peer name ${TEST_IF_PEER}
+ ip link set ${TEST_IF} up
+ ip link set ${TEST_IF_PEER} up
+
+ wait_for_ip
+
+ tc qdisc add dev ${TEST_IF} clsact
+ tc filter add dev ${TEST_IF} egress bpf obj ${BPF_PROG_OBJ} \
+ sec ${BPF_PROG_SECTION} da
+
+ BPF_PROG_ID=$(tc filter show dev ${TEST_IF} egress | \
+ awk '/ id / {sub(/.* id /, "", $0); print($1)}')
+}
+
+cleanup()
+{
+ ip link del ${TEST_IF} 2>/dev/null || :
+ ip link del ${TEST_IF_PEER} 2>/dev/null || :
+}
+
+main()
+{
+ trap cleanup EXIT 2 3 6 15
+ setup
+ ${PROG} ${TEST_IF} ${BPF_PROG_ID}
+}
+
+DIR=$(dirname $0)
+TEST_IF="test_cgid_1"
+TEST_IF_PEER="test_cgid_2"
+MAX_PING_TRIES=5
+BPF_PROG_OBJ="${DIR}/test_skb_cgroup_id_kern.o"
+BPF_PROG_SECTION="cgroup_id_logger"
+BPF_PROG_ID=0
+PROG="${DIR}/test_skb_cgroup_id_user"
+
+main
diff --git a/tools/testing/selftests/bpf/test_skb_cgroup_id_kern.c b/tools/testing/selftests/bpf/test_skb_cgroup_id_kern.c
new file mode 100644
index 000000000000..68cf9829f5a7
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_skb_cgroup_id_kern.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Facebook
+
+#include <linux/bpf.h>
+#include <linux/pkt_cls.h>
+
+#include <string.h>
+
+#include "bpf_helpers.h"
+
+#define NUM_CGROUP_LEVELS 4
+
+struct bpf_map_def SEC("maps") cgroup_ids = {
+ .type = BPF_MAP_TYPE_ARRAY,
+ .key_size = sizeof(__u32),
+ .value_size = sizeof(__u64),
+ .max_entries = NUM_CGROUP_LEVELS,
+};
+
+static __always_inline void log_nth_level(struct __sk_buff *skb, __u32 level)
+{
+ __u64 id;
+
+ /* [1] &level passed to external function that may change it, it's
+ * incompatible with loop unroll.
+ */
+ id = bpf_skb_ancestor_cgroup_id(skb, level);
+ bpf_map_update_elem(&cgroup_ids, &level, &id, 0);
+}
+
+SEC("cgroup_id_logger")
+int log_cgroup_id(struct __sk_buff *skb)
+{
+ /* Loop unroll can't be used here due to [1]. Unrolling manually.
+ * Number of calls should be in sync with NUM_CGROUP_LEVELS.
+ */
+ log_nth_level(skb, 0);
+ log_nth_level(skb, 1);
+ log_nth_level(skb, 2);
+ log_nth_level(skb, 3);
+
+ return TC_ACT_OK;
+}
+
+int _version SEC("version") = 1;
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/test_skb_cgroup_id_user.c b/tools/testing/selftests/bpf/test_skb_cgroup_id_user.c
new file mode 100644
index 000000000000..c121cc59f314
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_skb_cgroup_id_user.c
@@ -0,0 +1,187 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2018 Facebook
+
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <arpa/inet.h>
+#include <net/if.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+#include <sys/types.h>
+
+
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#include "bpf_rlimit.h"
+#include "cgroup_helpers.h"
+
+#define CGROUP_PATH "/skb_cgroup_test"
+#define NUM_CGROUP_LEVELS 4
+
+/* RFC 4291, Section 2.7.1 */
+#define LINKLOCAL_MULTICAST "ff02::1"
+
+static int mk_dst_addr(const char *ip, const char *iface,
+ struct sockaddr_in6 *dst)
+{
+ memset(dst, 0, sizeof(*dst));
+
+ dst->sin6_family = AF_INET6;
+ dst->sin6_port = htons(1025);
+
+ if (inet_pton(AF_INET6, ip, &dst->sin6_addr) != 1) {
+ log_err("Invalid IPv6: %s", ip);
+ return -1;
+ }
+
+ dst->sin6_scope_id = if_nametoindex(iface);
+ if (!dst->sin6_scope_id) {
+ log_err("Failed to get index of iface: %s", iface);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int send_packet(const char *iface)
+{
+ struct sockaddr_in6 dst;
+ char msg[] = "msg";
+ int err = 0;
+ int fd = -1;
+
+ if (mk_dst_addr(LINKLOCAL_MULTICAST, iface, &dst))
+ goto err;
+
+ fd = socket(AF_INET6, SOCK_DGRAM, 0);
+ if (fd == -1) {
+ log_err("Failed to create UDP socket");
+ goto err;
+ }
+
+ if (sendto(fd, &msg, sizeof(msg), 0, (const struct sockaddr *)&dst,
+ sizeof(dst)) == -1) {
+ log_err("Failed to send datagram");
+ goto err;
+ }
+
+ goto out;
+err:
+ err = -1;
+out:
+ if (fd >= 0)
+ close(fd);
+ return err;
+}
+
+int get_map_fd_by_prog_id(int prog_id)
+{
+ struct bpf_prog_info info = {};
+ __u32 info_len = sizeof(info);
+ __u32 map_ids[1];
+ int prog_fd = -1;
+ int map_fd = -1;
+
+ prog_fd = bpf_prog_get_fd_by_id(prog_id);
+ if (prog_fd < 0) {
+ log_err("Failed to get fd by prog id %d", prog_id);
+ goto err;
+ }
+
+ info.nr_map_ids = 1;
+ info.map_ids = (__u64) (unsigned long) map_ids;
+
+ if (bpf_obj_get_info_by_fd(prog_fd, &info, &info_len)) {
+ log_err("Failed to get info by prog fd %d", prog_fd);
+ goto err;
+ }
+
+ if (!info.nr_map_ids) {
+ log_err("No maps found for prog fd %d", prog_fd);
+ goto err;
+ }
+
+ map_fd = bpf_map_get_fd_by_id(map_ids[0]);
+ if (map_fd < 0)
+ log_err("Failed to get fd by map id %d", map_ids[0]);
+err:
+ if (prog_fd >= 0)
+ close(prog_fd);
+ return map_fd;
+}
+
+int check_ancestor_cgroup_ids(int prog_id)
+{
+ __u64 actual_ids[NUM_CGROUP_LEVELS], expected_ids[NUM_CGROUP_LEVELS];
+ __u32 level;
+ int err = 0;
+ int map_fd;
+
+ expected_ids[0] = 0x100000001; /* root cgroup */
+ expected_ids[1] = get_cgroup_id("");
+ expected_ids[2] = get_cgroup_id(CGROUP_PATH);
+ expected_ids[3] = 0; /* non-existent cgroup */
+
+ map_fd = get_map_fd_by_prog_id(prog_id);
+ if (map_fd < 0)
+ goto err;
+
+ for (level = 0; level < NUM_CGROUP_LEVELS; ++level) {
+ if (bpf_map_lookup_elem(map_fd, &level, &actual_ids[level])) {
+ log_err("Failed to lookup key %d", level);
+ goto err;
+ }
+ if (actual_ids[level] != expected_ids[level]) {
+ log_err("%llx (actual) != %llx (expected), level: %u\n",
+ actual_ids[level], expected_ids[level], level);
+ goto err;
+ }
+ }
+
+ goto out;
+err:
+ err = -1;
+out:
+ if (map_fd >= 0)
+ close(map_fd);
+ return err;
+}
+
+int main(int argc, char **argv)
+{
+ int cgfd = -1;
+ int err = 0;
+
+ if (argc < 3) {
+ fprintf(stderr, "Usage: %s iface prog_id\n", argv[0]);
+ exit(EXIT_FAILURE);
+ }
+
+ if (setup_cgroup_environment())
+ goto err;
+
+ cgfd = create_and_get_cgroup(CGROUP_PATH);
+ if (!cgfd)
+ goto err;
+
+ if (join_cgroup(CGROUP_PATH))
+ goto err;
+
+ if (send_packet(argv[1]))
+ goto err;
+
+ if (check_ancestor_cgroup_ids(atoi(argv[2])))
+ goto err;
+
+ goto out;
+err:
+ err = -1;
+out:
+ close(cgfd);
+ cleanup_cgroup_environment();
+ printf("[%s]\n", err ? "FAIL" : "PASS");
+ return err;
+}
--
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