* [PATCH bpf-next 1/7] xdp: add per mode attributes for attached programs
From: Jakub Kicinski @ 2018-07-12 3:36 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: oss-drivers, netdev, Jakub Kicinski
In-Reply-To: <20180712033644.23954-1-jakub.kicinski@netronome.com>
In preparation for support of simultaneous driver and hardware XDP
support add per-mode attributes. The catch-all IFLA_XDP_PROG_ID
will still be reported, but user space can now also access the
program ID in a new IFLA_XDP_<mode>_PROG_ID attribute.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Quentin Monnet <quentin.monnet@netronome.com>
---
include/uapi/linux/if_link.h | 3 +++
net/core/rtnetlink.c | 30 ++++++++++++++++++++++++++----
2 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index cf01b6824244..bc86c2b105ec 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -928,6 +928,9 @@ enum {
IFLA_XDP_ATTACHED,
IFLA_XDP_FLAGS,
IFLA_XDP_PROG_ID,
+ IFLA_XDP_DRV_PROG_ID,
+ IFLA_XDP_SKB_PROG_ID,
+ IFLA_XDP_HW_PROG_ID,
__IFLA_XDP_MAX,
};
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index e3f743c141b3..8ab95de1114c 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -964,7 +964,8 @@ static size_t rtnl_xdp_size(void)
{
size_t xdp_size = nla_total_size(0) + /* nest IFLA_XDP */
nla_total_size(1) + /* XDP_ATTACHED */
- nla_total_size(4); /* XDP_PROG_ID */
+ nla_total_size(4) + /* XDP_PROG_ID */
+ nla_total_size(4); /* XDP_<mode>_PROG_ID */
return xdp_size;
}
@@ -1378,16 +1379,17 @@ static u8 rtnl_xdp_attached_mode(struct net_device *dev, u32 *prog_id)
static int rtnl_xdp_fill(struct sk_buff *skb, struct net_device *dev)
{
+ u32 prog_attr, prog_id;
struct nlattr *xdp;
- u32 prog_id;
int err;
+ u8 mode;
xdp = nla_nest_start(skb, IFLA_XDP);
if (!xdp)
return -EMSGSIZE;
- err = nla_put_u8(skb, IFLA_XDP_ATTACHED,
- rtnl_xdp_attached_mode(dev, &prog_id));
+ mode = rtnl_xdp_attached_mode(dev, &prog_id);
+ err = nla_put_u8(skb, IFLA_XDP_ATTACHED, mode);
if (err)
goto err_cancel;
@@ -1395,6 +1397,26 @@ static int rtnl_xdp_fill(struct sk_buff *skb, struct net_device *dev)
err = nla_put_u32(skb, IFLA_XDP_PROG_ID, prog_id);
if (err)
goto err_cancel;
+
+ switch (mode) {
+ case XDP_ATTACHED_DRV:
+ prog_attr = IFLA_XDP_DRV_PROG_ID;
+ break;
+ case XDP_ATTACHED_SKB:
+ prog_attr = IFLA_XDP_SKB_PROG_ID;
+ break;
+ case XDP_ATTACHED_HW:
+ prog_attr = IFLA_XDP_HW_PROG_ID;
+ break;
+ case XDP_ATTACHED_NONE:
+ default:
+ err = -EINVAL;
+ goto err_cancel;
+ }
+
+ err = nla_put_u32(skb, prog_attr, prog_id);
+ if (err)
+ goto err_cancel;
}
nla_nest_end(skb, xdp);
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 0/7] xdp: simultaneous driver and HW XDP
From: Jakub Kicinski @ 2018-07-12 3:36 UTC (permalink / raw)
To: alexei.starovoitov, daniel; +Cc: oss-drivers, netdev, Jakub Kicinski
Hi!
This set is adding support for loading driver and offload XDP
at the same time. This enables advanced use cases where some
of the work is offloaded to the NIC and some is done by the host.
Separate netlink attributes are added for each mode of operation.
Driver callbacks for offload are cleaned up a little, including
removal of .prog_attached flag.
Jakub Kicinski (7):
xdp: add per mode attributes for attached programs
xdp: don't make drivers report attachment mode
xdp: factor out common program/flags handling from drivers
xdp: support simultaneous driver and hw XDP attachment
netdevsim: add support for simultaneous driver and hw XDP
selftests/bpf: add test for multiple programs
nfp: add support for simultaneous driver and hw XDP
drivers/net/ethernet/broadcom/bnxt/bnxt_xdp.c | 1 -
.../net/ethernet/cavium/thunder/nicvf_main.c | 1 -
drivers/net/ethernet/intel/i40e/i40e_main.c | 1 -
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 1 -
.../net/ethernet/intel/ixgbevf/ixgbevf_main.c | 1 -
.../net/ethernet/mellanox/mlx4/en_netdev.c | 1 -
.../net/ethernet/mellanox/mlx5/core/en_main.c | 1 -
drivers/net/ethernet/netronome/nfp/bpf/main.c | 11 +--
drivers/net/ethernet/netronome/nfp/nfp_net.h | 10 ++-
.../ethernet/netronome/nfp/nfp_net_common.c | 58 ++++++---------
.../net/ethernet/qlogic/qede/qede_filter.c | 1 -
drivers/net/netdevsim/bpf.c | 41 ++++-------
drivers/net/netdevsim/netdev.c | 3 +-
drivers/net/netdevsim/netdevsim.h | 6 +-
drivers/net/tun.c | 1 -
drivers/net/virtio_net.c | 1 -
include/linux/netdevice.h | 12 ++--
include/net/xdp.h | 13 ++++
include/uapi/linux/if_link.h | 4 ++
net/core/dev.c | 48 +++++++------
net/core/rtnetlink.c | 71 ++++++++++++++-----
net/core/xdp.c | 34 +++++++++
tools/testing/selftests/bpf/test_offload.py | 71 ++++++++++++++++---
23 files changed, 246 insertions(+), 146 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCH net-next v5 0/4] net: vhost: improve performance when enable busyloop
From: Michael S. Tsirkin @ 2018-07-12 3:34 UTC (permalink / raw)
To: Jason Wang
Cc: Tonghao Zhang, makita.toshiaki, virtualization,
Linux Kernel Network Developers
In-Reply-To: <0dcaf8f1-01b5-417d-420b-d5b716a82a8a@redhat.com>
On Thu, Jul 12, 2018 at 11:26:12AM +0800, Jason Wang wrote:
>
>
> On 2018年07月11日 19:59, Michael S. Tsirkin wrote:
> > On Wed, Jul 11, 2018 at 01:12:59PM +0800, Jason Wang wrote:
> > >
> > > On 2018年07月11日 11:49, Tonghao Zhang wrote:
> > > > On Wed, Jul 11, 2018 at 10:56 AM Jason Wang <jasowang@redhat.com> wrote:
> > > > >
> > > > > On 2018年07月04日 12:31, xiangxia.m.yue@gmail.com wrote:
> > > > > > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > > > > >
> > > > > > This patches improve the guest receive and transmit performance.
> > > > > > On the handle_tx side, we poll the sock receive queue at the same time.
> > > > > > handle_rx do that in the same way.
> > > > > >
> > > > > > For more performance report, see patch 4.
> > > > > >
> > > > > > v4 -> v5:
> > > > > > fix some issues
> > > > > >
> > > > > > v3 -> v4:
> > > > > > fix some issues
> > > > > >
> > > > > > v2 -> v3:
> > > > > > This patches are splited from previous big patch:
> > > > > > http://patchwork.ozlabs.org/patch/934673/
> > > > > >
> > > > > > Tonghao Zhang (4):
> > > > > > vhost: lock the vqs one by one
> > > > > > net: vhost: replace magic number of lock annotation
> > > > > > net: vhost: factor out busy polling logic to vhost_net_busy_poll()
> > > > > > net: vhost: add rx busy polling in tx path
> > > > > >
> > > > > > drivers/vhost/net.c | 108 ++++++++++++++++++++++++++++----------------------
> > > > > > drivers/vhost/vhost.c | 24 ++++-------
> > > > > > 2 files changed, 67 insertions(+), 65 deletions(-)
> > > > > >
> > > > > Hi, any progress on the new version?
> > > > >
> > > > > I plan to send a new series of packed virtqueue support of vhost. If you
> > > > > plan to send it soon, I can wait. Otherwise, I will send my series.
> > > > I rebase the codes. and find there is no improvement anymore, the
> > > > patches of makita may solve the problem. jason you may send your
> > > > patches, and I will do some research on busypoll.
> > > I see. Maybe you can try some bi-directional traffic.
> > >
> > > Btw, lots of optimizations could be done for busy polling. E.g integrating
> > > with host NAPI busy polling or a 100% busy polling vhost_net. You're welcome
> > > to work or propose new ideas.
> > >
> > > Thanks
> > It seems clear we do need adaptive polling.
>
> Yes.
>
> > The difficulty with NAPI
> > polling is it can't access guest memory easily. But maybe
> > get_user_pages on the polled memory+NAPI polling can work.
>
> You mean something like zerocopy? Looks like we can do busy polling without
> it. I mean something like https://patchwork.kernel.org/patch/8707511/.
>
> Thanks
How does this patch work? vhost_vq_avail_empty can sleep,
you are calling it within an rcu read side critical section.
That's not the only problem btw, another one is that the
CPU time spent polling isn't accounted with the VM.
> >
> > > > > Thanks
^ permalink raw reply
* Re: [libvirt] opening tap devices that are created in a container
From: Jason Baron @ 2018-07-12 3:33 UTC (permalink / raw)
To: nert; +Cc: fabiand, libvir-list, netdev, Roman Mohr, ebiederm, davem,
Laine Stump
In-Reply-To: <20180711101005.GA13392@wheatley>
On 07/11/2018 06:10 AM, nert@wheatley wrote:
> On Mon, Jul 09, 2018 at 05:00:49PM -0400, Jason Baron wrote:
>>
>>
>> On 07/08/2018 02:01 AM, Martin Kletzander wrote:
>>> On Thu, Jul 05, 2018 at 06:24:20PM +0200, Roman Mohr wrote:
>>>> On Thu, Jul 5, 2018 at 4:20 PM Jason Baron <jbaron@akamai.com> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Opening tap devices, such as macvtap, that are created in
>>>>> containers is
>>>>> problematic because the interface for opening tap devices is via
>>>>> /dev/tapNN and devtmpfs is not typically mounted inside a container as
>>>>> its not namespace aware. It is possible to do a mknod() in the
>>>>> container, once the tap devices are created, however, since the tap
>>>>> devices are created dynamically its not possible to apriori allow
>>>>> access
>>>>> to certain major/minor numbers, since we don't know what these are
>>>>> going
>>>>> to be. In addition, its desirable to not allow the mknod capability in
>>>>> containers. This behavior, I think is somewhat inconsistent with the
>>>>> tuntap driver where one can create tuntap devices inside a
>>>>> container by
>>>>> first opening /dev/net/tun and then using them by supplying the tuntap
>>>>> device name via the ioctl(TUNSETIFF). And since TUNSETIFF validates
>>>>> the
>>>>> network namespace, one is limited to opening network devices that
>>>>> belong
>>>>> to your current network namespace.
>>>>>
>>>>> Here are some options to this issue, that I wanted to get feedback
>>>>> about, and just wondering if anybody else has run into this.
>>>>>
>>>>> 1)
>>>>>
>>>>> Don't create the tap device, such as macvtap in the container.
>>>>> Instead,
>>>>> create the tap device outside of the container and then move it
>>>>> into the
>>>>> desired container network namespace. In addition, do a mknod() for the
>>>>> corresponding /dev/tapNN device from outside the container before
>>>>> doing
>>>>> chroot().
>>>>>
>>>>> This solution still doesn't allow tap devices to be created inside the
>>>>> container. Thus, in the case of kubevirt, which runs libvirtd
>>>>> inside of
>>>>> a container, it would mean changing libvirtd to open existing tap
>>>>> devices (as opposed to the current behavior of creating new ones).
>>>>> This
>>>>> would not require any kernel changes, but as mentioned seems
>>>>> inconsistent with the tuntap interface.
>>>>>
>>>>
>>>> For KubeVirt, apart from how exactly the device ends up in the
>>>> container, I
>>>> would want to pursue a way where all network preparations which require
>>>> privileges happens from a privileged process *outside* of the
>>>> container.
>>>> Like CNI solutions do it. They run outside, have privileges and then
>>>> create
>>>> devices in the right network/mount namespace or move them there. The
>>>> final
>>>> goal for KubeVirt is that our pod with the qemu process is completely
>>>> unprivileged and privileged setup happens from outside.
>>>>
>>>> As a consequence, and depending on which route Dan pursues with the
>>>> restructured libvirt, I would assume that either a privileged
>>>> libvirtd-part
>>>> outside of containers creates the devices by entering the right
>>>> namespaces,
>>>> or that libvirt in the container can consume pre-created tun/tap
>>>> devices,
>>>> like qemu.
>>>>
>>>
>>> That would be nice, but as far as I understand there will always be a
>>> need for
>>> some privileges if you want to use a tap device. It's nice that CNI
>>> does that
>>> and all the containers can run unprivileged, but that's because they do
>>> not open
>>> the tap device and they do not do any privileged operations on it. But
>>> QEMU
>>> needs to. So the only way would be passing an opened fd to the
>>> container or
>>> opening the tap device there and making the fd usable for one process in
>>> the
>>> container. Is this already supported for some type of containers in
>>> some way?
>>>
>>> Martin
>>
>> Hi,
>>
>> So another option here call it #3 is to pass open fds via unix sockets.
>> If there are privileged operations that QEMU is trying to do with the fd
>> though, how will opening it first and then passing it to an unprivileged
>> QEMU address that? Is the opener doing those operations first?
>>
>
> Sorry for the confusion, but QEMU is not doing any privileged
> operations. I got
> confused by the fact that anyone can open and do a R/W on a tap device.
> But it
> looks like that's on purpose. No capabilities are needed for opening
> /dev/net/tun and calling ioctl(TUNSETIFF) with existing name and then
> doing R/W
> operations on it. It just works.
>
> Correct me if I'm wrong, but to sum it all up, the only things that we
> need to
> figure out (which might possibly be solved by ideas in the other thread)
> are:
>
> tap:
> - Existence of /dev/net/tun
> - Having permissions to open it (0666 by default, shouldn't be a nig deal)
> - Knowing the device name
>
> macvtap:
> - Existence of /dev/tapXX
> - Having permissions to open /dev/tapXX
> - One of the following:
> - Knowing the device name (and being able to translate it using a
> netlink socket)
> - Knowing the the device index
>
Right - from the device name one can grab the device index using
SIOCGIFINDEX and then use that to access /dev/tap{device index}. Since
devtmpfs is not mounted in containers (since its not namespaced) and
mknod() is I think often not allowed, the mknod() has to happen by a
privileged process when the the container is being created. In addition,
libvirtd would need to be changed to open this existing device
(currently it only opens macvtap devices that it creates). This is
option #1.
> The rest should be an implementation detail.
>
> Am I right? Did I miss anything?
I don't think so, I'm interested if option #1 is workable or if there is
interest in option #2 which is to do something like /dev/net/tun in the
kernel for macvtap devices.
Since as Daniel pointed out something like option #1 is going to be
needed anyways to work on older kernels it seems like the best option?
Thanks,
-Jason
^ permalink raw reply
* Re: [PATCH bpf-next v5 0/3] bpf: btf: print bpftool map data with btf
From: Jakub Kicinski @ 2018-07-12 3:30 UTC (permalink / raw)
To: Okash Khawaja
Cc: Daniel Borkmann, Martin KaFai Lau, Alexei Starovoitov,
Yonghong Song, Quentin Monnet, David S. Miller, netdev,
kernel-team, linux-kernel
In-Reply-To: <20180712030803.875913594@fb.com>
On Wed, 11 Jul 2018 20:08:03 -0700, Okash Khawaja wrote:
> Hi,
>
> Here are the changes from v4:
>
> patch 2:
>
> - sort headers in btf_dumper.c
> - remove extra parentheses
> - include asm/byteorder.h
> - compile error when big and small endian bitfields macro undefined
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Thanks!
^ permalink raw reply
* Re: [PATCH net-next v5 0/4] net: vhost: improve performance when enable busyloop
From: Jason Wang @ 2018-07-12 3:26 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Linux Kernel Network Developers, virtualization
In-Reply-To: <20180711145421-mutt-send-email-mst@kernel.org>
On 2018年07月11日 19:59, Michael S. Tsirkin wrote:
> On Wed, Jul 11, 2018 at 01:12:59PM +0800, Jason Wang wrote:
>>
>> On 2018年07月11日 11:49, Tonghao Zhang wrote:
>>> On Wed, Jul 11, 2018 at 10:56 AM Jason Wang <jasowang@redhat.com> wrote:
>>>>
>>>> On 2018年07月04日 12:31, xiangxia.m.yue@gmail.com wrote:
>>>>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>>>>>
>>>>> This patches improve the guest receive and transmit performance.
>>>>> On the handle_tx side, we poll the sock receive queue at the same time.
>>>>> handle_rx do that in the same way.
>>>>>
>>>>> For more performance report, see patch 4.
>>>>>
>>>>> v4 -> v5:
>>>>> fix some issues
>>>>>
>>>>> v3 -> v4:
>>>>> fix some issues
>>>>>
>>>>> v2 -> v3:
>>>>> This patches are splited from previous big patch:
>>>>> http://patchwork.ozlabs.org/patch/934673/
>>>>>
>>>>> Tonghao Zhang (4):
>>>>> vhost: lock the vqs one by one
>>>>> net: vhost: replace magic number of lock annotation
>>>>> net: vhost: factor out busy polling logic to vhost_net_busy_poll()
>>>>> net: vhost: add rx busy polling in tx path
>>>>>
>>>>> drivers/vhost/net.c | 108 ++++++++++++++++++++++++++++----------------------
>>>>> drivers/vhost/vhost.c | 24 ++++-------
>>>>> 2 files changed, 67 insertions(+), 65 deletions(-)
>>>>>
>>>> Hi, any progress on the new version?
>>>>
>>>> I plan to send a new series of packed virtqueue support of vhost. If you
>>>> plan to send it soon, I can wait. Otherwise, I will send my series.
>>> I rebase the codes. and find there is no improvement anymore, the
>>> patches of makita may solve the problem. jason you may send your
>>> patches, and I will do some research on busypoll.
>> I see. Maybe you can try some bi-directional traffic.
>>
>> Btw, lots of optimizations could be done for busy polling. E.g integrating
>> with host NAPI busy polling or a 100% busy polling vhost_net. You're welcome
>> to work or propose new ideas.
>>
>> Thanks
> It seems clear we do need adaptive polling.
Yes.
> The difficulty with NAPI
> polling is it can't access guest memory easily. But maybe
> get_user_pages on the polled memory+NAPI polling can work.
You mean something like zerocopy? Looks like we can do busy polling
without it. I mean something like
https://patchwork.kernel.org/patch/8707511/.
Thanks
>
>>>> Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* 答复: [PATCH] net: convert gro_count to bitmask
From: Li,Rongqing @ 2018-07-12 2:31 UTC (permalink / raw)
To: Eric Dumazet, netdev@vger.kernel.org
In-Reply-To: <56c49677-8b55-b337-7f5a-c297a11c82b0@gmail.com>
> -----邮件原件-----
> 发件人: Eric Dumazet [mailto:eric.dumazet@gmail.com]
> 发送时间: 2018年7月11日 19:32
> 收件人: Li,Rongqing <lirongqing@baidu.com>; netdev@vger.kernel.org
> 主题: Re: [PATCH] net: convert gro_count to bitmask
>
>
>
> On 07/11/2018 02:15 AM, Li RongQing wrote:
> > gro_hash size is 192 bytes, and uses 3 cache lines, if there is few
> > flows, gro_hash may be not fully used, so it is unnecessary to iterate
> > all gro_hash in napi_gro_flush(), to occupy unnecessary cacheline.
> >
> > convert gro_count to a bitmask, and rename it as gro_bitmask, each bit
> > represents a element of gro_hash, only flush a gro_hash element if the
> > related bit is set, to speed up napi_gro_flush().
> >
> > and update gro_bitmask only if it will be changed, to reduce cache
> > update
> >
> > Suggested-by: Eric Dumazet <edumazet@google.com>
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > ---
> > include/linux/netdevice.h | 2 +-
> > net/core/dev.c | 35 +++++++++++++++++++++++------------
> > 2 files changed, 24 insertions(+), 13 deletions(-)
> >
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index b683971e500d..df49b36ef378 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -322,7 +322,7 @@ struct napi_struct {
> >
> > unsigned long state;
> > int weight;
> > - unsigned int gro_count;
> > + unsigned long gro_bitmask;
> > int (*poll)(struct napi_struct *, int);
> > #ifdef CONFIG_NETPOLL
> > int poll_owner;
> > diff --git a/net/core/dev.c b/net/core/dev.c index
> > d13cddcac41f..a08dbdd217a6 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -5171,9 +5171,11 @@ static void __napi_gro_flush_chain(struct
> napi_struct *napi, u32 index,
> > return;
> > list_del_init(&skb->list);
> > napi_gro_complete(skb);
> > - napi->gro_count--;
> > napi->gro_hash[index].count--;
> > }
> > +
> > + if (!napi->gro_hash[index].count)
> > + clear_bit(index, &napi->gro_bitmask);
>
> I suggest you not add an atomic operation here.
>
> Current cpu owns this NAPI after all.
>
> Same remark for the whole patch.
>
> -> __clear_bit(), __set_bit() and similar operators
>
> Ideally you should provide TCP_RR number with busy polling enabled, to
> eventually catch regressions.
>
I will change and do the test
Thank you.
-RongQing
> Thanks.
^ permalink raw reply
* 答复: [PATCH] net: convert gro_count to bitmask
From: Li,Rongqing @ 2018-07-12 3:03 UTC (permalink / raw)
To: David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <20180711.194920.590731873210311426.davem@davemloft.net>
> -----邮件原件-----
> 发件人: David Miller [mailto:davem@davemloft.net]
> 发送时间: 2018年7月12日 10:49
> 收件人: Li,Rongqing <lirongqing@baidu.com>
> 抄送: netdev@vger.kernel.org
> 主题: Re: [PATCH] net: convert gro_count to bitmask
>
> From: Li RongQing <lirongqing@baidu.com>
> Date: Wed, 11 Jul 2018 17:15:53 +0800
>
> > + clear_bit(index, &napi->gro_bitmask);
>
> Please don't use atomics here, at least use __clear_bit().
>
Thanks, this is same as Eric's suggestion.
> This is why I did the operations by hand in my version of the patch.
> Also, if you are going to preempt my patch, at least retain the comment I
> added around the GRO_HASH_BUCKETS definitions which warns the reader
> about the limit.
>
I add BUILD_BUG_ON in netdev_init, so I think we need not to add comment
@@ -9151,6 +9159,9 @@ static struct hlist_head * __net_init netdev_create_hash(void)
/* Initialize per network namespace state */ static int __net_init netdev_init(struct net *net) {
+ BUILD_BUG_ON(GRO_HASH_BUCKETS >
+ FIELD_SIZEOF(struct napi_struct, gro_bitmask));
+
-RongQing
> Thanks.
^ permalink raw reply
* [PATCH bpf-next v5 3/3] bpf: btf: print map dump and lookup with btf info
From: Okash Khawaja @ 2018-07-12 3:08 UTC (permalink / raw)
To: Daniel Borkmann, Martin KaFai Lau, Alexei Starovoitov,
Yonghong Song, Quentin Monnet, Jakub Kicinski, David S. Miller
Cc: netdev, kernel-team, linux-kernel
In-Reply-To: <20180712030803.875913594@fb.com>
[-- Attachment #1: 03-json-print-btf-info-for-map --]
[-- Type: text/plain, Size: 8313 bytes --]
This patch augments the output of bpftool's map dump and map lookup
commands to print data along side btf info, if the correspondin btf
info is available. The outputs for each of map dump and map lookup
commands are augmented in two ways:
1. when neither of -j and -p are supplied, btf-ful map data is printed
whose aim is human readability. This means no commitments for json- or
backward- compatibility.
2. when either -j or -p are supplied, a new json object named
"formatted" is added for each key-value pair. This object contains the
same data as the key-value pair, but with btf info. "formatted" object
promises json- and backward- compatibility. Below is a sample output.
$ bpftool map dump -p id 8
[{
"key": ["0x0f","0x00","0x00","0x00"
],
"value": ["0x03", "0x00", "0x00", "0x00", ...
],
"formatted": {
"key": 15,
"value": {
"int_field": 3,
...
}
}
}
]
This patch calls btf_dumper introduced in previous patch to accomplish
the above. Indeed, btf-ful info is only displayed if btf data for the
given map is available. Otherwise existing output is displayed as-is.
Signed-off-by: Okash Khawaja <osk@fb.com>
---
tools/bpf/bpftool/map.c | 217 ++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 201 insertions(+), 16 deletions(-)
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -34,6 +34,7 @@
#include <assert.h>
#include <errno.h>
#include <fcntl.h>
+#include <linux/err.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
@@ -44,6 +45,8 @@
#include <bpf.h>
+#include "btf.h"
+#include "json_writer.h"
#include "main.h"
static const char * const map_type_name[] = {
@@ -148,8 +151,109 @@ int map_parse_fd_and_info(int *argc, cha
return fd;
}
+static int do_dump_btf(const struct btf_dumper *d,
+ struct bpf_map_info *map_info, void *key,
+ void *value)
+{
+ int ret;
+
+ /* start of key-value pair */
+ jsonw_start_object(d->jw);
+
+ jsonw_name(d->jw, "key");
+
+ ret = btf_dumper_type(d, map_info->btf_key_type_id, key);
+ if (ret)
+ goto err_end_obj;
+
+ jsonw_name(d->jw, "value");
+
+ ret = btf_dumper_type(d, map_info->btf_value_type_id, value);
+
+err_end_obj:
+ /* end of key-value pair */
+ jsonw_end_object(d->jw);
+
+ return ret;
+}
+
+static int get_btf(struct bpf_map_info *map_info, struct btf **btf)
+{
+ struct bpf_btf_info btf_info = { 0 };
+ __u32 len = sizeof(btf_info);
+ __u32 last_size;
+ int btf_fd;
+ void *ptr;
+ int err;
+
+ err = 0;
+ *btf = NULL;
+ btf_fd = bpf_btf_get_fd_by_id(map_info->btf_id);
+ if (btf_fd < 0)
+ return 0;
+
+ /* we won't know btf_size until we call bpf_obj_get_info_by_fd(). so
+ * let's start with a sane default - 4KiB here - and resize it only if
+ * bpf_obj_get_info_by_fd() needs a bigger buffer.
+ */
+ btf_info.btf_size = 4096;
+ last_size = btf_info.btf_size;
+ ptr = malloc(last_size);
+ if (!ptr) {
+ err = -ENOMEM;
+ goto exit_free;
+ }
+
+ bzero(ptr, last_size);
+ btf_info.btf = ptr_to_u64(ptr);
+ err = bpf_obj_get_info_by_fd(btf_fd, &btf_info, &len);
+
+ if (!err && btf_info.btf_size > last_size) {
+ void *temp_ptr;
+
+ last_size = btf_info.btf_size;
+ temp_ptr = realloc(ptr, last_size);
+ if (!temp_ptr) {
+ err = -ENOMEM;
+ goto exit_free;
+ }
+ ptr = temp_ptr;
+ bzero(ptr, last_size);
+ btf_info.btf = ptr_to_u64(ptr);
+ err = bpf_obj_get_info_by_fd(btf_fd, &btf_info, &len);
+ }
+
+ if (err || btf_info.btf_size > last_size) {
+ err = errno;
+ goto exit_free;
+ }
+
+ *btf = btf__new((__u8 *)btf_info.btf, btf_info.btf_size, NULL);
+ if (IS_ERR(*btf)) {
+ err = PTR_ERR(btf);
+ *btf = NULL;
+ }
+
+exit_free:
+ close(btf_fd);
+ free(ptr);
+
+ return err;
+}
+
+static json_writer_t *get_btf_writer(void)
+{
+ json_writer_t *jw = jsonw_new(stdout);
+
+ if (!jw)
+ return NULL;
+ jsonw_pretty(jw, true);
+
+ return jw;
+}
+
static void print_entry_json(struct bpf_map_info *info, unsigned char *key,
- unsigned char *value)
+ unsigned char *value, struct btf *btf)
{
jsonw_start_object(json_wtr);
@@ -158,6 +262,16 @@ static void print_entry_json(struct bpf_
print_hex_data_json(key, info->key_size);
jsonw_name(json_wtr, "value");
print_hex_data_json(value, info->value_size);
+ if (btf) {
+ struct btf_dumper d = {
+ .btf = btf,
+ .jw = json_wtr,
+ .is_plain_text = false,
+ };
+
+ jsonw_name(json_wtr, "formatted");
+ do_dump_btf(&d, info, key, value);
+ }
} else {
unsigned int i, n;
@@ -508,10 +622,12 @@ static int do_show(int argc, char **argv
static int do_dump(int argc, char **argv)
{
+ struct bpf_map_info info = {};
void *key, *value, *prev_key;
unsigned int num_elems = 0;
- struct bpf_map_info info = {};
__u32 len = sizeof(info);
+ json_writer_t *btf_wtr;
+ struct btf *btf = NULL;
int err;
int fd;
@@ -537,8 +653,27 @@ static int do_dump(int argc, char **argv
}
prev_key = NULL;
+
+ err = get_btf(&info, &btf);
+ if (err) {
+ p_err("failed to get btf");
+ goto exit_free;
+ }
+
if (json_output)
jsonw_start_array(json_wtr);
+ else
+ if (btf) {
+ btf_wtr = get_btf_writer();
+ if (!btf_wtr) {
+ p_info("failed to create json writer for btf. falling back to plain output");
+ btf__free(btf);
+ btf = NULL;
+ } else {
+ jsonw_start_array(btf_wtr);
+ }
+ }
+
while (true) {
err = bpf_map_get_next_key(fd, prev_key, key);
if (err) {
@@ -549,9 +684,19 @@ static int do_dump(int argc, char **argv
if (!bpf_map_lookup_elem(fd, key, value)) {
if (json_output)
- print_entry_json(&info, key, value);
+ print_entry_json(&info, key, value, btf);
else
- print_entry_plain(&info, key, value);
+ if (btf) {
+ struct btf_dumper d = {
+ .btf = btf,
+ .jw = btf_wtr,
+ .is_plain_text = true,
+ };
+
+ do_dump_btf(&d, &info, key, value);
+ } else {
+ print_entry_plain(&info, key, value);
+ }
} else {
if (json_output) {
jsonw_name(json_wtr, "key");
@@ -574,14 +719,19 @@ static int do_dump(int argc, char **argv
if (json_output)
jsonw_end_array(json_wtr);
- else
+ else if (btf) {
+ jsonw_end_array(btf_wtr);
+ jsonw_destroy(&btf_wtr);
+ } else {
printf("Found %u element%s\n", num_elems,
num_elems != 1 ? "s" : "");
+ }
exit_free:
free(key);
free(value);
close(fd);
+ btf__free(btf);
return err;
}
@@ -637,6 +787,8 @@ static int do_lookup(int argc, char **ar
{
struct bpf_map_info info = {};
__u32 len = sizeof(info);
+ json_writer_t *btf_wtr;
+ struct btf *btf = NULL;
void *key, *value;
int err;
int fd;
@@ -661,27 +813,60 @@ static int do_lookup(int argc, char **ar
goto exit_free;
err = bpf_map_lookup_elem(fd, key, value);
- if (!err) {
- if (json_output)
- print_entry_json(&info, key, value);
- else
+ if (err) {
+ if (errno == ENOENT) {
+ if (json_output) {
+ jsonw_null(json_wtr);
+ } else {
+ printf("key:\n");
+ fprint_hex(stdout, key, info.key_size, " ");
+ printf("\n\nNot found\n");
+ }
+ } else {
+ p_err("lookup failed: %s", strerror(errno));
+ }
+
+ goto exit_free;
+ }
+
+ /* here means bpf_map_lookup_elem() succeeded */
+ err = get_btf(&info, &btf);
+ if (err) {
+ p_err("failed to get btf");
+ goto exit_free;
+ }
+
+ if (json_output) {
+ print_entry_json(&info, key, value, btf);
+ } else if (btf) {
+ /* if here json_wtr wouldn't have been initialised,
+ * so let's create separate writer for btf
+ */
+ btf_wtr = get_btf_writer();
+ if (!btf_wtr) {
+ p_info("failed to create json writer for btf. falling back to plain output");
+ btf__free(btf);
+ btf = NULL;
print_entry_plain(&info, key, value);
- } else if (errno == ENOENT) {
- if (json_output) {
- jsonw_null(json_wtr);
} else {
- printf("key:\n");
- fprint_hex(stdout, key, info.key_size, " ");
- printf("\n\nNot found\n");
+ struct btf_dumper d = {
+ .btf = btf,
+ .jw = btf_wtr,
+ .is_plain_text = true,
+ };
+
+ do_dump_btf(&d, &info, key, value);
+ jsonw_destroy(&btf_wtr);
}
} else {
- p_err("lookup failed: %s", strerror(errno));
+ print_entry_plain(&info, key, value);
}
exit_free:
free(key);
free(value);
close(fd);
+ btf__free(btf);
return err;
}
^ permalink raw reply
* [PATCH bpf-next v5 2/3] bpf: btf: add btf print functionality
From: Okash Khawaja @ 2018-07-12 3:08 UTC (permalink / raw)
To: Daniel Borkmann, Martin KaFai Lau, Alexei Starovoitov,
Yonghong Song, Quentin Monnet, Jakub Kicinski, David S. Miller
Cc: netdev, kernel-team, linux-kernel
In-Reply-To: <20180712030803.875913594@fb.com>
[-- Attachment #1: 02-add-btf-dump-map.patch --]
[-- Type: text/plain, Size: 9540 bytes --]
This consumes functionality exported in the previous patch. It does the
main job of printing with BTF data. This is used in the following patch
to provide a more readable output of a map's dump. It relies on
json_writer to do json printing. Below is sample output where map keys
are ints and values are of type struct A:
typedef int int_type;
enum E {
E0,
E1,
};
struct B {
int x;
int y;
};
struct A {
int m;
unsigned long long n;
char o;
int p[8];
int q[4][8];
enum E r;
void *s;
struct B t;
const int u;
int_type v;
unsigned int w1: 3;
unsigned int w2: 3;
};
$ sudo bpftool map dump id 14
[{
"key": 0,
"value": {
"m": 1,
"n": 2,
"o": "c",
"p": [15,16,17,18,15,16,17,18
],
"q": [[25,26,27,28,25,26,27,28
],[35,36,37,38,35,36,37,38
],[45,46,47,48,45,46,47,48
],[55,56,57,58,55,56,57,58
]
],
"r": 1,
"s": 0x7ffd80531cf8,
"t": {
"x": 5,
"y": 10
},
"u": 100,
"v": 20,
"w1": 0x7,
"w2": 0x3
}
}
]
This patch uses json's {} and [] to imply struct/union and array. More
explicit information can be added later. For example, a command line
option can be introduced to print whether a key or value is struct
or union, name of a struct etc. This will however come at the expense
of duplicating info when, for example, printing an array of structs.
enums are printed as ints without their names.
Signed-off-by: Okash Khawaja <osk@fb.com>
---
tools/bpf/bpftool/btf_dumper.c | 251 +++++++++++++++++++++++++++++++++++++++++
tools/bpf/bpftool/main.h | 15 ++
2 files changed, 266 insertions(+)
--- /dev/null
+++ b/tools/bpf/bpftool/btf_dumper.c
@@ -0,0 +1,251 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2018 Facebook */
+
+#include <ctype.h>
+#include <stdio.h> /* for (FILE *) used by json_writer */
+#include <string.h>
+#include <asm/byteorder.h>
+#include <linux/bitops.h>
+#include <linux/btf.h>
+#include <linux/err.h>
+
+#include "btf.h"
+#include "json_writer.h"
+#include "main.h"
+
+#define BITS_PER_BYTE_MASK (BITS_PER_BYTE - 1)
+#define BITS_PER_BYTE_MASKED(bits) ((bits) & BITS_PER_BYTE_MASK)
+#define BITS_ROUNDDOWN_BYTES(bits) ((bits) >> 3)
+#define BITS_ROUNDUP_BYTES(bits) \
+ (BITS_ROUNDDOWN_BYTES(bits) + !!BITS_PER_BYTE_MASKED(bits))
+
+static int btf_dumper_do_type(const struct btf_dumper *d, __u32 type_id,
+ __u8 bit_offset, const void *data);
+
+static void btf_dumper_ptr(const void *data, json_writer_t *jw,
+ bool is_plain_text)
+{
+ if (is_plain_text)
+ jsonw_printf(jw, "%p", *(unsigned long *)data);
+ else
+ jsonw_printf(jw, "%u", *(unsigned long *)data);
+}
+
+static int btf_dumper_modifier(const struct btf_dumper *d, __u32 type_id,
+ const void *data)
+{
+ int actual_type_id;
+
+ actual_type_id = btf__resolve_type(d->btf, type_id);
+ if (actual_type_id < 0)
+ return actual_type_id;
+
+ return btf_dumper_do_type(d, actual_type_id, 0, data);
+}
+
+static void btf_dumper_enum(const void *data, json_writer_t *jw)
+{
+ jsonw_printf(jw, "%d", *(int *)data);
+}
+
+static int btf_dumper_array(const struct btf_dumper *d, __u32 type_id,
+ const void *data)
+{
+ const struct btf_type *t = btf__type_by_id(d->btf, type_id);
+ struct btf_array *arr = (struct btf_array *)(t + 1);
+ long long elem_size;
+ int ret = 0;
+ __u32 i;
+
+ elem_size = btf__resolve_size(d->btf, arr->type);
+ if (elem_size < 0)
+ return elem_size;
+
+ jsonw_start_array(d->jw);
+ for (i = 0; i < arr->nelems; i++) {
+ ret = btf_dumper_do_type(d, arr->type, 0,
+ data + i * elem_size);
+ if (ret)
+ break;
+ }
+
+ jsonw_end_array(d->jw);
+ return ret;
+}
+
+static void btf_dumper_int_bits(__u32 int_type, __u8 bit_offset,
+ const void *data, json_writer_t *jw,
+ bool is_plain_text)
+{
+ int left_shift_bits, right_shift_bits;
+ int nr_bits = BTF_INT_BITS(int_type);
+ int total_bits_offset;
+ int bytes_to_copy;
+ int bits_to_copy;
+ __u64 print_num;
+
+ total_bits_offset = bit_offset + BTF_INT_OFFSET(int_type);
+ data += BITS_ROUNDDOWN_BYTES(total_bits_offset);
+ bit_offset = BITS_PER_BYTE_MASKED(total_bits_offset);
+ bits_to_copy = bit_offset + nr_bits;
+ bytes_to_copy = BITS_ROUNDUP_BYTES(bits_to_copy);
+
+ print_num = 0;
+ memcpy(&print_num, data, bytes_to_copy);
+#if defined(__BIG_ENDIAN_BITFIELD)
+ left_shift_bits = bit_offset;
+#elif defined(__LITTLE_ENDIAN_BITFIELD)
+ left_shift_bits = 64 - bits_to_copy;
+#else
+#error neither big nor little endian
+#endif
+ right_shift_bits = 64 - nr_bits;
+
+ print_num <<= left_shift_bits;
+ print_num >>= right_shift_bits;
+ if (is_plain_text)
+ jsonw_printf(jw, "0x%llx", print_num);
+ else
+ jsonw_printf(jw, "%llu", print_num);
+}
+
+static int btf_dumper_int(const struct btf_type *t, __u8 bit_offset,
+ const void *data, json_writer_t *jw,
+ bool is_plain_text)
+{
+ __u32 *int_type;
+ __u32 nr_bits;
+
+ int_type = (__u32 *)(t + 1);
+ nr_bits = BTF_INT_BITS(*int_type);
+ /* if this is bit field */
+ if (bit_offset || BTF_INT_OFFSET(*int_type) ||
+ BITS_PER_BYTE_MASKED(nr_bits)) {
+ btf_dumper_int_bits(*int_type, bit_offset, data, jw,
+ is_plain_text);
+ return 0;
+ }
+
+ switch (BTF_INT_ENCODING(*int_type)) {
+ case 0:
+ if (BTF_INT_BITS(*int_type) == 64)
+ jsonw_printf(jw, "%lu", *(__u64 *)data);
+ else if (BTF_INT_BITS(*int_type) == 32)
+ jsonw_printf(jw, "%u", *(__u32 *)data);
+ else if (BTF_INT_BITS(*int_type) == 16)
+ jsonw_printf(jw, "%hu", *(__u16 *)data);
+ else if (BTF_INT_BITS(*int_type) == 8)
+ jsonw_printf(jw, "%hhu", *(__u8 *)data);
+ else
+ btf_dumper_int_bits(*int_type, bit_offset, data, jw,
+ is_plain_text);
+ break;
+ case BTF_INT_SIGNED:
+ if (BTF_INT_BITS(*int_type) == 64)
+ jsonw_printf(jw, "%ld", *(long long *)data);
+ else if (BTF_INT_BITS(*int_type) == 32)
+ jsonw_printf(jw, "%d", *(int *)data);
+ else if (BTF_INT_BITS(*int_type) == 16)
+ jsonw_printf(jw, "%hd", *(short *)data);
+ else if (BTF_INT_BITS(*int_type) == 8)
+ jsonw_printf(jw, "%hhd", *(char *)data);
+ else
+ btf_dumper_int_bits(*int_type, bit_offset, data, jw,
+ is_plain_text);
+ break;
+ case BTF_INT_CHAR:
+ if (isprint(*(char *)data))
+ jsonw_printf(jw, "\"%c\"", *(char *)data);
+ else
+ if (is_plain_text)
+ jsonw_printf(jw, "0x%hhx", *(char *)data);
+ else
+ jsonw_printf(jw, "\"\\u00%02hhx\"",
+ *(char *)data);
+ break;
+ case BTF_INT_BOOL:
+ jsonw_bool(jw, *(int *)data);
+ break;
+ default:
+ /* shouldn't happen */
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int btf_dumper_struct(const struct btf_dumper *d, __u32 type_id,
+ const void *data)
+{
+ const struct btf_type *t;
+ struct btf_member *m;
+ const void *data_off;
+ int ret = 0;
+ int i, vlen;
+
+ t = btf__type_by_id(d->btf, type_id);
+ if (!t)
+ return -EINVAL;
+
+ vlen = BTF_INFO_VLEN(t->info);
+ jsonw_start_object(d->jw);
+ m = (struct btf_member *)(t + 1);
+
+ for (i = 0; i < vlen; i++) {
+ data_off = data + BITS_ROUNDDOWN_BYTES(m[i].offset);
+ jsonw_name(d->jw, btf__name_by_offset(d->btf, m[i].name_off));
+ ret = btf_dumper_do_type(d, m[i].type,
+ BITS_PER_BYTE_MASKED(m[i].offset),
+ data_off);
+ if (ret)
+ break;
+ }
+
+ jsonw_end_object(d->jw);
+
+ return ret;
+}
+
+static int btf_dumper_do_type(const struct btf_dumper *d, __u32 type_id,
+ __u8 bit_offset, const void *data)
+{
+ const struct btf_type *t = btf__type_by_id(d->btf, type_id);
+
+ switch (BTF_INFO_KIND(t->info)) {
+ case BTF_KIND_INT:
+ return btf_dumper_int(t, bit_offset, data, d->jw,
+ d->is_plain_text);
+ case BTF_KIND_STRUCT:
+ case BTF_KIND_UNION:
+ return btf_dumper_struct(d, type_id, data);
+ case BTF_KIND_ARRAY:
+ return btf_dumper_array(d, type_id, data);
+ case BTF_KIND_ENUM:
+ btf_dumper_enum(data, d->jw);
+ return 0;
+ case BTF_KIND_PTR:
+ btf_dumper_ptr(data, d->jw, d->is_plain_text);
+ return 0;
+ case BTF_KIND_UNKN:
+ jsonw_printf(d->jw, "(unknown)");
+ return 0;
+ case BTF_KIND_FWD:
+ /* map key or value can't be forward */
+ jsonw_printf(d->jw, "(fwd-kind-invalid)");
+ return -EINVAL;
+ case BTF_KIND_TYPEDEF:
+ case BTF_KIND_VOLATILE:
+ case BTF_KIND_CONST:
+ case BTF_KIND_RESTRICT:
+ return btf_dumper_modifier(d, type_id, data);
+ default:
+ jsonw_printf(d->jw, "(unsupported-kind");
+ return -EINVAL;
+ }
+}
+
+int btf_dumper_type(const struct btf_dumper *d, __u32 type_id,
+ const void *data)
+{
+ return btf_dumper_do_type(d, type_id, 0, data);
+}
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -131,4 +131,19 @@ unsigned int get_page_size(void);
unsigned int get_possible_cpus(void);
const char *ifindex_to_bfd_name_ns(__u32 ifindex, __u64 ns_dev, __u64 ns_ino);
+struct btf_dumper {
+ const struct btf *btf;
+ json_writer_t *jw;
+ bool is_plain_text;
+};
+
+/* btf_dumper_type - print data along with type information
+ * @d: an instance containing context for dumping types
+ * @type_id: index in btf->types array. this points to the type to be dumped
+ * @data: pointer the actual data, i.e. the values to be printed
+ *
+ * Returns zero on success and negative error code otherwise
+ */
+int btf_dumper_type(const struct btf_dumper *d, __u32 type_id,
+ const void *data);
#endif
^ permalink raw reply
* [PATCH bpf-next v5 0/3] bpf: btf: print bpftool map data with btf
From: Okash Khawaja @ 2018-07-12 3:08 UTC (permalink / raw)
To: Daniel Borkmann, Martin KaFai Lau, Alexei Starovoitov,
Yonghong Song, Quentin Monnet, Jakub Kicinski, David S. Miller
Cc: netdev, kernel-team, linux-kernel
Hi,
Here are the changes from v4:
patch 2:
- sort headers in btf_dumper.c
- remove extra parentheses
- include asm/byteorder.h
- compile error when big and small endian bitfields macro undefined
Thanks,
Okash
^ permalink raw reply
* Re: [PATCH] net: convert gro_count to bitmask
From: David Miller @ 2018-07-12 2:49 UTC (permalink / raw)
To: lirongqing; +Cc: netdev
In-Reply-To: <1531300553-21413-1-git-send-email-lirongqing@baidu.com>
From: Li RongQing <lirongqing@baidu.com>
Date: Wed, 11 Jul 2018 17:15:53 +0800
> + clear_bit(index, &napi->gro_bitmask);
Please don't use atomics here, at least use __clear_bit().
This is why I did the operations by hand in my version of the patch.
Also, if you are going to preempt my patch, at least retain the
comment I added around the GRO_HASH_BUCKETS definitions which warns
the reader about the limit.
Thanks.
^ permalink raw reply
* Re: Bug report: epoll can fail to report EPOLLOUT when unix datagram socket peer is closed
From: Jason Baron @ 2018-07-12 2:46 UTC (permalink / raw)
To: Ian Lance Taylor, netdev
In-Reply-To: <CAOyqgcXzie6xRSEfVvtSS=uSkEaBxP-LjCcUxXLoqCjbvTO4yg@mail.gmail.com>
On 06/26/2018 10:18 AM, Ian Lance Taylor wrote:
> I'm reporting what appears to be a bug in the Linux kernel's epoll
> support. It seems that epoll appears to sometimes fail to report an
> EPOLLOUT event when the other side of an AF_UNIX/SOCK_DGRAM socket is
> closed. This bug report started as a Go program reported at
> https://golang.org/issue/23604. I've written a C program that
> demonstrates the same symptoms, at
> https://github.com/golang/go/issues/23604#issuecomment-398945027 .
>
> The C program sets up an AF_UNIX/SOCK_DGRAM server and serveral
> identical clients, all running in non-blocking mode. All the
> non-blocking sockets are added to epoll, using EPOLLET. The server
> periodically closes and reopens its socket. The clients look for
> ECONNREFUSED errors on their write calls, and close and reopen their
> sockets when they see one.
>
> The clients will sometimes fill up their buffer and block with EAGAIN.
> At that point they expect the poller to return an EPOLLOUT event to
> tell them when they are ready to write again. The expectation is that
> either the server will read data, freeing up buffer space, or will
> close the socket, which should cause the sending packets to be
> discarded, freeing up buffer space. Generally the EPOLLOUT event
> happens. But sometimes, the poller never returns such an event, and
> the client stalls. In the test program this is reported as a client
> that waits more than 20 seconds to be told to continue.
>
> A similar bug report was made, with few details, at
> https://stackoverflow.com/questions/38441059/edge-triggered-epoll-for-unix-domain-socket
> .
>
> I've tested the program and seen the failure on kernel 4.9.0-6-amd64.
> A colleague has tested the program and seen the failure on
> 4.18.0-smp-DEV #3 SMP @1529531011 x86_64 GNU/Linux.
>
> If there is a better way for me to report this, please let me know.
>
> Thanks for your attention.
>
> Ian
>
Hi,
Thanks for the report and the test program. The patch below seems to
have cured the reproducer for me. But perhaps you can confirm?
Thanks,
-Jason
[PATCH] af_unix: ensure POLLOUT on remote close() for connected dgram socket
Applictions use ECONNREFUSED as returned from write() in order to
determine that a socket should be closed. When using connected dgram
unix sockets in a poll/write loop, this relies on POLLOUT being
signaled when the remote end closes. However, due to a race POLLOUT
can be missed when the remote closes:
thread 1 (client) thread 2 (server)
connect() to server
write() returns -EAGAIN
unix_dgram_poll()
-> unix_recvq_full() is true
close()
->unix_release_sock()
->wake_up_interruptible_all()
unix_dgram_poll() (due to the
wake_up_interruptible_all)
-> unix_recvq_full() still is true
->free all skbs
Now thread 1 is stuck and will not receive anymore wakeups. In this
case, when thread 1 gets the -EAGAIN, it has not queued any skbs
otherwise the 'free all skbs' step would in fact cause a wakeup and
a POLLOUT return. So the race here is probably fairly rare because
it means there are no skbs that thread 1 queued and that thread 1
schedules before the 'free all skbs' step. Nevertheless, this has
been observed in the wild via syslog.
The proposed fix is to move the wake_up_interruptible_all() call
after the 'free all skbs' step.
Signed-off-by: Jason Baron <jbaron@akamai.com>
---
net/unix/af_unix.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index e5473c0..de242cf 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -529,8 +529,6 @@ static void unix_release_sock(struct sock *sk, int
embrion)
sk->sk_state = TCP_CLOSE;
unix_state_unlock(sk);
- wake_up_interruptible_all(&u->peer_wait);
-
skpair = unix_peer(sk);
if (skpair != NULL) {
@@ -560,6 +558,9 @@ static void unix_release_sock(struct sock *sk, int
embrion)
kfree_skb(skb);
}
+ /* after freeing skbs to make sure POLLOUT triggers */
+ wake_up_interruptible_all(&u->peer_wait);
+
if (path.dentry)
path_put(&path);
--
2.7.4
^ permalink raw reply related
* 答复: [PATCH] net: convert gro_count to bitmask
From: Li,Rongqing @ 2018-07-12 2:31 UTC (permalink / raw)
To: Stefano Brivio; +Cc: netdev@vger.kernel.org, Eric Dumazet
In-Reply-To: <20180711125133.60528540@epycfail>
> -----邮件原件-----
> 发件人: Stefano Brivio [mailto:sbrivio@redhat.com]
> 发送时间: 2018年7月11日 18:52
> 收件人: Li,Rongqing <lirongqing@baidu.com>
> 抄送: netdev@vger.kernel.org; Eric Dumazet <edumazet@google.com>
> 主题: Re: [PATCH] net: convert gro_count to bitmask
>
> On Wed, 11 Jul 2018 17:15:53 +0800
> Li RongQing <lirongqing@baidu.com> wrote:
>
> > @@ -5380,6 +5382,12 @@ static enum gro_result dev_gro_receive(struct
> napi_struct *napi, struct sk_buff
> > if (grow > 0)
> > gro_pull_from_frag0(skb, grow);
> > ok:
> > + if (napi->gro_hash[hash].count)
> > + if (!test_bit(hash, &napi->gro_bitmask))
> > + set_bit(hash, &napi->gro_bitmask);
> > + else if (test_bit(hash, &napi->gro_bitmask))
> > + clear_bit(hash, &napi->gro_bitmask);
>
> This might not do what you want.
>
> --
could you show detail ?
-RongQing
> Stefano
^ permalink raw reply
* Re: [PATCH net-next] net: sched: fix unprotected access to rcu cookie pointer
From: Marcelo Ricardo Leitner @ 2018-07-12 2:29 UTC (permalink / raw)
To: Vlad Buslov; +Cc: netdev, davem, jhs, xiyou.wangcong, jiri
In-Reply-To: <vbfpnzwqguh.fsf@reg-r-vrt-018-180.mtr.labs.mlnx>
On Mon, Jul 09, 2018 at 11:44:38PM +0300, Vlad Buslov wrote:
>
> On Mon 09 Jul 2018 at 20:34, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote:
> > On Mon, Jul 09, 2018 at 08:26:47PM +0300, Vlad Buslov wrote:
> >> Fix action attribute size calculation function to take rcu read lock and
> >> access act_cookie pointer with rcu dereference.
> >>
> >> Fixes: eec94fdb0480 ("net: sched: use rcu for action cookie update")
> >> Reported-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> >> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
> >> ---
> >> net/sched/act_api.c | 9 +++++++--
> >> 1 file changed, 7 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> >> index 66dc19746c63..148a89ab789b 100644
> >> --- a/net/sched/act_api.c
> >> +++ b/net/sched/act_api.c
> >> @@ -149,10 +149,15 @@ EXPORT_SYMBOL(__tcf_idr_release);
> >>
> >> static size_t tcf_action_shared_attrs_size(const struct tc_action *act)
> >> {
> >> + struct tc_cookie *act_cookie;
> >> u32 cookie_len = 0;
> >>
> >> - if (act->act_cookie)
> >> - cookie_len = nla_total_size(act->act_cookie->len);
> >> + rcu_read_lock();
> >> + act_cookie = rcu_dereference(act->act_cookie);
> >> +
> >> + if (act_cookie)
> >> + cookie_len = nla_total_size(act_cookie->len);
> >> + rcu_read_unlock();
> >
> > I am not sure if this is enough to fix the entire issue. Now it will
> > fetch the length correctly but, what guarantees that when it tries to
> > actually copy the key (tcf_action_dump_1), the same act_cookie pointer
> > will be used? As in, can't the new re-fetch be different/smaller than
> > the object used here?
>
> I checked the code of nlmsg_put() and similar functions, and they check
> that there is enough free space at skb tailroom. If not, they fail
> gracefully and return error. Am I missing something?
Talked offline with Vlad and I agree that this is fine as is.
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Thanks,
Marcelo
^ permalink raw reply
* Re: [PATCH iproute2-next] ipaddress: fix label matching
From: David Ahern @ 2018-07-12 1:01 UTC (permalink / raw)
To: Vincent Bernat, netdev, stephen, serhe.popovych
In-Reply-To: <20180711113603.16589-1-vincent@bernat.im>
On 7/11/18 7:36 AM, Vincent Bernat wrote:
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index 5009bfe6d2e3..20ef6724944e 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -837,11 +837,6 @@ int print_linkinfo(const struct sockaddr_nl *who,
> if (!name)
> return -1;
>
> - if (filter.label &&
> - (!filter.family || filter.family == AF_PACKET) &&
> - fnmatch(filter.label, name, 0))
> - return -1;
> -
The offending commit changed the return code:
if (filter.label &&
(!filter.family || filter.family == AF_PACKET) &&
- fnmatch(filter.label, RTA_DATA(tb[IFLA_IFNAME]), 0))
- return 0;
+ fnmatch(filter.label, name, 0))
+ return -1;
Vincent: can you try leaving the code as is, but change the return to 0?
^ permalink raw reply
* Re: [PATCH v4 iproute2-next 0/3] Add support for ETF qdisc
From: David Ahern @ 2018-07-12 0:53 UTC (permalink / raw)
To: Jesus Sanchez-Palencia, netdev; +Cc: jhs, xiyou.wangcong, jiri
In-Reply-To: <20180709235613.23642-1-jesus.sanchez-palencia@intel.com>
On 7/9/18 7:56 PM, Jesus Sanchez-Palencia wrote:
> fixes since v3:
> - Add support for clock names with the "CLOCK_" prefix;
> - Print clock name on print_opt();
> - Use strcasecmp() instead of strncasecmp().
>
>
> The ETF (earliest txtime first) qdisc was recently merged into net-next
> [1], so this patchset adds support for it through the tc command line
> tool.
>
> An initial man page is also provided.
>
> The first commit in this series is adding an updated version of
> include/uapi/linux/pkt_sched.h and is not meant to be merged. It's
> provided here just as a convenience for those who want to easily build
> this patchset.
>
> [1] https://patchwork.ozlabs.org/cover/938991/
>
applied to iproute2-next. Thanks,
^ permalink raw reply
* KASAN: use-after-free Write in tls_push_record (2)
From: syzbot @ 2018-07-12 0:49 UTC (permalink / raw)
To: aviadye, borisp, davejwatson, davem, linux-kernel, netdev,
syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: 1e09177acae3 Merge tag 'mips_fixes_4.18_3' of git://git.ke..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=128903b2400000
kernel config: https://syzkaller.appspot.com/x/.config?x=25856fac4e580aa7
dashboard link: https://syzkaller.appspot.com/bug?extid=6c4e6ecbf9a2797be67c
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=12312678400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13ef76c2400000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+6c4e6ecbf9a2797be67c@syzkaller.appspotmail.com
RDX: 00000000fffffdef RSI: 00000000200005c0 RDI: 0000000000000003
RBP: 00000000006cb018 R08: 0000000020000000 R09: 000000000000001c
R10: 0000000000000040 R11: 0000000000000212 R12: 0000000000000005
R13: ffffffffffffffff R14: 0000000000000000 R15: 0000000000000000
==================================================================
BUG: KASAN: use-after-free in tls_fill_prepend include/net/tls.h:339
[inline]
BUG: KASAN: use-after-free in tls_push_record+0x1091/0x1400
net/tls/tls_sw.c:239
Write of size 1 at addr ffff8801ae430000 by task syz-executor589/4567
CPU: 0 PID: 4567 Comm: syz-executor589 Not tainted 4.18.0-rc4+ #141
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+0x1c9/0x2b4 lib/dump_stack.c:113
print_address_description+0x6c/0x20b mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
__asan_report_store1_noabort+0x17/0x20 mm/kasan/report.c:435
tls_fill_prepend include/net/tls.h:339 [inline]
tls_push_record+0x1091/0x1400 net/tls/tls_sw.c:239
tls_sw_push_pending_record+0x22/0x30 net/tls/tls_sw.c:276
tls_handle_open_record net/tls/tls_main.c:164 [inline]
tls_sk_proto_close+0x74c/0xae0 net/tls/tls_main.c:264
inet_release+0x104/0x1f0 net/ipv4/af_inet.c:427
inet6_release+0x50/0x70 net/ipv6/af_inet6.c:459
__sock_release+0xd7/0x260 net/socket.c:599
sock_close+0x19/0x20 net/socket.c:1150
__fput+0x355/0x8b0 fs/file_table.c:209
____fput+0x15/0x20 fs/file_table.c:243
task_work_run+0x1ec/0x2a0 kernel/task_work.c:113
exit_task_work include/linux/task_work.h:22 [inline]
do_exit+0x1b08/0x2750 kernel/exit.c:865
do_group_exit+0x177/0x440 kernel/exit.c:968
__do_sys_exit_group kernel/exit.c:979 [inline]
__se_sys_exit_group kernel/exit.c:977 [inline]
__x64_sys_exit_group+0x3e/0x50 kernel/exit.c:977
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x43f358
Code: Bad RIP value.
RSP: 002b:00007fff51750198 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 000000000043f358
RDX: 0000000000000000 RSI: 000000000000003c RDI: 0000000000000000
RBP: 00000000004bf448 R08: 00000000000000e7 R09: ffffffffffffffd0
R10: 0000000000000040 R11: 0000000000000246 R12: 0000000000000001
R13: 00000000006d1180 R14: 0000000000000000 R15: 0000000000000000
The buggy address belongs to the page:
page:ffffea0006b90c00 count:0 mapcount:-128 mapping:0000000000000000
index:0x0
flags: 0x2fffc0000000000()
raw: 02fffc0000000000 ffffea0006b96408 ffff88021fffac18 0000000000000000
raw: 0000000000000000 0000000000000003 00000000ffffff7f 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8801ae42ff00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff8801ae42ff80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff8801ae430000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
^
ffff8801ae430080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff8801ae430100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==================================================================
---
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.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* [PATCH net] netfilter: nf_conntrack: prevent uninit-value in gc_worker
From: Eric Dumazet @ 2018-07-12 0:40 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal
Cc: netfilter-devel, netdev, Eric Dumazet, Eric Dumazet
KMSAN reported use of uninit-value in gc_worker [1]
We need to clear ct->timeout in __nf_conntrack_alloc()
otherwise __nf_conntrack_confirm() might propagate garbage when
adding nfct_time_stamp to ct->timeout :
ct->timeout += nfct_time_stamp;
[1]
BUG: KMSAN: uninit-value in gc_worker+0x89e/0x1530 net/netfilter/nf_conntrack_core.c:1028
CPU: 1 PID: 19 Comm: kworker/1:0 Not tainted 4.18.0-rc4+ #24
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: events_power_efficient gc_worker
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x185/0x1e0 lib/dump_stack.c:113
kmsan_report+0x195/0x2c0 mm/kmsan/kmsan.c:1092
__msan_warning_32+0x7d/0xe0 mm/kmsan/kmsan_instr.c:640
gc_worker+0x89e/0x1530 net/netfilter/nf_conntrack_core.c:1028
process_one_work+0x1655/0x2000 kernel/workqueue.c:2153
worker_thread+0x1136/0x2490 kernel/workqueue.c:2296
kthread+0x473/0x4b0 kernel/kthread.c:247
ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:415
Uninit was stored to memory at:
kmsan_save_stack_with_flags mm/kmsan/kmsan.c:256 [inline]
kmsan_save_stack mm/kmsan/kmsan.c:271 [inline]
kmsan_internal_chain_origin+0x13c/0x240 mm/kmsan/kmsan.c:683
__msan_chain_origin+0x76/0xd0 mm/kmsan/kmsan_instr.c:483
__nf_conntrack_confirm+0x2700/0x3f70 net/netfilter/nf_conntrack_core.c:793
nf_conntrack_confirm include/net/netfilter/nf_conntrack_core.h:71 [inline]
ipv6_confirm+0x573/0x740 net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c:165
nf_hook_entry_hookfn include/linux/netfilter.h:119 [inline]
nf_hook_slow+0x15d/0x3e0 net/netfilter/core.c:511
nf_hook include/linux/netfilter.h:242 [inline]
NF_HOOK_COND include/linux/netfilter.h:275 [inline]
ip6_output+0x37d/0x710 net/ipv6/ip6_output.c:171
dst_output include/net/dst.h:444 [inline]
ip6_local_out+0x164/0x1d0 net/ipv6/output_core.c:176
ip6_send_skb net/ipv6/ip6_output.c:1696 [inline]
ip6_push_pending_frames+0x218/0x4d0 net/ipv6/ip6_output.c:1716
rawv6_push_pending_frames net/ipv6/raw.c:616 [inline]
rawv6_sendmsg+0x45f0/0x5410 net/ipv6/raw.c:935
inet_sendmsg+0x3fc/0x760 net/ipv4/af_inet.c:798
sock_sendmsg_nosec net/socket.c:641 [inline]
sock_sendmsg net/socket.c:651 [inline]
__sys_sendto+0x798/0x8e0 net/socket.c:1797
__do_sys_sendto net/socket.c:1809 [inline]
__se_sys_sendto net/socket.c:1805 [inline]
__x64_sys_sendto+0x1a1/0x210 net/socket.c:1805
do_syscall_64+0x15b/0x230 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x63/0xe7
Uninit was created at:
kmsan_save_stack_with_flags mm/kmsan/kmsan.c:256 [inline]
kmsan_internal_poison_shadow+0xc8/0x1d0 mm/kmsan/kmsan.c:181
kmsan_kmalloc+0xa1/0x120 mm/kmsan/kmsan_hooks.c:91
kmem_cache_alloc+0xad2/0xbb0 mm/slub.c:2739
__nf_conntrack_alloc+0x166/0x670 net/netfilter/nf_conntrack_core.c:1137
init_conntrack+0x635/0x2840 net/netfilter/nf_conntrack_core.c:1219
resolve_normal_ct net/netfilter/nf_conntrack_core.c:1333 [inline]
nf_conntrack_in+0x1812/0x2070 net/netfilter/nf_conntrack_core.c:1416
ipv6_conntrack_local+0xc3/0xf0 net/ipv6/netfilter/nf_conntrack_l3proto_ipv6.c:179
nf_hook_entry_hookfn include/linux/netfilter.h:119 [inline]
nf_hook_slow+0x15d/0x3e0 net/netfilter/core.c:511
nf_hook include/linux/netfilter.h:242 [inline]
__ip6_local_out+0x64c/0x770 net/ipv6/output_core.c:164
ip6_local_out+0xa4/0x1d0 net/ipv6/output_core.c:174
ip6_send_skb net/ipv6/ip6_output.c:1696 [inline]
ip6_push_pending_frames+0x218/0x4d0 net/ipv6/ip6_output.c:1716
rawv6_push_pending_frames net/ipv6/raw.c:616 [inline]
rawv6_sendmsg+0x45f0/0x5410 net/ipv6/raw.c:935
inet_sendmsg+0x3fc/0x760 net/ipv4/af_inet.c:798
sock_sendmsg_nosec net/socket.c:641 [inline]
sock_sendmsg net/socket.c:651 [inline]
__sys_sendto+0x798/0x8e0 net/socket.c:1797
__do_sys_sendto net/socket.c:1809 [inline]
__se_sys_sendto net/socket.c:1805 [inline]
__x64_sys_sendto+0x1a1/0x210 net/socket.c:1805
do_syscall_64+0x15b/0x230 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x63/0xe7
Fixes: f330a7fdbe16 ("netfilter: conntrack: get rid of conntrack timer")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Florian Westphal <fw@strlen.de>
Reported-by: syzbot <syzkaller@googlegroups.com>
---
net/netfilter/nf_conntrack_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 3d52804250274602c521f3cfe6c0c3b8fa9e78e9..759d09bd27140c85f1a19fdb20390558e7e8f161 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1145,6 +1145,7 @@ __nf_conntrack_alloc(struct net *net,
/* save hash for reusing when confirming */
*(unsigned long *)(&ct->tuplehash[IP_CT_DIR_REPLY].hnnode.pprev) = hash;
ct->status = 0;
+ ct->timeout = 0;
write_pnet(&ct->ct_net, net);
memset(&ct->__nfct_init_offset[0], 0,
offsetof(struct nf_conn, proto) -
--
2.18.0.203.gfac676dfb9-goog
^ permalink raw reply related
* [PATCH bpf-next 1/6] bpf: Add BPF_SOCK_OPS_TCP_LISTEN_CB
From: Andrey Ignatov @ 2018-07-12 0:33 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, brakmo, kernel-team
In-Reply-To: <cover.1531355394.git.rdna@fb.com>
Add new TCP-BPF callback that is called on listen(2) right after socket
transition to TCP_LISTEN state.
It fills the gap for listening sockets in TCP-BPF. For example BPF
program can set BPF_SOCK_OPS_STATE_CB_FLAG when socket becomes listening
and track later transition from TCP_LISTEN to TCP_CLOSE with
BPF_SOCK_OPS_STATE_CB callback.
Before there was no way to do it with TCP-BPF and other options were
much harder to work with. E.g. socket state tracking can be done with
tracepoints (either raw or regular) but they can't be attached to cgroup
and their lifetime has to be managed separately.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
include/uapi/linux/bpf.h | 3 +++
net/ipv4/af_inet.c | 1 +
2 files changed, 4 insertions(+)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index b7db3261c62d..aa11cdcbfcaf 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2557,6 +2557,9 @@ enum {
* Arg1: old_state
* Arg2: new_state
*/
+ BPF_SOCK_OPS_TCP_LISTEN_CB, /* Called on listen(2), right after
+ * socket transition to LISTEN state.
+ */
};
/* List of TCP states. There is a build check in net/ipv4/tcp.c to detect
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index c716be13d58c..f2a0a3bab6b5 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -229,6 +229,7 @@ int inet_listen(struct socket *sock, int backlog)
err = inet_csk_listen_start(sk, backlog);
if (err)
goto out;
+ tcp_call_bpf(sk, BPF_SOCK_OPS_TCP_LISTEN_CB, 0, NULL);
}
sk->sk_max_ack_backlog = backlog;
err = 0;
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 5/6] selftests/bpf: Better verification in test_tcpbpf
From: Andrey Ignatov @ 2018-07-12 0:33 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, brakmo, kernel-team
In-Reply-To: <cover.1531355394.git.rdna@fb.com>
Reduce amount of copy/paste for debug info when result is verified in
the test and keep that info together with values being checked so that
they won't get out of sync.
It also improves debug experience: instead of checking manually what
doesn't match in debug output for all fields, only unexpected field is
printed.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
.../testing/selftests/bpf/test_tcpbpf_user.c | 64 +++++++++++--------
1 file changed, 39 insertions(+), 25 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_tcpbpf_user.c b/tools/testing/selftests/bpf/test_tcpbpf_user.c
index fa97ec6428de..971f1644b9c7 100644
--- a/tools/testing/selftests/bpf/test_tcpbpf_user.c
+++ b/tools/testing/selftests/bpf/test_tcpbpf_user.c
@@ -1,4 +1,5 @@
// SPDX-License-Identifier: GPL-2.0
+#include <inttypes.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
@@ -15,6 +16,42 @@
#include "test_tcpbpf.h"
+#define EXPECT_EQ(expected, actual, fmt) \
+ do { \
+ if ((expected) != (actual)) { \
+ printf(" Value of: " #actual "\n" \
+ " Actual: %" fmt "\n" \
+ " Expected: %" fmt "\n", \
+ (actual), (expected)); \
+ goto err; \
+ } \
+ } while (0)
+
+int verify_result(const struct tcpbpf_globals *result)
+{
+ __u32 expected_events;
+
+ expected_events = ((1 << BPF_SOCK_OPS_TIMEOUT_INIT) |
+ (1 << BPF_SOCK_OPS_RWND_INIT) |
+ (1 << BPF_SOCK_OPS_TCP_CONNECT_CB) |
+ (1 << BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB) |
+ (1 << BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB) |
+ (1 << BPF_SOCK_OPS_NEEDS_ECN) |
+ (1 << BPF_SOCK_OPS_STATE_CB));
+
+ EXPECT_EQ(expected_events, result->event_map, "#" PRIx32);
+ EXPECT_EQ(501ULL, result->bytes_received, "llu");
+ EXPECT_EQ(1002ULL, result->bytes_acked, "llu");
+ EXPECT_EQ(1, result->data_segs_in, PRIu32);
+ EXPECT_EQ(1, result->data_segs_out, PRIu32);
+ EXPECT_EQ(0x80, result->bad_cb_test_rv, PRIu32);
+ EXPECT_EQ(0, result->good_cb_test_rv, PRIu32);
+
+ return 0;
+err:
+ return -1;
+}
+
static int bpf_find_map(const char *test, struct bpf_object *obj,
const char *name)
{
@@ -33,7 +70,6 @@ int main(int argc, char **argv)
const char *file = "test_tcpbpf_kern.o";
struct tcpbpf_globals g = {0};
const char *cg_path = "/foo";
- bool debug_flag = false;
int error = EXIT_FAILURE;
struct bpf_object *obj;
int prog_fd, map_fd;
@@ -41,9 +77,6 @@ int main(int argc, char **argv)
__u32 key = 0;
int rv;
- if (argc > 1 && strcmp(argv[1], "-d") == 0)
- debug_flag = true;
-
if (setup_cgroup_environment())
goto err;
@@ -81,30 +114,11 @@ int main(int argc, char **argv)
goto err;
}
- if (g.bytes_received != 501 || g.bytes_acked != 1002 ||
- g.data_segs_in != 1 || g.data_segs_out != 1 ||
- (g.event_map ^ 0x47e) != 0 || g.bad_cb_test_rv != 0x80 ||
- g.good_cb_test_rv != 0) {
+ if (verify_result(&g)) {
printf("FAILED: Wrong stats\n");
- if (debug_flag) {
- printf("\n");
- printf("bytes_received: %d (expecting 501)\n",
- (int)g.bytes_received);
- printf("bytes_acked: %d (expecting 1002)\n",
- (int)g.bytes_acked);
- printf("data_segs_in: %d (expecting 1)\n",
- g.data_segs_in);
- printf("data_segs_out: %d (expecting 1)\n",
- g.data_segs_out);
- printf("event_map: 0x%x (at least 0x47e)\n",
- g.event_map);
- printf("bad_cb_test_rv: 0x%x (expecting 0x80)\n",
- g.bad_cb_test_rv);
- printf("good_cb_test_rv:0x%x (expecting 0)\n",
- g.good_cb_test_rv);
- }
goto err;
}
+
printf("PASSED!\n");
error = 0;
err:
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 2/6] bpf: Sync bpf.h to tools/
From: Andrey Ignatov @ 2018-07-12 0:33 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, brakmo, kernel-team
In-Reply-To: <cover.1531355394.git.rdna@fb.com>
Sync BPF_SOCK_OPS_TCP_LISTEN_CB related UAPI changes to tools/.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
tools/include/uapi/linux/bpf.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 59b19b6a40d7..3b0ab93bc94f 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2555,6 +2555,9 @@ enum {
* Arg1: old_state
* Arg2: new_state
*/
+ BPF_SOCK_OPS_TCP_LISTEN_CB, /* Called on listen(2), right after
+ * socket transition to LISTEN state.
+ */
};
/* List of TCP states. There is a build check in net/ipv4/tcp.c to detect
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 4/6] selftests/bpf: Switch test_tcpbpf_user to cgroup_helpers
From: Andrey Ignatov @ 2018-07-12 0:33 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, brakmo, kernel-team
In-Reply-To: <cover.1531355394.git.rdna@fb.com>
Switch to cgroup_helpers to simplify the code and fix cgroup cleanup:
before cgroup was not cleaned up after the test.
It also removes SYSTEM macro, that only printed error, but didn't
terminate the test.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
tools/testing/selftests/bpf/Makefile | 1 +
.../testing/selftests/bpf/test_tcpbpf_user.c | 55 +++++++------------
2 files changed, 22 insertions(+), 34 deletions(-)
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 7a6214e9ae58..478bf1bcbbf5 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -61,6 +61,7 @@ $(OUTPUT)/test_dev_cgroup: cgroup_helpers.c
$(OUTPUT)/test_sock: cgroup_helpers.c
$(OUTPUT)/test_sock_addr: cgroup_helpers.c
$(OUTPUT)/test_sockmap: cgroup_helpers.c
+$(OUTPUT)/test_tcpbpf_user: cgroup_helpers.c
$(OUTPUT)/test_progs: trace_helpers.c
$(OUTPUT)/get_cgroup_id_user: cgroup_helpers.c
diff --git a/tools/testing/selftests/bpf/test_tcpbpf_user.c b/tools/testing/selftests/bpf/test_tcpbpf_user.c
index 84ab5163c828..fa97ec6428de 100644
--- a/tools/testing/selftests/bpf/test_tcpbpf_user.c
+++ b/tools/testing/selftests/bpf/test_tcpbpf_user.c
@@ -1,25 +1,18 @@
// SPDX-License-Identifier: GPL-2.0
#include <stdio.h>
#include <stdlib.h>
-#include <stdio.h>
#include <unistd.h>
#include <errno.h>
-#include <signal.h>
#include <string.h>
-#include <assert.h>
-#include <linux/perf_event.h>
-#include <linux/ptrace.h>
#include <linux/bpf.h>
-#include <sys/ioctl.h>
-#include <sys/time.h>
#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
#include <bpf/bpf.h>
#include <bpf/libbpf.h>
-#include "bpf_util.h"
+
#include "bpf_rlimit.h"
-#include <linux/perf_event.h>
+#include "bpf_util.h"
+#include "cgroup_helpers.h"
+
#include "test_tcpbpf.h"
static int bpf_find_map(const char *test, struct bpf_object *obj,
@@ -35,42 +28,32 @@ static int bpf_find_map(const char *test, struct bpf_object *obj,
return bpf_map__fd(map);
}
-#define SYSTEM(CMD) \
- do { \
- if (system(CMD)) { \
- printf("system(%s) FAILS!\n", CMD); \
- } \
- } while (0)
-
int main(int argc, char **argv)
{
const char *file = "test_tcpbpf_kern.o";
struct tcpbpf_globals g = {0};
- int cg_fd, prog_fd, map_fd;
+ const char *cg_path = "/foo";
bool debug_flag = false;
int error = EXIT_FAILURE;
struct bpf_object *obj;
- char cmd[100], *dir;
- struct stat buffer;
+ int prog_fd, map_fd;
+ int cg_fd = -1;
__u32 key = 0;
- int pid;
int rv;
if (argc > 1 && strcmp(argv[1], "-d") == 0)
debug_flag = true;
- dir = "/tmp/cgroupv2/foo";
+ if (setup_cgroup_environment())
+ goto err;
+
+ cg_fd = create_and_get_cgroup(cg_path);
+ if (!cg_fd)
+ goto err;
- if (stat(dir, &buffer) != 0) {
- SYSTEM("mkdir -p /tmp/cgroupv2");
- SYSTEM("mount -t cgroup2 none /tmp/cgroupv2");
- SYSTEM("mkdir -p /tmp/cgroupv2/foo");
- }
- pid = (int) getpid();
- sprintf(cmd, "echo %d >> /tmp/cgroupv2/foo/cgroup.procs", pid);
- SYSTEM(cmd);
+ if (join_cgroup(cg_path))
+ goto err;
- cg_fd = open(dir, O_DIRECTORY, O_RDONLY);
if (bpf_prog_load(file, BPF_PROG_TYPE_SOCK_OPS, &obj, &prog_fd)) {
printf("FAILED: load_bpf_file failed for: %s\n", file);
goto err;
@@ -83,7 +66,10 @@ int main(int argc, char **argv)
goto err;
}
- SYSTEM("./tcp_server.py");
+ if (system("./tcp_server.py")) {
+ printf("FAILED: TCP server\n");
+ goto err;
+ }
map_fd = bpf_find_map(__func__, obj, "global_map");
if (map_fd < 0)
@@ -123,6 +109,7 @@ int main(int argc, char **argv)
error = 0;
err:
bpf_prog_detach(cg_fd, BPF_CGROUP_SOCK_OPS);
+ close(cg_fd);
+ cleanup_cgroup_environment();
return error;
-
}
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 3/6] selftests/bpf: Fix const'ness in cgroup_helpers
From: Andrey Ignatov @ 2018-07-12 0:33 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, brakmo, kernel-team
In-Reply-To: <cover.1531355394.git.rdna@fb.com>
Lack of const in cgroup helpers signatures forces to write ugly client
code. Fix it.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
tools/testing/selftests/bpf/cgroup_helpers.c | 6 +++---
tools/testing/selftests/bpf/cgroup_helpers.h | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/cgroup_helpers.c b/tools/testing/selftests/bpf/cgroup_helpers.c
index c87b4e052ce9..cf16948aad4a 100644
--- a/tools/testing/selftests/bpf/cgroup_helpers.c
+++ b/tools/testing/selftests/bpf/cgroup_helpers.c
@@ -118,7 +118,7 @@ static int join_cgroup_from_top(char *cgroup_path)
*
* On success, it returns 0, otherwise on failure it returns 1.
*/
-int join_cgroup(char *path)
+int join_cgroup(const char *path)
{
char cgroup_path[PATH_MAX + 1];
@@ -158,7 +158,7 @@ void cleanup_cgroup_environment(void)
* On success, it returns the file descriptor. On failure it returns 0.
* If there is a failure, it prints the error to stderr.
*/
-int create_and_get_cgroup(char *path)
+int create_and_get_cgroup(const char *path)
{
char cgroup_path[PATH_MAX + 1];
int fd;
@@ -186,7 +186,7 @@ int create_and_get_cgroup(char *path)
* which is an invalid cgroup id.
* If there is a failure, it prints the error to stderr.
*/
-unsigned long long get_cgroup_id(char *path)
+unsigned long long get_cgroup_id(const char *path)
{
int dirfd, err, flags, mount_id, fhsize;
union {
diff --git a/tools/testing/selftests/bpf/cgroup_helpers.h b/tools/testing/selftests/bpf/cgroup_helpers.h
index 20a4a5dcd469..d64bb8957090 100644
--- a/tools/testing/selftests/bpf/cgroup_helpers.h
+++ b/tools/testing/selftests/bpf/cgroup_helpers.h
@@ -9,10 +9,10 @@
__FILE__, __LINE__, clean_errno(), ##__VA_ARGS__)
-int create_and_get_cgroup(char *path);
-int join_cgroup(char *path);
+int create_and_get_cgroup(const char *path);
+int join_cgroup(const char *path);
int setup_cgroup_environment(void);
void cleanup_cgroup_environment(void);
-unsigned long long get_cgroup_id(char *path);
+unsigned long long get_cgroup_id(const char *path);
#endif
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 6/6] selftests/bpf: Test case for BPF_SOCK_OPS_TCP_LISTEN_CB
From: Andrey Ignatov @ 2018-07-12 0:33 UTC (permalink / raw)
To: netdev; +Cc: Andrey Ignatov, ast, daniel, brakmo, kernel-team
In-Reply-To: <cover.1531355394.git.rdna@fb.com>
Cover new TCP-BPF callback in test_tcpbpf: when listen() is called on
socket, set BPF_SOCK_OPS_STATE_CB_FLAG so that BPF_SOCK_OPS_STATE_CB
callback can be called on future state transition, and when such a
transition happens (TCP_LISTEN -> TCP_CLOSE), track it in the map and
verify it in user space later.
Signed-off-by: Andrey Ignatov <rdna@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
tools/testing/selftests/bpf/test_tcpbpf.h | 1 +
tools/testing/selftests/bpf/test_tcpbpf_kern.c | 17 ++++++++++++-----
tools/testing/selftests/bpf/test_tcpbpf_user.c | 4 +++-
3 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_tcpbpf.h b/tools/testing/selftests/bpf/test_tcpbpf.h
index 2fe43289943c..7bcfa6207005 100644
--- a/tools/testing/selftests/bpf/test_tcpbpf.h
+++ b/tools/testing/selftests/bpf/test_tcpbpf.h
@@ -12,5 +12,6 @@ struct tcpbpf_globals {
__u32 good_cb_test_rv;
__u64 bytes_received;
__u64 bytes_acked;
+ __u32 num_listen;
};
#endif
diff --git a/tools/testing/selftests/bpf/test_tcpbpf_kern.c b/tools/testing/selftests/bpf/test_tcpbpf_kern.c
index 3e645ee41ed5..4b7fd540cea9 100644
--- a/tools/testing/selftests/bpf/test_tcpbpf_kern.c
+++ b/tools/testing/selftests/bpf/test_tcpbpf_kern.c
@@ -96,15 +96,22 @@ int bpf_testcb(struct bpf_sock_ops *skops)
if (!gp)
break;
g = *gp;
- g.total_retrans = skops->total_retrans;
- g.data_segs_in = skops->data_segs_in;
- g.data_segs_out = skops->data_segs_out;
- g.bytes_received = skops->bytes_received;
- g.bytes_acked = skops->bytes_acked;
+ if (skops->args[0] == BPF_TCP_LISTEN) {
+ g.num_listen++;
+ } else {
+ g.total_retrans = skops->total_retrans;
+ g.data_segs_in = skops->data_segs_in;
+ g.data_segs_out = skops->data_segs_out;
+ g.bytes_received = skops->bytes_received;
+ g.bytes_acked = skops->bytes_acked;
+ }
bpf_map_update_elem(&global_map, &key, &g,
BPF_ANY);
}
break;
+ case BPF_SOCK_OPS_TCP_LISTEN_CB:
+ bpf_sock_ops_cb_flags_set(skops, BPF_SOCK_OPS_STATE_CB_FLAG);
+ break;
default:
rv = -1;
}
diff --git a/tools/testing/selftests/bpf/test_tcpbpf_user.c b/tools/testing/selftests/bpf/test_tcpbpf_user.c
index 971f1644b9c7..a275c2971376 100644
--- a/tools/testing/selftests/bpf/test_tcpbpf_user.c
+++ b/tools/testing/selftests/bpf/test_tcpbpf_user.c
@@ -37,7 +37,8 @@ int verify_result(const struct tcpbpf_globals *result)
(1 << BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB) |
(1 << BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB) |
(1 << BPF_SOCK_OPS_NEEDS_ECN) |
- (1 << BPF_SOCK_OPS_STATE_CB));
+ (1 << BPF_SOCK_OPS_STATE_CB) |
+ (1 << BPF_SOCK_OPS_TCP_LISTEN_CB));
EXPECT_EQ(expected_events, result->event_map, "#" PRIx32);
EXPECT_EQ(501ULL, result->bytes_received, "llu");
@@ -46,6 +47,7 @@ int verify_result(const struct tcpbpf_globals *result)
EXPECT_EQ(1, result->data_segs_out, PRIu32);
EXPECT_EQ(0x80, result->bad_cb_test_rv, PRIu32);
EXPECT_EQ(0, result->good_cb_test_rv, PRIu32);
+ EXPECT_EQ(1, result->num_listen, PRIu32);
return 0;
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