* Re: [PATCH bpf-next v4 0/2] libbpf: adding AF_XDP support
From: Daniel Borkmann @ 2019-02-18 9:38 UTC (permalink / raw)
To: Magnus Karlsson
Cc: Jesper Dangaard Brouer, Jonathan Lemon, Magnus Karlsson,
Björn Töpel, ast, Network Development, Jakub Kicinski,
Björn Töpel, Zhang, Qi Z, xiaolong.ye,
xdp-newbies@vger.kernel.org
In-Reply-To: <CAJ8uoz3kRrXJCthhm4u=0Z-7=uNryB_r8Yj3p7inYEPjtLp2Kw@mail.gmail.com>
On 02/18/2019 09:20 AM, Magnus Karlsson wrote:
> On Fri, Feb 15, 2019 at 5:48 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>>
>> On 02/13/2019 12:55 PM, Jesper Dangaard Brouer wrote:
>>> On Wed, 13 Feb 2019 12:32:47 +0100
>>> Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
>>>> On Mon, Feb 11, 2019 at 9:44 PM Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
>>>>> On 8 Feb 2019, at 5:05, Magnus Karlsson wrote:
>>>>>
>>>>>> This patch proposes to add AF_XDP support to libbpf. The main reason
>>>>>> for this is to facilitate writing applications that use AF_XDP by
>>>>>> offering higher-level APIs that hide many of the details of the AF_XDP
>>>>>> uapi. This is in the same vein as libbpf facilitates XDP adoption by
>>>>>> offering easy-to-use higher level interfaces of XDP
>>>>>> functionality. Hopefully this will facilitate adoption of AF_XDP, make
>>>>>> applications using it simpler and smaller, and finally also make it
>>>>>> possible for applications to benefit from optimizations in the AF_XDP
>>>>>> user space access code. Previously, people just copied and pasted the
>>>>>> code from the sample application into their application, which is not
>>>>>> desirable.
>>>>>
>>>>> I like the idea of encapsulating the boilerplate logic in a library.
>>>>>
>>>>> I do think there is an important missing piece though - there should be
>>>>> some code which queries the netdev for how many queues are attached, and
>>>>> create the appropriate number of umem/AF_XDP sockets.
>>>>>
>>>>> I ran into this issue when testing the current AF_XDP code - on my test
>>>>> boxes, the mlx5 card has 55 channels (aka queues), so when the test program
>>>>> binds only to channel 0, nothing works as expected, since not all traffic
>>>>> is being intercepted. While obvious in hindsight, this took a while to
>>>>> track down.
>>>>
>>>> Yes, agreed. You are not the first one to stumble upon this problem
>>>> :-). Let me think a little bit on how to solve this in a good way. We
>>>> need this to be simple and intuitive, as you say.
>>>
>>> I see people hitting this with AF_XDP all the time... I had some
>>> backup-slides[2] in our FOSDEM presentation[1] that describe the issue,
>>> give the performance reason why and propose a workaround.
>>
>> Magnus, I presume you're going to address this for the initial libbpf merge
>> since the plan is to make it easier to consume for users?
>
> I think the first thing we need is education and documentation. Have a
> FAQ or "common mistakes" section in the Documentation. And of course,
> sending Jesper around the world reminding people about this ;-).
>
> To address this on a libbpf interface level, I think the best way is
> to reprogram the NIC to send all traffic to the queue that you
> provided in the xsk_socket__create call. This "set up NIC routing"
> behavior can then be disable with a flag, just as the XDP program
> loading can be disabled. The standard config of xsk_socket__create
> will then set up as many things for the user as possible just to get
> up and running quickly. More advanced users can then disable parts of
> it to gain more flexibility. Does this sound OK? Do not want to go the
> route of polling multiple sockets and aggregating the traffic as this
> will have significant negative performance implications.
I think that is fine, I would probably make this one a dedicated API call
in order to have some more flexibility than just simple flag. E.g. once
nfp AF_XDP support lands at some point, I could imagine that this call
resp. a drop-in replacement API call for more advanced steering could
also take an offloaded BPF prog fd, for example, which then would program
the steering on the NIC [0]. Seems at least there's enough complexity on
its own to have a dedicated API for it. Thoughts?
Thanks,
Daniel
[0] https://patchwork.ozlabs.org/cover/910614/
^ permalink raw reply
* Re: [PATCH] net/mlx4_en: fix spelling mistake: "quiting" -> "quitting"
From: Tariq Toukan @ 2019-02-18 9:37 UTC (permalink / raw)
To: Colin King, David S . Miller, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org
Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190217230331.18678-1-colin.king@canonical.com>
On 2/18/2019 1:03 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> There is a spelling mistake in a en_err error message. Fix it.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index 6b88881b8e35..c1438ae52a11 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -3360,7 +3360,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
> dev->addr_len = ETH_ALEN;
> mlx4_en_u64_to_mac(dev->dev_addr, mdev->dev->caps.def_mac[priv->port]);
> if (!is_valid_ether_addr(dev->dev_addr)) {
> - en_err(priv, "Port: %d, invalid mac burned: %pM, quiting\n",
> + en_err(priv, "Port: %d, invalid mac burned: %pM, quitting\n",
> priv->port, dev->dev_addr);
> err = -EINVAL;
> goto out;
>
Hi Colin, thanks for your patch.
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
I would suggest adding a Fixes line, but looking into the history of the
typo, it went through many patches that modified this line but preserved
the typo.
Actually, it dates back to the very first commit that introduces mlx4
driver:
Patches history:
2b3ddf27f48c net/mlx4_core: Replace VF zero mac with random mac in mlx4_core
ef96f7d46ad8 net/mlx4_en: Handle unassigned VF MAC address correctly
6bbb6d99f3d2 net/mlx4_en: Optimize Rx fast path filter checks
453a60827735 mlx4_en: Giving interface name in debug messages
c27a02cd94d6 mlx4_en: Add driver for Mellanox ConnectX 10GbE NIC
I'm not sure what the "Fixes:" policy is in these cases.
Thanks,
Tariq
^ permalink raw reply
* Re: [PATCH net-next 11/12] net: sched: flower: track rtnl lock state
From: Vlad Buslov @ 2019-02-18 9:35 UTC (permalink / raw)
To: Stefano Brivio
Cc: netdev@vger.kernel.org, jhs@mojatatu.com,
xiyou.wangcong@gmail.com, jiri@resnulli.us, davem@davemloft.net
In-Reply-To: <20190215234659.3630ad3c@redhat.com>
On Fri 15 Feb 2019 at 22:46, Stefano Brivio <sbrivio@redhat.com> wrote:
> On Thu, 14 Feb 2019 09:47:11 +0200
> Vlad Buslov <vladbu@mellanox.com> wrote:
>
>> static int fl_hw_replace_filter(struct tcf_proto *tp,
>> - struct cls_fl_filter *f,
>> + struct cls_fl_filter *f, bool rtnl_held,
>> struct netlink_ext_ack *extack)
>> {
>> struct tc_cls_flower_offload cls_flower = {};
>> struct tcf_block *block = tp->chain->block;
>> bool skip_sw = tc_skip_sw(f->flags);
>> - int err;
>> + int err = 0;
>> +
>> + if (!rtnl_held)
>> + rtnl_lock();
>>
>> cls_flower.rule = flow_rule_alloc(tcf_exts_num_actions(&f->exts));
>> if (!cls_flower.rule)
>
> return -ENOMEM;
>
> Don't you need to:
>
> err = -ENOMEM;
> goto errout;
>
> here?
>
> Same...
>
> err = tc_setup_flow_action(&cls_flower.rule->action, &f->exts);
> if (err) {
> kfree(cls_flower.rule);
> if (skip_sw) {
> NL_SET_ERR_MSG_MOD(extack, "Failed to setup flow action");
> return err;
>
> here,
>
> }
> return 0;
>
> and here.
Thanks for catching this!
These error handlers were introduced by recent Pablo's patches and I
missed them during rebase.
^ permalink raw reply
* [PATCH net-next] net: sched: potential NULL dereference in tcf_block_find()
From: Dan Carpenter @ 2019-02-18 9:26 UTC (permalink / raw)
To: Jamal Hadi Salim, Vlad Buslov
Cc: Cong Wang, Jiri Pirko, David S. Miller, netdev, kernel-janitors
The error code isn't set on this path so it would result in returning
ERR_PTR(0) and a NULL dereference in the caller.
Fixes: 18d3eefb17cf ("net: sched: refactor tcf_block_find() into standalone functions")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
net/sched/cls_api.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 266fcb34fefe..aac72b853fe9 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1310,8 +1310,10 @@ static struct tcf_block *tcf_block_find(struct net *net, struct Qdisc **q,
goto errout_qdisc;
block = __tcf_block_find(net, *q, *cl, ifindex, block_index, extack);
- if (IS_ERR(block))
+ if (IS_ERR(block)) {
+ err = PTR_ERR(block);
goto errout_qdisc;
+ }
return block;
--
2.17.1
^ permalink raw reply related
* [net PATCH] net: vrf: remove MTU limits for vrf device
From: Hangbin Liu @ 2019-02-18 9:14 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, David S . Miller, Davide Caratti, Hangbin Liu
Similiar to commit e94cd8113ce63 ("net: remove MTU limits for dummy and
ifb device"), MTU is irrelevant for VRF device. We init it as 64K while
limit it to [68, 1500] may make users feel confused.
Reported-by: Jianlin Shi <jishi@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
drivers/net/vrf.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index 95909e262ba4..7c1430ed0244 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -1273,6 +1273,9 @@ static void vrf_setup(struct net_device *dev)
/* default to no qdisc; user can add if desired */
dev->priv_flags |= IFF_NO_QUEUE;
+
+ dev->min_mtu = 0;
+ dev->max_mtu = 0;
}
static int vrf_validate(struct nlattr *tb[], struct nlattr *data[],
--
2.19.2
^ permalink raw reply related
* Re: [PATCH bpf-next v4 1/2] libbpf: add support for using AF_XDP sockets
From: Magnus Karlsson @ 2019-02-18 8:59 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Magnus Karlsson, Björn Töpel, ast, Network Development,
Jakub Kicinski, Björn Töpel, Zhang, Qi Z,
Jesper Dangaard Brouer, xiaolong.ye
In-Reply-To: <f2f4134d-a01f-8d14-26c3-2ca82405a960@iogearbox.net>
On Fri, Feb 15, 2019 at 6:09 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 02/08/2019 02:05 PM, Magnus Karlsson wrote:
> > This commit adds AF_XDP support to libbpf. The main reason for this is
> > to facilitate writing applications that use AF_XDP by offering
> > higher-level APIs that hide many of the details of the AF_XDP
> > uapi. This is in the same vein as libbpf facilitates XDP adoption by
> > offering easy-to-use higher level interfaces of XDP
> > functionality. Hopefully this will facilitate adoption of AF_XDP, make
> > applications using it simpler and smaller, and finally also make it
> > possible for applications to benefit from optimizations in the AF_XDP
> > user space access code. Previously, people just copied and pasted the
> > code from the sample application into their application, which is not
> > desirable.
> >
> > The interface is composed of two parts:
> >
> > * Low-level access interface to the four rings and the packet
> > * High-level control plane interface for creating and setting
> > up umems and af_xdp sockets as well as a simple XDP program.
> >
> > Tested-by: Björn Töpel <bjorn.topel@intel.com>
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> [...]
> > diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
> > new file mode 100644
> > index 0000000..a982a76
> > --- /dev/null
> > +++ b/tools/lib/bpf/xsk.c
> > @@ -0,0 +1,742 @@
> > +// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
> > +
> > +/*
> > + * AF_XDP user-space access library.
> > + *
> > + * Copyright(c) 2018 - 2019 Intel Corporation.
> > + *
> > + * Author(s): Magnus Karlsson <magnus.karlsson@intel.com>
> > + */
> > +
> > +#include <errno.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <unistd.h>
> > +#include <arpa/inet.h>
> > +#include <asm/barrier.h>
> > +#include <linux/compiler.h>
> > +#include <linux/filter.h>
> > +#include <linux/if_ether.h>
> > +#include <linux/if_link.h>
> > +#include <linux/if_packet.h>
> > +#include <linux/if_xdp.h>
> > +#include <linux/rtnetlink.h>
> > +#include <net/if.h>
> > +#include <sys/mman.h>
> > +#include <sys/socket.h>
> > +#include <sys/types.h>
> > +
> > +#include "bpf.h"
> > +#include "libbpf.h"
> > +#include "libbpf_util.h"
> > +#include "nlattr.h"
> > +#include "xsk.h"
> > +
> > +#ifndef SOL_XDP
> > + #define SOL_XDP 283
> > +#endif
> > +
> > +#ifndef AF_XDP
> > + #define AF_XDP 44
> > +#endif
> > +
> > +#ifndef PF_XDP
> > + #define PF_XDP AF_XDP
> > +#endif
> > +
> > +struct xsk_umem {
> > + struct xsk_ring_prod *fill;
> > + struct xsk_ring_cons *comp;
> > + char *umem_area;
> > + struct xsk_umem_config config;
> > + int fd;
> > + int refcount;
> > +};
> > +
> > +struct xsk_socket {
> > + struct xsk_ring_cons *rx;
> > + struct xsk_ring_prod *tx;
> > + __u64 outstanding_tx;
> > + struct xsk_umem *umem;
> > + struct xsk_socket_config config;
> > + int fd;
> > + int xsks_map;
> > + int ifindex;
> > + int prog_fd;
> > + int qidconf_map_fd;
> > + int xsks_map_fd;
> > + __u32 queue_id;
> > +};
> > +
> > +struct xsk_nl_info {
> > + bool xdp_prog_attached;
> > + int ifindex;
> > + int fd;
> > +};
> > +
> > +#define MAX_QUEUES 128
>
> Why is this a fixed constant here, shouldn't this be dynamic due to being NIC
> specific anyway?
It was only here for simplicity. If a NIC had more queues, it would
require a recompile of the lib. Obviously, not desirable in a distro.
What I could do is to read the max "combined" queues (pre-set maximum
in the ethtool output) from the same interface as ethool uses and size
the array after that. Or is there a simpler way? What to do if the NIC
does not have a "combined", or is there no such NIC (seems the common
HW ones set this)?
> [...]
> > +void *xsk_umem__get_data(struct xsk_umem *umem, __u64 addr)
> > +{
> > + return &((char *)(umem->umem_area))[addr];
> > +}
>
> There's also a xsk_umem__get_data_raw() doing the same. Why having both, resp.
> when to choose which? ;)
There is enough to have the xsk_umem__get_data_raw() function.
xsk_umem__get_data() is just a convenience function for which the
application does not have to store the beginning of the umem. But as
the application always has to provide this anyway in the
xsk_umem__create() function, it might as well store this pointer. I
will delete xsk_umem__get_data() and rename xsk_umem__get_data_raw()
to xsk_umem__get_data().
> > +int xsk_umem__fd(const struct xsk_umem *umem)
> > +{
> > + return umem ? umem->fd : -EINVAL;
> > +}
> > +
> > +int xsk_socket__fd(const struct xsk_socket *xsk)
> > +{
> > + return xsk ? xsk->fd : -EINVAL;
> > +}
> > +
> > +static bool xsk_page_aligned(void *buffer)
> > +{
> > + unsigned long addr = (unsigned long)buffer;
> > +
> > + return !(addr & (getpagesize() - 1));
> > +}
> > +
> > +static void xsk_set_umem_config(struct xsk_umem_config *cfg,
> > + const struct xsk_umem_config *usr_cfg)
> > +{
> > + if (!usr_cfg) {
> > + cfg->fill_size = XSK_RING_PROD__DEFAULT_NUM_DESCS;
> > + cfg->comp_size = XSK_RING_CONS__DEFAULT_NUM_DESCS;
> > + cfg->frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE;
> > + cfg->frame_headroom = XSK_UMEM__DEFAULT_FRAME_HEADROOM;
> > + return;
> > + }
> > +
> > + cfg->fill_size = usr_cfg->fill_size;
> > + cfg->comp_size = usr_cfg->comp_size;
> > + cfg->frame_size = usr_cfg->frame_size;
> > + cfg->frame_headroom = usr_cfg->frame_headroom;
>
> Just optional nit, might be a bit nicer to have it in this form:
>
> cfg->fill_size = usr_cfg ? usr_cfg->fill_size :
> XSK_RING_PROD__DEFAULT_NUM_DESCS;
I actually think the current form is clearer when there are multiple
lines. If there was only one line, I would agree with you.
> > +}
> > +
> > +static void xsk_set_xdp_socket_config(struct xsk_socket_config *cfg,
> > + const struct xsk_socket_config *usr_cfg)
> > +{
> > + if (!usr_cfg) {
> > + cfg->rx_size = XSK_RING_CONS__DEFAULT_NUM_DESCS;
> > + cfg->tx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS;
> > + cfg->libbpf_flags = 0;
> > + cfg->xdp_flags = 0;
> > + cfg->bind_flags = 0;
> > + return;
> > + }
> > +
> > + cfg->rx_size = usr_cfg->rx_size;
> > + cfg->tx_size = usr_cfg->tx_size;
> > + cfg->libbpf_flags = usr_cfg->libbpf_flags;
> > + cfg->xdp_flags = usr_cfg->xdp_flags;
> > + cfg->bind_flags = usr_cfg->bind_flags;
>
> (Ditto)
>
> > +}
> > +
> > +int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area, __u64 size,
> > + struct xsk_ring_prod *fill, struct xsk_ring_cons *comp,
> > + const struct xsk_umem_config *usr_config)
> > +{
> > + struct xdp_mmap_offsets off;
> > + struct xdp_umem_reg mr;
> > + struct xsk_umem *umem;
> > + socklen_t optlen;
> > + void *map;
> > + int err;
> > +
> > + if (!umem_area || !umem_ptr || !fill || !comp)
> > + return -EFAULT;
> > + if (!size && !xsk_page_aligned(umem_area))
> > + return -EINVAL;
> > +
> > + umem = calloc(1, sizeof(*umem));
> > + if (!umem)
> > + return -ENOMEM;
> > +
> > + umem->fd = socket(AF_XDP, SOCK_RAW, 0);
> > + if (umem->fd < 0) {
> > + err = -errno;
> > + goto out_umem_alloc;
> > + }
> > +
> > + umem->umem_area = umem_area;
> > + xsk_set_umem_config(&umem->config, usr_config);
> > +
> > + mr.addr = (uintptr_t)umem_area;
> > + mr.len = size;
> > + mr.chunk_size = umem->config.frame_size;
> > + mr.headroom = umem->config.frame_headroom;
> > +
> > + err = setsockopt(umem->fd, SOL_XDP, XDP_UMEM_REG, &mr, sizeof(mr));
> > + if (err) {
> > + err = -errno;
> > + goto out_socket;
> > + }
> > + err = setsockopt(umem->fd, SOL_XDP, XDP_UMEM_FILL_RING,
> > + &umem->config.fill_size,
> > + sizeof(umem->config.fill_size));
> > + if (err) {
> > + err = -errno;
> > + goto out_socket;
> > + }
> > + err = setsockopt(umem->fd, SOL_XDP, XDP_UMEM_COMPLETION_RING,
> > + &umem->config.comp_size,
> > + sizeof(umem->config.comp_size));
> > + if (err) {
> > + err = -errno;
> > + goto out_socket;
> > + }
> > +
> > + optlen = sizeof(off);
> > + err = getsockopt(umem->fd, SOL_XDP, XDP_MMAP_OFFSETS, &off, &optlen);
> > + if (err) {
> > + err = -errno;
> > + goto out_socket;
> > + }
> > +
> > + map = xsk_mmap(NULL, off.fr.desc +
> > + umem->config.fill_size * sizeof(__u64),
> > + PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE,
> > + umem->fd, XDP_UMEM_PGOFF_FILL_RING);
> > + if (map == MAP_FAILED) {
> > + err = -errno;
> > + goto out_socket;
> > + }
> > +
> > + umem->fill = fill;
> > + fill->mask = umem->config.fill_size - 1;
> > + fill->size = umem->config.fill_size;
> > + fill->producer = map + off.fr.producer;
> > + fill->consumer = map + off.fr.consumer;
> > + fill->ring = map + off.fr.desc;
> > + fill->cached_cons = umem->config.fill_size;
> > +
> > + map = xsk_mmap(NULL,
> > + off.cr.desc + umem->config.comp_size * sizeof(__u64),
> > + PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE,
> > + umem->fd, XDP_UMEM_PGOFF_COMPLETION_RING);
> > + if (map == MAP_FAILED) {
> > + err = -errno;
> > + goto out_mmap;
> > + }
> > +
> > + umem->comp = comp;
> > + comp->mask = umem->config.comp_size - 1;
> > + comp->size = umem->config.comp_size;
> > + comp->producer = map + off.cr.producer;
> > + comp->consumer = map + off.cr.consumer;
> > + comp->ring = map + off.cr.desc;
> > +
> > + *umem_ptr = umem;
> > + return 0;
> > +
> > +out_mmap:
> > + munmap(umem->fill,
> > + off.fr.desc + umem->config.fill_size * sizeof(__u64));
> > +out_socket:
> > + close(umem->fd);
> > +out_umem_alloc:
> > + free(umem);
> > + return err;
> > +}
> > +
> > +static int xsk_parse_nl(void *cookie, void *msg, struct nlattr **tb)
> > +{
> > + struct nlattr *tb_parsed[IFLA_XDP_MAX + 1];
> > + struct xsk_nl_info *nl_info = cookie;
> > + struct ifinfomsg *ifinfo = msg;
> > + unsigned char mode;
> > + int err;
> > +
> > + if (nl_info->ifindex && nl_info->ifindex != ifinfo->ifi_index)
> > + return 0;
> > +
> > + if (!tb[IFLA_XDP])
> > + return 0;
> > +
> > + err = libbpf_nla_parse_nested(tb_parsed, IFLA_XDP_MAX, tb[IFLA_XDP],
> > + NULL);
> > + if (err)
> > + return err;
> > +
> > + if (!tb_parsed[IFLA_XDP_ATTACHED] || !tb_parsed[IFLA_XDP_FD])
> > + return 0;
> > +
> > + mode = libbpf_nla_getattr_u8(tb_parsed[IFLA_XDP_ATTACHED]);
> > + if (mode == XDP_ATTACHED_NONE)
> > + return 0;
> > +
> > + nl_info->xdp_prog_attached = true;
> > + nl_info->fd = libbpf_nla_getattr_u32(tb_parsed[IFLA_XDP_FD]);
>
> Hm, I don't think this works if I read the intention of this helper correctly.
>
> IFLA_XDP_FD is never set for retrieving the prog from the kernel. So the
> above is a bug.
>
> We also have bpf_get_link_xdp_id(). This should probably just be reused in
> this context here.
If bpf_get_link_xdp_id() will fit my bill, I will happily use it. I
will check it out and hopefully I can drop all this code. Thanks.
> > + return 0;
> > +}
> > +
> > +static bool xsk_xdp_prog_attached(struct xsk_socket *xsk)
> > +{
> > + struct xsk_nl_info nl_info;
> > + unsigned int nl_pid;
> > + char err_buf[256];
> > + int sock, err;
> > +
> > + sock = libbpf_netlink_open(&nl_pid);
> > + if (sock < 0)
> > + return false;
> > +
> > + nl_info.xdp_prog_attached = false;
> > + nl_info.ifindex = xsk->ifindex;
> > + nl_info.fd = -1;
> > +
> > + err = libbpf_nl_get_link(sock, nl_pid, xsk_parse_nl, &nl_info);
> > + if (err) {
> > + libbpf_strerror(err, err_buf, sizeof(err_buf));
> > + pr_warning("Error:\n%s\n", err_buf);
> > + close(sock);
> > + return false;
> > + }
> > +
> > + close(sock);
> > + xsk->prog_fd = nl_info.fd;
> > + return nl_info.xdp_prog_attached;
> > +}
>
> (See bpf_get_link_xdp_id().)
>
> > +
> > +static int xsk_load_xdp_prog(struct xsk_socket *xsk)
> > +{
> > + char bpf_log_buf[BPF_LOG_BUF_SIZE];
> > + int err, prog_fd;
> > +
> > + /* This is the C-program:
> > + * SEC("xdp_sock") int xdp_sock_prog(struct xdp_md *ctx)
> > + * {
> > + * int *qidconf, index = ctx->rx_queue_index;
> [...]
> > +
> > +int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
> > + __u32 queue_id, struct xsk_umem *umem,
> > + struct xsk_ring_cons *rx, struct xsk_ring_prod *tx,
> > + const struct xsk_socket_config *usr_config)
> > +{
> > + struct sockaddr_xdp sxdp = {};
> > + struct xdp_mmap_offsets off;
> > + struct xsk_socket *xsk;
> > + socklen_t optlen;
> > + void *map;
> > + int err;
> > +
> > + if (!umem || !xsk_ptr || !rx || !tx)
> > + return -EFAULT;
> > +
> > + if (umem->refcount) {
> > + pr_warning("Error: shared umems not supported by libbpf.\n");
> > + return -EBUSY;
> > + }
> > +
> > + xsk = calloc(1, sizeof(*xsk));
> > + if (!xsk)
> > + return -ENOMEM;
> > +
> > + if (umem->refcount++ > 0) {
>
> Should this refcount rather be atomic actually?
Neither our config nor data plane interfaces are reentrant for
performance reasons. Any concurrency has to be handled explicitly on
the application level. This so that it only penalizes apps that really
need this.
Thanks for all your reviews: Magnus
> > + xsk->fd = socket(AF_XDP, SOCK_RAW, 0);
> > + if (xsk->fd < 0) {
> > + err = -errno;
> > + goto out_xsk_alloc;
> > + }
> > + } else {
> > + xsk->fd = umem->fd;
> > + }
> > +
> > + xsk->outstanding_tx = 0;
> > + xsk->queue_id = queue_id;
> > + xsk->umem = umem;
> > + xsk->ifindex = if_nametoindex(ifname);
> > + if (!xsk->ifindex) {
> > + err = -errno;
> > + goto out_socket;
> > + }
> > +
> > + xsk_set_xdp_socket_config(&xsk->config, usr_config);
> [...]
^ permalink raw reply
* [PATCH net-next v2 2/3] net: stmmac: dwmac4: Also use TBU interrupt to clean TX path
From: Jose Abreu @ 2019-02-18 8:49 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1550479678.git.joabreu@synopsys.com>
TBU interrupt is a normal interrupt and can be used to trigger the
cleaning of TX path. Lets check if it's active in DMA interrupt handler.
While at it, refactor a little bit the function:
- Don't check if RI is enabled because at function exit we will
only clear the interrupts that are enabled so, no event will be
missed.
In my tests with GMAC5 this increased performance.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
index 49f5687879df..545cb9c47433 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c
@@ -124,9 +124,9 @@ void dwmac4_disable_dma_irq(void __iomem *ioaddr, u32 chan)
int dwmac4_dma_interrupt(void __iomem *ioaddr,
struct stmmac_extra_stats *x, u32 chan)
{
- int ret = 0;
-
u32 intr_status = readl(ioaddr + DMA_CHAN_STATUS(chan));
+ u32 intr_en = readl(ioaddr + DMA_CHAN_INTR_ENA(chan));
+ int ret = 0;
/* ABNORMAL interrupts */
if (unlikely(intr_status & DMA_CHAN_STATUS_AIS)) {
@@ -151,16 +151,11 @@ int dwmac4_dma_interrupt(void __iomem *ioaddr,
if (likely(intr_status & DMA_CHAN_STATUS_NIS)) {
x->normal_irq_n++;
if (likely(intr_status & DMA_CHAN_STATUS_RI)) {
- u32 value;
-
- value = readl(ioaddr + DMA_CHAN_INTR_ENA(chan));
- /* to schedule NAPI on real RIE event. */
- if (likely(value & DMA_CHAN_INTR_ENA_RIE)) {
- x->rx_normal_irq_n++;
- ret |= handle_rx;
- }
+ x->rx_normal_irq_n++;
+ ret |= handle_rx;
}
- if (likely(intr_status & DMA_CHAN_STATUS_TI)) {
+ if (likely(intr_status & (DMA_CHAN_STATUS_TI |
+ DMA_CHAN_STATUS_TBU))) {
x->tx_normal_irq_n++;
ret |= handle_tx;
}
@@ -168,12 +163,7 @@ int dwmac4_dma_interrupt(void __iomem *ioaddr,
x->rx_early_irq++;
}
- /* Clear the interrupt by writing a logic 1 to the chanX interrupt
- * status [21-0] expect reserved bits [5-3]
- */
- writel((intr_status & 0x3fffc7),
- ioaddr + DMA_CHAN_STATUS(chan));
-
+ writel(intr_status & intr_en, ioaddr + DMA_CHAN_STATUS(chan));
return ret;
}
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 3/3] net: stmmac: dwxgmac2: Also use TBU interrupt to clean TX path
From: Jose Abreu @ 2019-02-18 8:49 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Jose Abreu, Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue
In-Reply-To: <cover.1550479678.git.joabreu@synopsys.com>
TBU interrupt is a normal interrupt and can be used to trigger the
cleaning of TX path. Lets check if it's active in DMA interrupt handler.
While at it, refactor a little bit the function:
- Don't check if RI is enabled because at function exit we will
only clear the interrupts that are enabled so, no event will
be missed.
In my tests withe XGMAC2 this increased performance.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 4 +++-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 8 +++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
index d6bb953685fa..37d5e6fe7473 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h
@@ -193,9 +193,10 @@
#define XGMAC_AIE BIT(14)
#define XGMAC_RBUE BIT(7)
#define XGMAC_RIE BIT(6)
+#define XGMAC_TBUE BIT(2)
#define XGMAC_TIE BIT(0)
#define XGMAC_DMA_INT_DEFAULT_EN (XGMAC_NIE | XGMAC_AIE | XGMAC_RBUE | \
- XGMAC_RIE | XGMAC_TIE)
+ XGMAC_RIE | XGMAC_TBUE | XGMAC_TIE)
#define XGMAC_DMA_CH_Rx_WATCHDOG(x) (0x0000313c + (0x80 * (x)))
#define XGMAC_RWT GENMASK(7, 0)
#define XGMAC_DMA_CH_STATUS(x) (0x00003160 + (0x80 * (x)))
@@ -204,6 +205,7 @@
#define XGMAC_FBE BIT(12)
#define XGMAC_RBU BIT(7)
#define XGMAC_RI BIT(6)
+#define XGMAC_TBU BIT(2)
#define XGMAC_TPS BIT(1)
#define XGMAC_TI BIT(0)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
index c5e25580a43f..2ba712b48a89 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c
@@ -283,12 +283,10 @@ static int dwxgmac2_dma_interrupt(void __iomem *ioaddr,
x->normal_irq_n++;
if (likely(intr_status & XGMAC_RI)) {
- if (likely(intr_en & XGMAC_RIE)) {
- x->rx_normal_irq_n++;
- ret |= handle_rx;
- }
+ x->rx_normal_irq_n++;
+ ret |= handle_rx;
}
- if (likely(intr_status & XGMAC_TI)) {
+ if (likely(intr_status & (XGMAC_TI | XGMAC_TBU))) {
x->tx_normal_irq_n++;
ret |= handle_tx;
}
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 0/3] net: stmmac: Performance improvements in Multi-Queue
From: Jose Abreu @ 2019-02-18 8:49 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Jose Abreu, Florian Fainelli, Joao Pinto, David S . Miller,
Giuseppe Cavallaro, Alexandre Torgue
Tested in XGMAC2 and GMAC5.
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Jose Abreu (3):
net: stmmac: Fix NAPI poll in TX path when in multi-queue
net: stmmac: dwmac4: Also use TBU interrupt to clean TX path
net: stmmac: dwxgmac2: Also use TBU interrupt to clean TX path
drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c | 24 ++---
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 4 +-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 8 +-
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 5 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 114 ++++++++++++---------
5 files changed, 79 insertions(+), 76 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH net-next v2 1/3] net: stmmac: Fix NAPI poll in TX path when in multi-queue
From: Jose Abreu @ 2019-02-18 8:49 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: Jose Abreu, Florian Fainelli, Joao Pinto, David S . Miller,
Giuseppe Cavallaro, Alexandre Torgue
In-Reply-To: <cover.1550479678.git.joabreu@synopsys.com>
Commit 8fce33317023 introduced the concept of NAPI per-channel and
independent cleaning of TX path.
This is currently breaking performance in some cases. The scenario
happens when all packets are being received in Queue 0 but the TX is
performed in Queue != 0.
Fix this by using different NAPI instances per each TX and RX queue, as
suggested by Florian.
Changes from v1:
- Pass entire ring size to TX clean path (Florian)
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Joao Pinto <jpinto@synopsys.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 5 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 114 ++++++++++++----------
2 files changed, 66 insertions(+), 53 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index 63e1064b27a2..e697ecd9b0a6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -78,11 +78,10 @@ struct stmmac_rx_queue {
};
struct stmmac_channel {
- struct napi_struct napi ____cacheline_aligned_in_smp;
+ struct napi_struct rx_napi ____cacheline_aligned_in_smp;
+ struct napi_struct tx_napi ____cacheline_aligned_in_smp;
struct stmmac_priv *priv_data;
u32 index;
- int has_rx;
- int has_tx;
};
struct stmmac_tc_entry {
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 685d20472358..c193bb1a3322 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -155,7 +155,10 @@ static void stmmac_disable_all_queues(struct stmmac_priv *priv)
for (queue = 0; queue < maxq; queue++) {
struct stmmac_channel *ch = &priv->channel[queue];
- napi_disable(&ch->napi);
+ if (queue < rx_queues_cnt)
+ napi_disable(&ch->rx_napi);
+ if (queue < tx_queues_cnt)
+ napi_disable(&ch->tx_napi);
}
}
@@ -173,7 +176,10 @@ static void stmmac_enable_all_queues(struct stmmac_priv *priv)
for (queue = 0; queue < maxq; queue++) {
struct stmmac_channel *ch = &priv->channel[queue];
- napi_enable(&ch->napi);
+ if (queue < rx_queues_cnt)
+ napi_enable(&ch->rx_napi);
+ if (queue < tx_queues_cnt)
+ napi_enable(&ch->tx_napi);
}
}
@@ -1939,6 +1945,10 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int budget, u32 queue)
mod_timer(&priv->eee_ctrl_timer, STMMAC_LPI_T(eee_timer));
}
+ /* We still have pending packets, let's call for a new scheduling */
+ if (tx_q->dirty_tx != tx_q->cur_tx)
+ mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(10));
+
__netif_tx_unlock_bh(netdev_get_tx_queue(priv->dev, queue));
return count;
@@ -2029,23 +2039,15 @@ static int stmmac_napi_check(struct stmmac_priv *priv, u32 chan)
int status = stmmac_dma_interrupt_status(priv, priv->ioaddr,
&priv->xstats, chan);
struct stmmac_channel *ch = &priv->channel[chan];
- bool needs_work = false;
- if ((status & handle_rx) && ch->has_rx) {
- needs_work = true;
- } else {
- status &= ~handle_rx;
- }
-
- if ((status & handle_tx) && ch->has_tx) {
- needs_work = true;
- } else {
- status &= ~handle_tx;
+ if ((status & handle_rx) && (chan < priv->plat->rx_queues_to_use)) {
+ stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
+ napi_schedule_irqoff(&ch->rx_napi);
}
- if (needs_work && napi_schedule_prep(&ch->napi)) {
+ if ((status & handle_tx) && (chan < priv->plat->tx_queues_to_use)) {
stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
- __napi_schedule(&ch->napi);
+ napi_schedule_irqoff(&ch->tx_napi);
}
return status;
@@ -2241,8 +2243,14 @@ static void stmmac_tx_timer(struct timer_list *t)
ch = &priv->channel[tx_q->queue_index];
- if (likely(napi_schedule_prep(&ch->napi)))
- __napi_schedule(&ch->napi);
+ /*
+ * If NAPI is already running we can miss some events. Let's rearm
+ * the timer and try again.
+ */
+ if (likely(napi_schedule_prep(&ch->tx_napi)))
+ __napi_schedule(&ch->tx_napi);
+ else
+ mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(10));
}
/**
@@ -3498,7 +3506,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
else
skb->ip_summed = CHECKSUM_UNNECESSARY;
- napi_gro_receive(&ch->napi, skb);
+ napi_gro_receive(&ch->rx_napi, skb);
priv->dev->stats.rx_packets++;
priv->dev->stats.rx_bytes += frame_len;
@@ -3513,41 +3521,43 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
return count;
}
-/**
- * stmmac_poll - stmmac poll method (NAPI)
- * @napi : pointer to the napi structure.
- * @budget : maximum number of packets that the current CPU can receive from
- * all interfaces.
- * Description :
- * To look at the incoming frames and clear the tx resources.
- */
-static int stmmac_napi_poll(struct napi_struct *napi, int budget)
+static int stmmac_napi_poll_rx(struct napi_struct *napi, int budget)
{
struct stmmac_channel *ch =
- container_of(napi, struct stmmac_channel, napi);
+ container_of(napi, struct stmmac_channel, rx_napi);
struct stmmac_priv *priv = ch->priv_data;
- int work_done, rx_done = 0, tx_done = 0;
u32 chan = ch->index;
+ int work_done;
priv->xstats.napi_poll++;
- if (ch->has_tx)
- tx_done = stmmac_tx_clean(priv, budget, chan);
- if (ch->has_rx)
- rx_done = stmmac_rx(priv, budget, chan);
+ work_done = stmmac_rx(priv, budget, chan);
+ if (work_done < budget && napi_complete_done(napi, work_done))
+ stmmac_enable_dma_irq(priv, priv->ioaddr, chan);
+ return work_done;
+}
- work_done = max(rx_done, tx_done);
- work_done = min(work_done, budget);
+static int stmmac_napi_poll_tx(struct napi_struct *napi, int budget)
+{
+ struct stmmac_channel *ch =
+ container_of(napi, struct stmmac_channel, tx_napi);
+ struct stmmac_priv *priv = ch->priv_data;
+ struct stmmac_tx_queue *tx_q;
+ u32 chan = ch->index;
+ int work_done;
+
+ priv->xstats.napi_poll++;
- if (work_done < budget && napi_complete_done(napi, work_done)) {
- int stat;
+ work_done = stmmac_tx_clean(priv, DMA_TX_SIZE, chan);
+ work_done = min(work_done, budget);
+ if (work_done < budget && napi_complete_done(napi, work_done))
stmmac_enable_dma_irq(priv, priv->ioaddr, chan);
- stat = stmmac_dma_interrupt_status(priv, priv->ioaddr,
- &priv->xstats, chan);
- if (stat && napi_reschedule(napi))
- stmmac_disable_dma_irq(priv, priv->ioaddr, chan);
- }
+
+ /* Force transmission restart */
+ tx_q = &priv->tx_queue[chan];
+ stmmac_enable_dma_transmission(priv, priv->ioaddr);
+ stmmac_set_tx_tail_ptr(priv, priv->ioaddr, tx_q->tx_tail_addr, chan);
return work_done;
}
@@ -4323,13 +4333,14 @@ int stmmac_dvr_probe(struct device *device,
ch->priv_data = priv;
ch->index = queue;
- if (queue < priv->plat->rx_queues_to_use)
- ch->has_rx = true;
- if (queue < priv->plat->tx_queues_to_use)
- ch->has_tx = true;
-
- netif_napi_add(ndev, &ch->napi, stmmac_napi_poll,
- NAPI_POLL_WEIGHT);
+ if (queue < priv->plat->rx_queues_to_use) {
+ netif_napi_add(ndev, &ch->rx_napi, stmmac_napi_poll_rx,
+ NAPI_POLL_WEIGHT);
+ }
+ if (queue < priv->plat->tx_queues_to_use) {
+ netif_napi_add(ndev, &ch->tx_napi, stmmac_napi_poll_tx,
+ NAPI_POLL_WEIGHT);
+ }
}
mutex_init(&priv->lock);
@@ -4385,7 +4396,10 @@ int stmmac_dvr_probe(struct device *device,
for (queue = 0; queue < maxq; queue++) {
struct stmmac_channel *ch = &priv->channel[queue];
- netif_napi_del(&ch->napi);
+ if (queue < priv->plat->rx_queues_to_use)
+ netif_napi_del(&ch->rx_napi);
+ if (queue < priv->plat->tx_queues_to_use)
+ netif_napi_del(&ch->tx_napi);
}
error_hw_init:
destroy_workqueue(priv->wq);
--
2.7.4
^ permalink raw reply related
* Re: stmmac / meson8b-dwmac
From: Jose Abreu @ 2019-02-18 8:45 UTC (permalink / raw)
To: Martin Blumenstingl, Jose Abreu
Cc: Simon Huelck, Emiliano Ingrassia, Gpeppe.cavallaro,
alexandre.torgue, linux-amlogic, netdev
In-Reply-To: <2abbf79a-cdff-b6a7-42de-79ea2c6ab367@synopsys.com>
On 2/18/2019 8:42 AM, Jose Abreu wrote:
> On 2/17/2019 2:48 PM, Martin Blumenstingl wrote:
>> Jose, is my command for disabling offloading correct?
>
> Yes, I think so. What about NIC statistics and logs ? ("ethtool
> -S eth0", "dmesg | grep -i stmmac")
And, "ifconfig eth0" after running the test.
>
> Thanks,
> Jose Miguel Abreu
>
^ permalink raw reply
* Re: stmmac / meson8b-dwmac
From: Jose Abreu @ 2019-02-18 8:42 UTC (permalink / raw)
To: Martin Blumenstingl, Jose Abreu
Cc: Simon Huelck, Emiliano Ingrassia, Gpeppe.cavallaro,
alexandre.torgue, linux-amlogic, netdev
In-Reply-To: <CAFBinCAJwk5Go4ZEt2cqVKyfeeOxsa-+3pfLpzQo=XNr5u4Shw@mail.gmail.com>
On 2/17/2019 2:48 PM, Martin Blumenstingl wrote:
> Jose, is my command for disabling offloading correct?
Yes, I think so. What about NIC statistics and logs ? ("ethtool
-S eth0", "dmesg | grep -i stmmac")
Thanks,
Jose Miguel Abreu
^ permalink raw reply
* NET_RX_DROP vs dev->stats.rx_dropped++ in drivers
From: Joakim Tjernlund @ 2019-02-18 8:30 UTC (permalink / raw)
To: netdev@vger.kernel.org
We have noticed odd dropped packets in a few cases and these comes from
drivers that do this:
...
dropped = netif_rx(sb);
if (dropped == NET_RX_DROP) {
dev->stats.rx_dropped++;
...
This seems a bit odd to me, should drivers account for dropped pkgs in this case?
Jocke
^ permalink raw reply
* Re: [PATCH 2/2] RTL8153-BD is used in Dell DA300 type-C dongle. It should be added to the whitelist of devices to activate MAC address pass through.
From: David Chen @ 2019-02-18 8:24 UTC (permalink / raw)
To: Oliver Neukum
Cc: linux-kernel, linux-usb, netdev, kai.heng.feng, davem,
david.chen7, mario.limonciello, f.fainelli, edumazet, zhongjiang,
bigeasy, hayeswang, jslaby
In-Reply-To: <1550477077.11088.2.camel@suse.com>
Hi Oliver,
Thanks for reviewing. I have made some mistake, will correct it and resend.
Thanks and Regards,
-David
> Oliver Neukum <oneukum@suse.com> 於 2019年2月18日 下午4:04 寫道：
>
>> On Mo, 2019-02-18 at 11:48 +0800, David Chen wrote:
>> From: David Chen <david.chen7@dell.com>
>>
>> Per confirming with Realtek all devices containing RTL8153-BD should
>> activate MAC pass through and there won't use pass through bit on efuse
>> like in RTL8153-AD.
>>
>> Signed-off-by: David Chen <david.chen7@dell.com>
>> ---
>> drivers/net/usb/r8152.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
>> index ada6baf8847a..86c8c64fbb0f 100644
>> --- a/drivers/net/usb/r8152.c
>> +++ b/drivers/net/usb/r8152.c
>> @@ -1179,7 +1179,7 @@ static int vendor_mac_passthru_addr_read(struct r8152 *tp, struct sockaddr *sa)
>> } else {
>> /* test for RTL8153-BND and RTL8153-BD */
>> ocp_data = ocp_read_byte(tp, MCU_TYPE_USB, USB_MISC_1);
>> - if ((ocp_data & BND_MASK) == 0 && (ocp_data & BD_MASK)) {
>> + if ((ocp_data & BND_MASK) == 0 && (ocp_data & BD_MASK) == 0) {
>
> You are inverting the second half of the test. How can this possibly be
> right? Had you dropped it, I would understand. But this? Are you sure?
>
> Regards
> Oliver
>
^ permalink raw reply
* Re: [PATCH bpf-next v4 0/2] libbpf: adding AF_XDP support
From: Magnus Karlsson @ 2019-02-18 8:20 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Jesper Dangaard Brouer, Jonathan Lemon, Magnus Karlsson,
Björn Töpel, ast, Network Development, Jakub Kicinski,
Björn Töpel, Zhang, Qi Z, xiaolong.ye,
xdp-newbies@vger.kernel.org
In-Reply-To: <20ba7719-b660-462c-a6bf-6c749e1f2f30@iogearbox.net>
On Fri, Feb 15, 2019 at 5:48 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 02/13/2019 12:55 PM, Jesper Dangaard Brouer wrote:
> > On Wed, 13 Feb 2019 12:32:47 +0100
> > Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
> >> On Mon, Feb 11, 2019 at 9:44 PM Jonathan Lemon <jonathan.lemon@gmail.com> wrote:
> >>> On 8 Feb 2019, at 5:05, Magnus Karlsson wrote:
> >>>
> >>>> This patch proposes to add AF_XDP support to libbpf. The main reason
> >>>> for this is to facilitate writing applications that use AF_XDP by
> >>>> offering higher-level APIs that hide many of the details of the AF_XDP
> >>>> uapi. This is in the same vein as libbpf facilitates XDP adoption by
> >>>> offering easy-to-use higher level interfaces of XDP
> >>>> functionality. Hopefully this will facilitate adoption of AF_XDP, make
> >>>> applications using it simpler and smaller, and finally also make it
> >>>> possible for applications to benefit from optimizations in the AF_XDP
> >>>> user space access code. Previously, people just copied and pasted the
> >>>> code from the sample application into their application, which is not
> >>>> desirable.
> >>>
> >>> I like the idea of encapsulating the boilerplate logic in a library.
> >>>
> >>> I do think there is an important missing piece though - there should be
> >>> some code which queries the netdev for how many queues are attached, and
> >>> create the appropriate number of umem/AF_XDP sockets.
> >>>
> >>> I ran into this issue when testing the current AF_XDP code - on my test
> >>> boxes, the mlx5 card has 55 channels (aka queues), so when the test program
> >>> binds only to channel 0, nothing works as expected, since not all traffic
> >>> is being intercepted. While obvious in hindsight, this took a while to
> >>> track down.
> >>
> >> Yes, agreed. You are not the first one to stumble upon this problem
> >> :-). Let me think a little bit on how to solve this in a good way. We
> >> need this to be simple and intuitive, as you say.
> >
> > I see people hitting this with AF_XDP all the time... I had some
> > backup-slides[2] in our FOSDEM presentation[1] that describe the issue,
> > give the performance reason why and propose a workaround.
>
> Magnus, I presume you're going to address this for the initial libbpf merge
> since the plan is to make it easier to consume for users?
I think the first thing we need is education and documentation. Have a
FAQ or "common mistakes" section in the Documentation. And of course,
sending Jesper around the world reminding people about this ;-).
To address this on a libbpf interface level, I think the best way is
to reprogram the NIC to send all traffic to the queue that you
provided in the xsk_socket__create call. This "set up NIC routing"
behavior can then be disable with a flag, just as the XDP program
loading can be disabled. The standard config of xsk_socket__create
will then set up as many things for the user as possible just to get
up and running quickly. More advanced users can then disable parts of
it to gain more flexibility. Does this sound OK? Do not want to go the
route of polling multiple sockets and aggregating the traffic as this
will have significant negative performance implications.
/Magnus
> Few more minor items in individual patches, will reply there.
>
> Thanks,
> Daniel
>
> > [1] https://github.com/xdp-project/xdp-project/tree/master/conference/FOSDEM2019
> > [2] https://github.com/xdp-project/xdp-project/blob/master/conference/FOSDEM2019/xdp_building_block.org#backup-slides
> >
> > Alternative work-around
> > * Create as many AF_XDP sockets as RXQs
> > * Have userspace poll()/select on all sockets
> >
>
^ permalink raw reply
* Re: [PATCH 2/2] RTL8153-BD is used in Dell DA300 type-C dongle. It should be added to the whitelist of devices to activate MAC address pass through.
From: Oliver Neukum @ 2019-02-18 8:04 UTC (permalink / raw)
To: David Chen, linux-kernel, linux-usb, netdev
Cc: kai.heng.feng, davem, david.chen7, mario.limonciello, f.fainelli,
edumazet, zhongjiang, bigeasy, hayeswang, jslaby
In-Reply-To: <20190218034817.7515-1-david0813@gmail.com>
On Mo, 2019-02-18 at 11:48 +0800, David Chen wrote:
> From: David Chen <david.chen7@dell.com>
>
> Per confirming with Realtek all devices containing RTL8153-BD should
> activate MAC pass through and there won't use pass through bit on efuse
> like in RTL8153-AD.
>
> Signed-off-by: David Chen <david.chen7@dell.com>
> ---
> drivers/net/usb/r8152.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index ada6baf8847a..86c8c64fbb0f 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -1179,7 +1179,7 @@ static int vendor_mac_passthru_addr_read(struct r8152 *tp, struct sockaddr *sa)
> } else {
> /* test for RTL8153-BND and RTL8153-BD */
> ocp_data = ocp_read_byte(tp, MCU_TYPE_USB, USB_MISC_1);
> - if ((ocp_data & BND_MASK) == 0 && (ocp_data & BD_MASK)) {
> + if ((ocp_data & BND_MASK) == 0 && (ocp_data & BD_MASK) == 0) {
You are inverting the second half of the test. How can this possibly be
right? Had you dropped it, I would understand. But this? Are you sure?
Regards
Oliver
^ permalink raw reply
* [PATCH net-next] yellowfin: fix remove set but not used variable warning
From: YueHaibing @ 2019-02-18 8:15 UTC (permalink / raw)
To: David S . Miller, Yang Wei; +Cc: YueHaibing, netdev, kernel-janitors
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/net/ethernet/packetengines/yellowfin.c: In function 'yellowfin_rx':
drivers/net/ethernet/packetengines/yellowfin.c:1053:18: warning:
variable 'yf_size' set but not used [-Wunused-but-set-variable]
This puts the variable declaration into the YF_PROTOTYPE macro.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/packetengines/yellowfin.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/packetengines/yellowfin.c b/drivers/net/ethernet/packetengines/yellowfin.c
index 6f8d6584f809..edc8cc7f0d0d 100644
--- a/drivers/net/ethernet/packetengines/yellowfin.c
+++ b/drivers/net/ethernet/packetengines/yellowfin.c
@@ -1050,8 +1050,11 @@ static int yellowfin_rx(struct net_device *dev)
struct sk_buff *rx_skb = yp->rx_skbuff[entry];
s16 frame_status;
u16 desc_status;
- int data_size, yf_size;
+ int data_size;
u8 *buf_addr;
+#ifdef YF_PROTOTYPE
+ int yf_size = sizeof(struct yellowfin_desc);
+#endif
if(!desc->result_status)
break;
@@ -1068,8 +1071,6 @@ static int yellowfin_rx(struct net_device *dev)
if (--boguscnt < 0)
break;
- yf_size = sizeof(struct yellowfin_desc);
-
if ( ! (desc_status & RX_EOP)) {
if (data_size != 0)
netdev_warn(dev, "Oversized Ethernet frame spanned multiple buffers, status %04x, data_size %d!\n",
^ permalink raw reply related
* [PATCH] brcmfmac: remove set but not used variable 'old_state'
From: YueHaibing @ 2019-02-18 8:08 UTC (permalink / raw)
To: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
Wright Feng, Kalle Valo
Cc: YueHaibing, linux-wireless, netdev, brcm80211-dev-list.pdl,
brcm80211-dev-list, kernel-janitors
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c: In function 'brcmf_usb_state_change':
drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c:578:6: warning:
variable 'old_state' set but not used [-Wunused-but-set-variable]
It's never used and can be removed.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
index 76cfaf6999c8..398536b30b7a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
@@ -575,7 +575,6 @@ static void
brcmf_usb_state_change(struct brcmf_usbdev_info *devinfo, int state)
{
struct brcmf_bus *bcmf_bus = devinfo->bus_pub.bus;
- int old_state;
brcmf_dbg(USB, "Enter, current state=%d, new state=%d\n",
devinfo->bus_pub.state, state);
@@ -583,7 +582,6 @@ brcmf_usb_state_change(struct brcmf_usbdev_info *devinfo, int state)
if (devinfo->bus_pub.state == state)
return;
- old_state = devinfo->bus_pub.state;
devinfo->bus_pub.state = state;
/* update state of upper layer */
^ permalink raw reply related
* [PATCH] rtl818x_pci: Remove set but not used variables 'io_addr, mem_addr'
From: YueHaibing @ 2019-02-18 7:52 UTC (permalink / raw)
To: Kalle Valo, John W. Linville, Colin Ian King
Cc: YueHaibing, linux-wireless, netdev, kernel-janitors
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c: In function 'rtl8180_probe':
drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c:1727:15: warning:
variable 'io_addr' set but not used [-Wunused-but-set-variable]
drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c:1726:16: warning:
variable 'mem_addr' set but not used [-Wunused-but-set-variable]
They're never used since introduction.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c b/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c
index e2b1bfbcfbd4..d5f65372356b 100644
--- a/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c
+++ b/drivers/net/wireless/realtek/rtl818x/rtl8180/dev.c
@@ -1723,8 +1723,8 @@ static int rtl8180_probe(struct pci_dev *pdev,
{
struct ieee80211_hw *dev;
struct rtl8180_priv *priv;
- unsigned long mem_addr, mem_len;
- unsigned int io_addr, io_len;
+ unsigned long mem_len;
+ unsigned int io_len;
int err;
const char *chip_name, *rf_name = NULL;
u32 reg;
@@ -1743,9 +1743,7 @@ static int rtl8180_probe(struct pci_dev *pdev,
goto err_disable_dev;
}
- io_addr = pci_resource_start(pdev, 0);
io_len = pci_resource_len(pdev, 0);
- mem_addr = pci_resource_start(pdev, 1);
mem_len = pci_resource_len(pdev, 1);
if (mem_len < sizeof(struct rtl818x_csr) ||
^ permalink raw reply related
* [PATCH] rsi: remove set but not used variables 'info, vif'
From: YueHaibing @ 2019-02-18 7:51 UTC (permalink / raw)
To: Amitkumar Karwar, Siva Rebbagondla, Kalle Valo
Cc: YueHaibing, linux-wireless, netdev, kernel-janitors
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/net/wireless/rsi/rsi_91x_main.c: In function 'rsi_prepare_skb':
drivers/net/wireless/rsi/rsi_91x_main.c:127:24: warning:
variable 'vif' set but not used [-Wunused-but-set-variable]
drivers/net/wireless/rsi/rsi_91x_main.c:124:28: warning:
variable 'info' set but not used [-Wunused-but-set-variable]
They're not used any more since 160ee2a11ce0 ("rsi: fill rx_params only once.")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/wireless/rsi/rsi_91x_main.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/net/wireless/rsi/rsi_91x_main.c b/drivers/net/wireless/rsi/rsi_91x_main.c
index ca3a55ed72e4..29d83049c5f5 100644
--- a/drivers/net/wireless/rsi/rsi_91x_main.c
+++ b/drivers/net/wireless/rsi/rsi_91x_main.c
@@ -121,11 +121,8 @@ static struct sk_buff *rsi_prepare_skb(struct rsi_common *common,
u32 pkt_len,
u8 extended_desc)
{
- struct ieee80211_tx_info *info;
struct sk_buff *skb = NULL;
u8 payload_offset;
- struct ieee80211_vif *vif;
- struct ieee80211_hdr *wh;
if (WARN(!pkt_len, "%s: Dummy pkt received", __func__))
return NULL;
@@ -144,10 +141,7 @@ static struct sk_buff *rsi_prepare_skb(struct rsi_common *common,
payload_offset = (extended_desc + FRAME_DESC_SZ);
skb_put(skb, pkt_len);
memcpy((skb->data), (buffer + payload_offset), skb->len);
- wh = (struct ieee80211_hdr *)skb->data;
- vif = rsi_get_vif(common->priv, wh->addr1);
- info = IEEE80211_SKB_CB(skb);
return skb;
}
^ permalink raw reply related
* [PATCH net-next] mlxsw: spectrum: Change IP2ME CPU policer rate and burst size values
From: Ido Schimmel @ 2019-02-18 7:19 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: davem@davemloft.net, Jiri Pirko, mlxsw, Shalom Toledo,
Ido Schimmel
From: Shalom Toledo <shalomt@mellanox.com>
The IP2ME packet trap is triggered by packets hitting local routes.
After evaluating current defaults used by the driver it was decided to
reduce the amount of traffic generated by this trap to 1Kpps and
increase the burst size. This is inline with similarly deployed systems.
Signed-off-by: Shalom Toledo <shalomt@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index abac923a8d04..bc349d8ca08a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3739,8 +3739,8 @@ static int mlxsw_sp_cpu_policers_set(struct mlxsw_core *mlxsw_core)
burst_size = 7;
break;
case MLXSW_REG_HTGT_TRAP_GROUP_SP_IP2ME:
- rate = 4 * 1024;
- burst_size = 4;
+ rate = 1024;
+ burst_size = 7;
break;
default:
continue;
--
2.20.1
^ permalink raw reply related
* Re: [RFC PATCH] bonding: use mutex lock in bond_get_stats()
From: Kefeng Wang @ 2019-02-18 6:39 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, Willem de Bruijn, David S . Miller, Jay Vosburgh,
Veaceslav Falico, weiyongjun1
In-Reply-To: <CANn89iKXfrfsoYAvbq0Rwx+5FtBgQf0zYU3WAt2v_tpbS59how@mail.gmail.com>
On 2019/2/17 2:18, Eric Dumazet wrote:
> On Fri, Feb 15, 2019 at 9:36 PM Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>>
>>
>> On 2019/2/15 21:57, Eric Dumazet wrote:
>>> On Fri, Feb 15, 2019 at 5:37 AM Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>>>> With CONFIG_DEBUG_SPINLOCK=y, we find following stack,
>>>>
>>>> BUG: spinlock wrong CPU on CPU#0, ip/16047
>>>> lock: 0xffff803f5febc998, .magic: dead4ead, .owner: ip/16047, .owner_cpu: 0
>>>> CPU: 1 PID: 16047 Comm: ip Kdump: loaded Tainted: G E 4.19.12.aarch64 #1
>>>> Hardware name: Huawei TaiShan 2280 V2/BC82AMDA, BIOS TA BIOS TaiShan 2280 V2 - B900 01/29/2019
>>>> Call trace:
>>>> dump_backtrace+0x0/0x1c0
>>>> show_stack+0x24/0x30
>>>> dump_stack+0x90/0xbc
>>>> spin_dump+0x84/0xa8
>>>> do_raw_spin_unlock+0xf8/0x100
>>>> _raw_spin_unlock+0x20/0x30
>>>> bond_get_stats+0x110/0x140 [bonding]
>>>> rtnl_fill_stats+0x50/0x150
>>>> rtnl_fill_ifinfo+0x4d4/0xd18
>>>> rtnl_dump_ifinfo+0x200/0x3a8
>>>> netlink_dump+0x100/0x2b0
>>>> netlink_recvmsg+0x310/0x3e8
>>>> sock_recvmsg+0x58/0x68
>>>> ___sys_recvmsg+0xd0/0x278
>>>> __sys_recvmsg+0x74/0xd0
>>>> __arm64_sys_recvmsg+0x2c/0x38
>>>> el0_svc_common+0x7c/0x118
>>>> el0_svc_handler+0x30/0x40
>>>> el0_svc+0x8/0xc
>>>>
>>>> and then lead to softlockup issue, fix this by using mutex lock instead
>>>> of spin lock.
>>>>
>>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>>> ---
>>>>
>>>> Not sure if this is right fix, please correct me if I'm wrong.
>>>>
>>> Make sure to also try :
>>>
>>> cat /proc/net/dev
>> Yes, no regression with this patch in our test or 'cat /proc/net/dev'
>>
> You really should have a warning showing up.
>
> Make sure you have : DEBUG_ATOMIC_SLEEP=y (I thought this was
> automatically selected with LOCKDEP on, maybe not...)
>
> Please take a look at net/core/net-procfs.c , functions
> dev_seq_start(), dev_seq_printf_stats(), dev_seq_show()
>
> We can not use a mutex in a section protected by rcu_read_lock()
ok, thanks for your direction, we will do more research and test.
> .
>
^ permalink raw reply
* [PATCH] net: hamradio: remove unused hweight*() defines
From: Masahiro Yamada @ 2019-02-18 6:35 UTC (permalink / raw)
To: David S . Miller, netdev
Cc: Masahiro Yamada, linux-hams, Thomas Sailer, linux-kernel
This file does not use hweight*() at all, and the definition is
surrounded by #if 0 ... #endif.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
drivers/net/hamradio/baycom_ser_fdx.c | 26 --------------------------
1 file changed, 26 deletions(-)
diff --git a/drivers/net/hamradio/baycom_ser_fdx.c b/drivers/net/hamradio/baycom_ser_fdx.c
index 190f66c..ed08416 100644
--- a/drivers/net/hamradio/baycom_ser_fdx.c
+++ b/drivers/net/hamradio/baycom_ser_fdx.c
@@ -203,32 +203,6 @@ static inline void ser12_set_divisor(struct net_device *dev,
*/
}
-/* --------------------------------------------------------------------- */
-
-#if 0
-static inline unsigned int hweight16(unsigned int w)
- __attribute__ ((unused));
-static inline unsigned int hweight8(unsigned int w)
- __attribute__ ((unused));
-
-static inline unsigned int hweight16(unsigned int w)
-{
- unsigned short res = (w & 0x5555) + ((w >> 1) & 0x5555);
- res = (res & 0x3333) + ((res >> 2) & 0x3333);
- res = (res & 0x0F0F) + ((res >> 4) & 0x0F0F);
- return (res & 0x00FF) + ((res >> 8) & 0x00FF);
-}
-
-static inline unsigned int hweight8(unsigned int w)
-{
- unsigned short res = (w & 0x55) + ((w >> 1) & 0x55);
- res = (res & 0x33) + ((res >> 2) & 0x33);
- return (res & 0x0F) + ((res >> 4) & 0x0F);
-}
-#endif
-
-/* --------------------------------------------------------------------- */
-
static __inline__ void ser12_rx(struct net_device *dev, struct baycom_state *bc, struct timespec64 *ts, unsigned char curs)
{
int timediff;
--
2.7.4
^ permalink raw reply related
* [PATCH 1/2] wireless: mt76: call hweight8() instead of __sw_hweight8()
From: Masahiro Yamada @ 2019-02-18 5:59 UTC (permalink / raw)
To: Ingo Molnar, Thomas Gleixner, Borislav Petkov, H . Peter Anvin,
x86
Cc: Kalle Valo, linux-wireless, Christoph Hellwig, Masahiro Yamada,
David S. Miller, netdev, linux-kernel, Lorenzo Bianconi,
linux-mediatek, linux-arm-kernel, Felix Fietkau, Matthias Brugger
In-Reply-To: <1550469571-25933-1-git-send-email-yamada.masahiro@socionext.com>
__sw_hweight8() is just internal implementation.
Drivers should use the common API, hweight8().
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
This patch should go to x86 tree along with 2/2.
Otherwise, all{yes,mod}config of x86 would be broken.
This patch is trivial enough.
I want ACK from the net/wireless maintainer
so that this can go in via x86 tree.
drivers/net/wireless/mediatek/mt76/mac80211.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c
index 7b926df..c42e0d3 100644
--- a/drivers/net/wireless/mediatek/mt76/mac80211.c
+++ b/drivers/net/wireless/mediatek/mt76/mac80211.c
@@ -124,7 +124,7 @@ static void mt76_init_stream_cap(struct mt76_dev *dev,
bool vht)
{
struct ieee80211_sta_ht_cap *ht_cap = &sband->ht_cap;
- int i, nstream = __sw_hweight8(dev->antenna_mask);
+ int i, nstream = hweight8(dev->antenna_mask);
struct ieee80211_sta_vht_cap *vht_cap;
u16 mcs_map = 0;
--
2.7.4
^ permalink raw reply related
* [PATCH 0/2] x86: turn off wrongly enabled CONFIG_GENERIC_HWEIGHT
From: Masahiro Yamada @ 2019-02-18 5:59 UTC (permalink / raw)
To: Ingo Molnar, Thomas Gleixner, Borislav Petkov, H . Peter Anvin,
x86
Cc: Kalle Valo, linux-wireless, Christoph Hellwig, Masahiro Yamada,
David S. Miller, netdev, linux-kernel, Lorenzo Bianconi,
linux-mediatek, linux-arm-kernel, Felix Fietkau, Matthias Brugger
x86 should not enable CONFIG_GENERIC_HWEIGHT
because lib/hweight.c is not used by x86.
The only user is drivers/net/wireless/mediatek/mt76/mac80211.c
Of course, this driver is wrong since drivers should use the
generic API, hweight8().
After fixing it, x86 does not need to compile lib/hweight.c at all.
The real implementation is located in arch/x86/lib/hweight.S
Masahiro Yamada (2):
wireless: mt76: call hweight8() instead of __sw_hweight8()
x86: disable CONFIG_GENERIC_HWEIGHT and remove __HAVE_ARCH_SW_HWEIGHT
arch/x86/Kconfig | 3 ---
arch/x86/include/asm/arch_hweight.h | 2 --
drivers/net/wireless/mediatek/mt76/mac80211.c | 2 +-
lib/hweight.c | 4 ----
4 files changed, 1 insertion(+), 10 deletions(-)
--
2.7.4
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox