* Re: [PATCH v3 1/7] dt-bindings: net: Add bindings for Realtek PHYs
From: Andrew Lunn @ 2019-07-08 21:13 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: David S . Miller, Rob Herring, Mark Rutland, Florian Fainelli,
Heiner Kallweit, netdev, devicetree, linux-kernel,
Douglas Anderson
In-Reply-To: <20190708200136.GM250418@google.com>
On Mon, Jul 08, 2019 at 01:01:36PM -0700, Matthias Kaehlcke wrote:
> On Mon, Jul 08, 2019 at 09:46:15PM +0200, Andrew Lunn wrote:
> > On Mon, Jul 08, 2019 at 12:24:53PM -0700, Matthias Kaehlcke wrote:
> > > Add the 'realtek,eee-led-mode-disable' property to disable EEE
> > > LED mode on Realtek PHYs that support it.
> > >
> > > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > > ---
> > > TODO: adapt PHY core to deal with optional compatible strings
> >
> > Yes. Does this even work at the moment? I would expect
> > of_mdiobus_child_is_phy() to return false, indicating the device is
> > not actually a PHY.
>
> Indeed, it currently doesn't work atm. I found that removing the check
> for dev->of_node in of_mdiobus_link_mdiodev() helps, but I imagine
> doing (only) this might have undesired side-effects.
O.K.
Please put RFC in patches like this which don't actually work and so
should not be merged. We don't want David accidentally merging them!
Andrew
^ permalink raw reply
* Re: [PATCH] tipc: ensure skb->lock is initialised
From: Chris Packham @ 2019-07-08 21:13 UTC (permalink / raw)
To: Eric Dumazet, jon.maloy@ericsson.com, ying.xue@windriver.com,
davem@davemloft.net
Cc: netdev@vger.kernel.org, tipc-discussion@lists.sourceforge.net,
linux-kernel@vger.kernel.org
In-Reply-To: <361940337b0d4833a5634ddd1e1896a9@svr-chch-ex1.atlnz.lc>
On 9/07/19 8:43 AM, Chris Packham wrote:
> On 8/07/19 8:18 PM, Eric Dumazet wrote:
>>
>>
>> On 7/8/19 12:53 AM, Chris Packham wrote:
>>> tipc_named_node_up() creates a skb list. It passes the list to
>>> tipc_node_xmit() which has some code paths that can call
>>> skb_queue_purge() which relies on the list->lock being initialised.
>>> Ensure tipc_named_node_up() uses skb_queue_head_init() so that the lock
>>> is explicitly initialised.
>>>
>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>>
>> I would rather change the faulty skb_queue_purge() to __skb_queue_purge()
>>
>
> Makes sense. I'll look at that for v2.
>
Actually maybe not. tipc_rcast_xmit(), tipc_node_xmit_skb(),
tipc_send_group_msg(), __tipc_sendmsg(), __tipc_sendstream(), and
tipc_sk_timeout() all use skb_queue_head_init(). So my original change
brings tipc_named_node_up() into line with them.
I think it should be safe for tipc_node_xmit() to use
__skb_queue_purge() since all the callers seem to have exclusive access
to the list of skbs. It still seems that the callers should all use
skb_queue_head_init() for consistency.
^ permalink raw reply
* Re: [PATCH 1/2] forcedeth: add recv cache make nic work steadily
From: Jakub Kicinski @ 2019-07-08 20:52 UTC (permalink / raw)
To: Zhu Yanjun; +Cc: netdev, davem
In-Reply-To: <1562307568-21549-2-git-send-email-yanjun.zhu@oracle.com>
On Fri, 5 Jul 2019 02:19:27 -0400, Zhu Yanjun wrote:
> A recv cache is added. The size of recv cache is 1000Mb / skb_length.
> When the system memory is not enough, this recv cache can make nic work
> steadily.
> When nic is up, this recv cache and work queue are created. When nic
> is down, this recv cache will be destroyed and delayed workqueue is
> canceled.
> When nic is polled or rx interrupt is triggerred, rx handler will
> get a skb from recv cache. Then the state of recv cache is checked.
> If recv cache is not in filling up state, a work is queued to fill
> up recv cache.
> When skb size is changed, the old recv cache is destroyed and new recv
> cache is created.
> When the system memory is not enough, the allocation of skb failed.
> recv cache will continue allocate skb until the recv cache is filled up.
> When the system memory is not enough, this can make nic work steadily.
> Becase of recv cache, the performance of nic is enhanced.
>
> CC: Joe Jin <joe.jin@oracle.com>
> CC: Junxiao Bi <junxiao.bi@oracle.com>
> Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
Could you tell us a little more about the use case and the system
condition?
> diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
> index b327b29..a673005 100644
> --- a/drivers/net/ethernet/nvidia/forcedeth.c
> +++ b/drivers/net/ethernet/nvidia/forcedeth.c
> @@ -674,6 +674,11 @@ struct nv_ethtool_stats {
> u64 tx_broadcast;
> };
>
> +/* 1000Mb is 125M bytes, 125 * 1024 * 1024 bytes
> + * The length of recv cache is 125M / skb_length
> + */
> +#define RECV_CACHE_LIST_LENGTH (125 * 1024 * 1024 / np->rx_buf_sz)
> +
> #define NV_DEV_STATISTICS_V3_COUNT (sizeof(struct nv_ethtool_stats)/sizeof(u64))
> #define NV_DEV_STATISTICS_V2_COUNT (NV_DEV_STATISTICS_V3_COUNT - 3)
> #define NV_DEV_STATISTICS_V1_COUNT (NV_DEV_STATISTICS_V2_COUNT - 6)
> @@ -844,8 +849,18 @@ struct fe_priv {
> char name_rx[IFNAMSIZ + 3]; /* -rx */
> char name_tx[IFNAMSIZ + 3]; /* -tx */
> char name_other[IFNAMSIZ + 6]; /* -other */
> +
> + /* This is to schedule work */
> + struct delayed_work recv_cache_work;
> + /* This list is to store skb queue for recv */
> + struct sk_buff_head recv_list;
> + unsigned long nv_recv_list_state;
> };
>
> +/* This is recv list state to fill up recv cache */
> +enum recv_list_state {
> + RECV_LIST_ALLOCATE
> +};
> /*
> * Maximum number of loops until we assume that a bit in the irq mask
> * is stuck. Overridable with module param.
> @@ -1804,7 +1819,11 @@ static int nv_alloc_rx(struct net_device *dev)
> less_rx = np->last_rx.orig;
>
> while (np->put_rx.orig != less_rx) {
> - struct sk_buff *skb = netdev_alloc_skb(dev, np->rx_buf_sz + NV_RX_ALLOC_PAD);
> + struct sk_buff *skb = skb_dequeue(&np->recv_list);
> +
> + if (!test_bit(RECV_LIST_ALLOCATE, &np->nv_recv_list_state))
> + schedule_delayed_work(&np->recv_cache_work, 0);
Interesting, this seems to be coming up in multiple places recently..
Could you explain why you have your own RECV_LIST_ALLOCATE bit here?
Workqueue implementation itself uses an atomic bit to avoid scheduling
work mutliple times:
bool queue_delayed_work_on(int cpu, struct workqueue_struct *wq,
struct delayed_work *dwork, unsigned long delay)
{
struct work_struct *work = &dwork->work;
bool ret = false;
unsigned long flags;
/* read the comment in __queue_work() */
local_irq_save(flags);
if (!test_and_set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(work))) {
__queue_delayed_work(cpu, wq, dwork, delay);
ret = true;
}
local_irq_restore(flags);
return ret;
}
EXPORT_SYMBOL(queue_delayed_work_on);
> if (likely(skb)) {
> np->put_rx_ctx->skb = skb;
> np->put_rx_ctx->dma = dma_map_single(&np->pci_dev->dev,
> @@ -1845,7 +1864,11 @@ static int nv_alloc_rx_optimized(struct net_device *dev)
> less_rx = np->last_rx.ex;
>
> while (np->put_rx.ex != less_rx) {
> - struct sk_buff *skb = netdev_alloc_skb(dev, np->rx_buf_sz + NV_RX_ALLOC_PAD);
> + struct sk_buff *skb = skb_dequeue(&np->recv_list);
> +
> + if (!test_bit(RECV_LIST_ALLOCATE, &np->nv_recv_list_state))
> + schedule_delayed_work(&np->recv_cache_work, 0);
It seems a little heavy to schedule this work on every packet, would it
make sense to add this in nv_napi_poll() instead?
> if (likely(skb)) {
> np->put_rx_ctx->skb = skb;
> np->put_rx_ctx->dma = dma_map_single(&np->pci_dev->dev,
> @@ -1957,6 +1980,40 @@ static void nv_init_tx(struct net_device *dev)
> }
> }
>
> +static void nv_init_recv_cache(struct net_device *dev)
> +{
> + struct fe_priv *np = netdev_priv(dev);
> +
> + skb_queue_head_init(&np->recv_list);
> + while (skb_queue_len(&np->recv_list) < RECV_CACHE_LIST_LENGTH) {
> + struct sk_buff *skb = netdev_alloc_skb(dev,
> + np->rx_buf_sz + NV_RX_ALLOC_PAD);
> + /* skb is null. This indicates that memory is not
> + * enough.
> + */
> + if (unlikely(!skb)) {
> + ndelay(3);
> + continue;
Does this path ever hit? Seems like doing an ndelay() and retrying
allocation is not the best idea for non-preempt kernel.
Also perhaps you should consider using __netdev_alloc_skb() and passing
GFP_KERNEL, that way the system has a chance to go into memory reclaim
(I presume this function can sleep).
> + }
> +
> + skb_queue_tail(&np->recv_list, skb);
> + }
> +}
> +
> +static void nv_destroy_recv_cache(struct net_device *dev)
> +{
> + struct sk_buff *skb;
> + struct fe_priv *np = netdev_priv(dev);
> +
> + cancel_delayed_work_sync(&np->recv_cache_work);
> + WARN_ON(delayed_work_pending(&np->recv_cache_work));
> +
> + while ((skb = skb_dequeue(&np->recv_list)))
> + kfree_skb(skb);
skb_queue_purge()
> + WARN_ON(skb_queue_len(&np->recv_list));
> +}
> +
> static int nv_init_ring(struct net_device *dev)
> {
> struct fe_priv *np = netdev_priv(dev);
> @@ -3047,6 +3104,8 @@ static int nv_change_mtu(struct net_device *dev, int new_mtu)
> nv_drain_rxtx(dev);
> /* reinit driver view of the rx queue */
> set_bufsize(dev);
> + nv_destroy_recv_cache(dev);
> + nv_init_recv_cache(dev);
> if (nv_init_ring(dev)) {
> if (!np->in_shutdown)
> mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
> @@ -4074,6 +4133,32 @@ static void nv_free_irq(struct net_device *dev)
> }
> }
>
> +static void nv_recv_cache_worker(struct work_struct *work)
> +{
> + struct fe_priv *np = container_of(work, struct fe_priv,
> + recv_cache_work.work);
> +
> + set_bit(RECV_LIST_ALLOCATE, &np->nv_recv_list_state);
> + while (skb_queue_len(&np->recv_list) < RECV_CACHE_LIST_LENGTH) {
> + struct sk_buff *skb = netdev_alloc_skb(np->dev,
> + np->rx_buf_sz + NV_RX_ALLOC_PAD);
> + /* skb is null. This indicates that memory is not
> + * enough.
> + * When the system memory is not enough, the kernel
> + * will compact memory or drop caches. At that time,
> + * if memory allocation fails, it had better wait some
> + * time for memory.
> + */
> + if (unlikely(!skb)) {
> + ndelay(3);
> + continue;
Same comments as for the init function.
> + }
> +
> + skb_queue_tail(&np->recv_list, skb);
> + }
> + clear_bit(RECV_LIST_ALLOCATE, &np->nv_recv_list_state);
> +}
> +
> static void nv_do_nic_poll(struct timer_list *t)
> {
> struct fe_priv *np = from_timer(np, t, nic_poll);
> @@ -4129,6 +4214,8 @@ static void nv_do_nic_poll(struct timer_list *t)
> nv_drain_rxtx(dev);
> /* reinit driver view of the rx queue */
> set_bufsize(dev);
> + nv_destroy_recv_cache(dev);
> + nv_init_recv_cache(dev);
> if (nv_init_ring(dev)) {
> if (!np->in_shutdown)
> mod_timer(&np->oom_kick, jiffies + OOM_REFILL);
^ permalink raw reply
* Re: kernel BUG at lib/lockref.c:LINE!
From: syzbot @ 2019-07-08 20:50 UTC (permalink / raw)
To: arvid.brodin, darrick.wong, davem, linux-fsdevel, linux-kernel,
linux-xfs, netdev, sfr, syzkaller-bugs, viro
In-Reply-To: <000000000000b519af058d3091d1@google.com>
syzbot has bisected this bug to:
commit 867c90eeea9d81ad1336881b61a4dcf692fc5d50
Author: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon Jul 8 00:22:38 2019 +0000
Merge remote-tracking branch 'xfs/for-next'
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=16c69bc3a00000
start commit: d58b5ab9 Add linux-next specific files for 20190708
git tree: linux-next
final crash: https://syzkaller.appspot.com/x/report.txt?x=15c69bc3a00000
console output: https://syzkaller.appspot.com/x/log.txt?x=11c69bc3a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=bf9882946ecc11d9
dashboard link: https://syzkaller.appspot.com/bug?extid=f70e9b00f8c7d4187bd0
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=173375c7a00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1536f9bfa00000
Reported-by: syzbot+f70e9b00f8c7d4187bd0@syzkaller.appspotmail.com
Fixes: 867c90eeea9d ("Merge remote-tracking branch 'xfs/for-next'")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
* Re: [PATCH] tipc: ensure skb->lock is initialised
From: Chris Packham @ 2019-07-08 20:43 UTC (permalink / raw)
To: Eric Dumazet, jon.maloy@ericsson.com, ying.xue@windriver.com,
davem@davemloft.net
Cc: netdev@vger.kernel.org, tipc-discussion@lists.sourceforge.net,
linux-kernel@vger.kernel.org
In-Reply-To: <2298b9eb-100f-6130-60c4-0e5e2c7b84d1@gmail.com>
On 8/07/19 8:18 PM, Eric Dumazet wrote:
>
>
> On 7/8/19 12:53 AM, Chris Packham wrote:
>> tipc_named_node_up() creates a skb list. It passes the list to
>> tipc_node_xmit() which has some code paths that can call
>> skb_queue_purge() which relies on the list->lock being initialised.
>> Ensure tipc_named_node_up() uses skb_queue_head_init() so that the lock
>> is explicitly initialised.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>
> I would rather change the faulty skb_queue_purge() to __skb_queue_purge()
>
Makes sense. I'll look at that for v2.
^ permalink raw reply
* Re: [PATCH ghak90 V6 02/10] audit: add container id
From: Paul Moore @ 2019-07-08 20:43 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: Tycho Andersen, Serge E. Hallyn, containers, linux-api,
Linux-Audit Mailing List, linux-fsdevel, LKML, netdev,
netfilter-devel, sgrubb, omosnace, dhowells, simo, Eric Paris,
ebiederm, nhorman
In-Reply-To: <20190708181237.5poheliito7zpvmc@madcap2.tricolour.ca>
On July 8, 2019 8:12:56 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2019-05-30 19:26, Paul Moore wrote:
>> On Thu, May 30, 2019 at 5:29 PM Tycho Andersen <tycho@tycho.ws> wrote:
>>> On Thu, May 30, 2019 at 03:29:32PM -0400, Paul Moore wrote:
>>>>
>>>>
>>>> [REMINDER: It is an "*audit* container ID" and not a general
>>>> "container ID" ;) Smiley aside, I'm not kidding about that part.]
>>>
>>> This sort of seems like a distinction without a difference; presumably
>>> audit is going to want to differentiate between everything that people
>>> in userspace call a container. So you'll have to support all this
>>> insanity anyway, even if it's "not a container ID".
>>
>> That's not quite right. Audit doesn't care about what a container is,
>> or is not, it also doesn't care if the "audit container ID" actually
>> matches the ID used by the container engine in userspace and I think
>> that is a very important line to draw. Audit is simply given a value
>> which it calls the "audit container ID", it ensures that the value is
>> inherited appropriately (e.g. children inherit their parent's audit
>> container ID), and it uses the value in audit records to provide some
>> additional context for log analysis. The distinction isn't limited to
>> the value itself, but also to how it is used; it is an "audit
>> container ID" and not a "container ID" because this value is
>> exclusively for use by the audit subsystem. We are very intentionally
>> not adding a generic container ID to the kernel. If the kernel does
>> ever grow a general purpose container ID we will be one of the first
>> ones in line to make use of it, but we are not going to be the ones to
>> generically add containers to the kernel. Enough people already hate
>> audit ;)
>>
>>>> I'm not interested in supporting/merging something that isn't useful;
>>>> if this doesn't work for your use case then we need to figure out what
>>>> would work. It sounds like nested containers are much more common in
>>>> the lxc world, can you elaborate a bit more on this?
>>>>
>>>>
>>>> As far as the possible solutions you mention above, I'm not sure I
>>>> like the per-userns audit container IDs, I'd much rather just emit the
>>>> necessary tracking information via the audit record stream and let the
>>>> log analysis tools figure it out. However, the bigger question is how
>>>> to limit (re)setting the audit container ID when you are in a non-init
>>>> userns. For reasons already mentioned, using capable() is a non
>>>> starter for everything but the initial userns, and using ns_capable()
>>>> is equally poor as it essentially allows any userns the ability to
>>>> munge it's audit container ID (obviously not good). It appears we
>>>> need a different method for controlling access to the audit container
>>>> ID.
>>>
>>> One option would be to make it a string, and have it be append only.
>>> That should be safe with no checks.
>>>
>>> I know there was a long thread about what type to make this thing. I
>>> think you could accomplish the append-only-ness with a u64 if you had
>>> some rule about only allowing setting lower order bits than those that
>>> are already set. With 4 bits for simplicity:
>>>
>>> 1100 # initial container id
>>> 1100 -> 1011 # not allowed
>>> 1100 -> 1101 # allowed, but now 1101 is set in stone since there are
>>> # no lower order bits left
>>>
>>> There are probably fancier ways to do it if you actually understand
>>> math :)
>>
>> ;)
>>
>>> Since userns nesting is limited to 32 levels (right now, IIRC), and
>>> you have 64 bits, this might be reasonable. You could just teach
>>> container engines to use the first say N bits for themselves, with a 1
>>> bit for the barrier at the end.
>>
>> I like the creativity, but I worry that at some point these
>> limitations are going to be raised (limits have a funny way of doing
>> that over time) and we will be in trouble. I say "trouble" because I
>> want to be able to quickly do an audit container ID comparison and
>> we're going to pay a penalty for these larger values (we'll need this
>> when we add multiple auditd support and the requisite record routing).
>>
>> Thinking about this makes me also realize we probably need to think a
>> bit longer about audit container ID conflicts between orchestrators.
>> Right now we just take the value that is given to us by the
>> orchestrator, but if we want to allow multiple container orchestrators
>> to work without some form of cooperation in userspace (I think we have
>> to assume the orchestrators will not talk to each other) we likely
>> need to have some way to block reuse of an audit container ID. We
>> would either need to prevent the orchestrator from explicitly setting
>> an audit container ID to a currently in use value, or instead generate
>> the audit container ID in the kernel upon an event triggered by the
>> orchestrator (e.g. a write to a /proc file). I suspect we should
>> start looking at the idr code, I think we will need to make use of it.
>
> To address this, I'd suggest that it is enforced to only allow the
> setting of descendants and to maintain a master list of audit container
> identifiers (with a hash table if necessary later) that includes the
> container owner.
>
> This also allows the orchestrator/engine to inject processes into
> existing containers by checking that the audit container identifier is
> only used again by the same owner.
>
> I have working code for both.
Just a quick note that due to some holiday travel I'm not going to be able to adequately respond to your latest messages on this thread for at least another week, likely a bit more. I'm only checking mail to put out fires, and the audit container ID work tends to be something that starts them ;)
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf, libbpf: add a new API bpf_object__reuse_maps()
From: Anton Protopopov @ 2019-07-08 20:37 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Daniel Borkmann, Alexei Starovoitov, Martin KaFai Lau, Song Liu,
Yonghong Song, Networking, bpf, open list, Andrii Nakryiko
In-Reply-To: <CAEf4BzaGGVv2z8jB8MnT7=gnn4nG0cp7DGYxfnnnpohOT=ujCA@mail.gmail.com>
пн, 8 июл. 2019 г. в 13:54, Andrii Nakryiko <andrii.nakryiko@gmail.com>:
>
> On Fri, Jul 5, 2019 at 2:53 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> > On 07/05/2019 10:44 PM, Anton Protopopov wrote:
> > > Add a new API bpf_object__reuse_maps() which can be used to replace all maps in
> > > an object by maps pinned to a directory provided in the path argument. Namely,
> > > each map M in the object will be replaced by a map pinned to path/M.name.
> > >
> > > Signed-off-by: Anton Protopopov <a.s.protopopov@gmail.com>
> > > ---
> > > tools/lib/bpf/libbpf.c | 34 ++++++++++++++++++++++++++++++++++
> > > tools/lib/bpf/libbpf.h | 2 ++
> > > tools/lib/bpf/libbpf.map | 1 +
> > > 3 files changed, 37 insertions(+)
> > >
> > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > > index 4907997289e9..84c9e8f7bfd3 100644
> > > --- a/tools/lib/bpf/libbpf.c
> > > +++ b/tools/lib/bpf/libbpf.c
> > > @@ -3144,6 +3144,40 @@ int bpf_object__unpin_maps(struct bpf_object *obj, const char *path)
> > > return 0;
> > > }
> > >
> > > +int bpf_object__reuse_maps(struct bpf_object *obj, const char *path)
>
> As is, bpf_object__reuse_maps() can be easily implemented by user
> applications, as it's only using public libbpf APIs, so I'm not 100%
> sure we need to add method like that to libbpf.
The bpf_object__reuse_maps() can definitely be implemented by user
applications, however, to use it a user also needs to re-implement the
bpf_prog_load_xattr funciton, so it seemed to me that adding this
functionality to the library is a better way.
>
> > > +{
> > > + struct bpf_map *map;
> > > +
> > > + if (!obj)
> > > + return -ENOENT;
> > > +
> > > + if (!path)
> > > + return -EINVAL;
> > > +
> > > + bpf_object__for_each_map(map, obj) {
> > > + int len, err;
> > > + int pinned_map_fd;
> > > + char buf[PATH_MAX];
> >
> > We'd need to skip the case of bpf_map__is_internal(map) since they are always
> > recreated for the given object.
> >
> > > + len = snprintf(buf, PATH_MAX, "%s/%s", path, bpf_map__name(map));
> > > + if (len < 0) {
> > > + return -EINVAL;
> > > + } else if (len >= PATH_MAX) {
> > > + return -ENAMETOOLONG;
> > > + }
> > > +
> > > + pinned_map_fd = bpf_obj_get(buf);
> > > + if (pinned_map_fd < 0)
> > > + return pinned_map_fd;
> >
> > Should we rather have a new map definition attribute that tells to reuse
> > the map if it's pinned in bpf fs, and if not, we create it and later on
> > pin it? This is what iproute2 is doing and which we're making use of heavily.
>
> I'd like something like that as well. This would play nicely with
> recently added BTF-defined maps as well.
>
> I think it should be not just pin/don't pin flag, but rather pinning
> strategy, to accommodate various typical strategies of handling maps
> that are already pinned. So something like this:
>
> 1. BPF_PIN_NOTHING - default, don't pin;
> 2. BPF_PIN_EXCLUSIVE - pin, but if map is already pinned - fail;
> 3. BPF_PIN_SET - pin; if existing map exists, reset its state to be
> exact state of object's map;
> 4. BPF_PIN_MERGE - pin, if map exists, fill in NULL entries only (this
> is how Cilium is pinning PROG_ARRAY maps, if I understand correctly);
> 5. BPF_PIN_MERGE_OVERWRITE - pin, if map exists, overwrite non-NULL values.
>
> This list is only for illustrative purposes, ideally people that have
> a lot of experience using pinning for real-world use cases would chime
> in on what strategies are useful and make sense.
My case was simply to reuse existing maps when reloading a program.
Does it make sense for you to add only the simplest cases of listed above?
Also, libbpf doesn't use standard naming conventions for pinning maps.
Does it make sense to provide a list of already open maps to the
bpf_prog_load_xattr function as an attribute? In this case a user
can execute his own policy on pinning, but still will have an option
to reuse, reset, and merge maps.
>
> > In bpf_object__reuse_maps() bailing out if bpf_obj_get() fails is perhaps
> > too limiting for a generic API as new version of an object file may contain
> > new maps which are not yet present in bpf fs at that point.
> >
> > > + err = bpf_map__reuse_fd(map, pinned_map_fd);
> > > + if (err)
> > > + return err;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > int bpf_object__pin_programs(struct bpf_object *obj, const char *path)
> > > {
> > > struct bpf_program *prog;
> > > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > > index d639f47e3110..7fe465a1be76 100644
> > > --- a/tools/lib/bpf/libbpf.h
> > > +++ b/tools/lib/bpf/libbpf.h
> > > @@ -82,6 +82,8 @@ int bpf_object__variable_offset(const struct bpf_object *obj, const char *name,
> > > LIBBPF_API int bpf_object__pin_maps(struct bpf_object *obj, const char *path);
> > > LIBBPF_API int bpf_object__unpin_maps(struct bpf_object *obj,
> > > const char *path);
> > > +LIBBPF_API int bpf_object__reuse_maps(struct bpf_object *obj,
> > > + const char *path);
> > > LIBBPF_API int bpf_object__pin_programs(struct bpf_object *obj,
> > > const char *path);
> > > LIBBPF_API int bpf_object__unpin_programs(struct bpf_object *obj,
> > > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > > index 2c6d835620d2..66a30be6696c 100644
> > > --- a/tools/lib/bpf/libbpf.map
> > > +++ b/tools/lib/bpf/libbpf.map
> > > @@ -172,5 +172,6 @@ LIBBPF_0.0.4 {
> > > btf_dump__new;
> > > btf__parse_elf;
> > > bpf_object__load_xattr;
> > > + bpf_object__reuse_maps;
> > > libbpf_num_possible_cpus;
> > > } LIBBPF_0.0.3;
> > >
> >
^ permalink raw reply
* Re: [PATCH] ath10k: work around uninitialized vht_pfr variable
From: Brian Norris @ 2019-07-08 20:34 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Kalle Valo, Miaoqing Pan, David S. Miller, Rakesh Pillai,
Balaji Pothunoori, Wen Gong, Pradeep kumar Chitrapu, Sriram R,
ath10k, linux-wireless, <netdev@vger.kernel.org>,
Linux Kernel, clang-built-linux
In-Reply-To: <20190708125050.3689133-1-arnd@arndb.de>
Hi Arnd,
On Mon, Jul 8, 2019 at 5:50 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> As clang points out, the vht_pfr is assigned to a struct member
> without being initialized in one case:
>
> drivers/net/wireless/ath/ath10k/mac.c:7528:7: error: variable 'vht_pfr' is used uninitialized whenever 'if' condition
> is false [-Werror,-Wsometimes-uninitialized]
> if (!ath10k_mac_can_set_bitrate_mask(ar, band, mask,
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/ath/ath10k/mac.c:7551:20: note: uninitialized use occurs here
> arvif->vht_pfr = vht_pfr;
> ^~~~~~~
> drivers/net/wireless/ath/ath10k/mac.c:7528:3: note: remove the 'if' if its condition is always true
> if (!ath10k_mac_can_set_bitrate_mask(ar, band, mask,
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wireless/ath/ath10k/mac.c:7483:12: note: initialize the variable 'vht_pfr' to silence this warning
> u8 vht_pfr;
>
> Add an explicit but probably incorrect initialization here.
> I suspect we want a better fix here, but chose this approach to
> illustrate the issue.
>
> Fixes: 8b97b055dc9d ("ath10k: fix failure to set multiple fixed rate")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/net/wireless/ath/ath10k/mac.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index e43a566eef77..0606416dc971 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -7541,6 +7541,8 @@ static int ath10k_mac_op_set_bitrate_mask(struct ieee80211_hw *hw,
> &vht_nss,
> true);
^^ Technically, this call to ath10k_mac_bitrate_mask_get_single_rate()
can fail to assign 'vht_pfr' as well. I can't immediately tell whether
it provably will never hit the -EINVAL case, but if we do, then you'd
have another uninitialized case.
I *believe* it shouldn't fail, since we already pre-checked the VHT
MCS lists for "exactly 1" rate. But it still seems like better code to
pre-initialize and/or add error-handling, so we don't rely on that
implicit proof.
I'm not quite sure yet what the "better" answer should be for
resolving this, but at a minimum, I think the above could be improved.
Brian
> update_bitrate_mask = false;
> + } else {
> + vht_pfr = 0;
> }
>
> mutex_lock(&ar->conf_mutex);
> --
> 2.20.0
>
^ permalink raw reply
* Re: [PATCH v8 net-next 0/5] net: ethernet: ti: cpsw: Add XDP support
From: Ivan Khoronzhuk @ 2019-07-08 20:32 UTC (permalink / raw)
To: David Miller
Cc: grygorii.strashko, hawk, ast, linux-kernel, linux-omap,
xdp-newbies, ilias.apalodimas, netdev, daniel, jakub.kicinski,
john.fastabend
In-Reply-To: <20190707.183511.503486832061897586.davem@davemloft.net>
On Sun, Jul 07, 2019 at 06:35:11PM -0700, David Miller wrote:
>From: David Miller <davem@davemloft.net>
>Date: Sun, 07 Jul 2019 18:31:46 -0700 (PDT)
>
>> From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
>> Date: Fri, 5 Jul 2019 18:04:57 +0300
>>
>>> This patchset adds XDP support for TI cpsw driver and base it on
>>> page_pool allocator. It was verified on af_xdp socket drop,
>>> af_xdp l2f, ebpf XDP_DROP, XDP_REDIRECT, XDP_PASS, XDP_TX.
>>>
>>> It was verified with following configs enabled:
>> ...
>>
>> I'm applying this to net-next, please deal with whatever follow-ups are
>> necessary.
>
>Nevermind, you really have to fix this:
>
>drivers/net/ethernet/ti/davinci_cpdma.c: In function ‘cpdma_chan_submit_si’:
>drivers/net/ethernet/ti/davinci_cpdma.c:1047:12: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
> buffer = (u32)si->data;
> ^
>drivers/net/ethernet/ti/davinci_cpdma.c: In function ‘cpdma_chan_idle_submit_mapped’:
>drivers/net/ethernet/ti/davinci_cpdma.c:1114:12: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
> si.data = (void *)(u32)data;
> ^
>drivers/net/ethernet/ti/davinci_cpdma.c: In function ‘cpdma_chan_submit_mapped’:
>drivers/net/ethernet/ti/davinci_cpdma.c:1164:12: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
> si.data = (void *)(u32)data;
> ^
Actrually that's fixed in reply v9 patch.
But, nevermind, i will send v9 for whole series.
--
Regards,
Ivan Khoronzhuk
^ permalink raw reply
* Re: [PATCH net-next v6 04/15] ethtool: introduce ethtool netlink interface
From: Michal Kubecek @ 2019-07-08 20:22 UTC (permalink / raw)
To: netdev
Cc: Jiri Pirko, David Miller, Jakub Kicinski, Andrew Lunn,
Florian Fainelli, John Linville, Stephen Hemminger, Johannes Berg,
linux-kernel
In-Reply-To: <20190708192629.GD2282@nanopsycho.orion>
On Mon, Jul 08, 2019 at 09:26:29PM +0200, Jiri Pirko wrote:
> Mon, Jul 08, 2019 at 07:27:29PM CEST, mkubecek@suse.cz wrote:
> >
> >There are two reasons for this design. First is to reduce the number of
> >requests needed to get the information. This is not so much a problem of
> >ethtool itself; the only existing commands that would result in multiple
> >request messages would be "ethtool <dev>" and "ethtool -s <dev>". Maybe
> >also "ethtool -x/-X <dev>" but even if the indirection table and hash
> >key have different bits assigned now, they don't have to be split even
> >if we split other commands. It may be bigger problem for daemons wanting
> >to keep track of system configuration which would have to issue many
> >requests whenever a new device appears.
> >
> >Second reason is that with 8-bit genetlink command/message id, the space
> >is not as infinite as it might seem. I counted quickly, right now the
> >full series uses 14 ids for kernel messages, with split you propose it
> >would most likely grow to 44. For full implementation of all ethtool
> >functionality, we could get to ~60 ids. It's still only 1/4 of the
> >available space but it's not clear what the future development will look
> >like. We would certainly need to be careful not to start allocating new
> >commands for single parameters and try to be foreseeing about what can
> >be grouped together. But we will need to do that in any case.
> >
> >On kernel side, splitting existing messages would make some things a bit
> >easier. It would also reduce the number of scenarios where only part of
> >requested information is available or only part of a SET request fails.
>
> Okay, I got your point. So why don't we look at if from the other angle.
> Why don't we have only single get/set command that would be in general
> used to get/set ALL info from/to the kernel. Where we can have these
> bits (perhaps rather varlen bitfield) to for user to indicate which data
> is he interested in? This scales. The other commands would be
> just for action.
>
> Something like RTM_GETLINK/RTM_SETLINK. Makes sense?
It's certainly an option but at the first glance it seems as just moving
what I tried to avoid one level lower. It would work around the u8 issue
(but as Johannes pointed out, we can handle it with genetlink when/if
the time comes). We would almost certainly have to split the replies
into multiple messages to keep the packet size reasonable. I'll have to
think more about the consequences for both kernel and userspace.
My gut feeling is that out of the two extreme options (one universal
message type and message types corresponding to current infomask bits),
the latter is more appealing. After all, ethtool has been gathering
features that would need those ~60 message types for 20 years.
Michal
^ permalink raw reply
* Re: [PATCH bpf-next] selftests/bpf: make verifier loop tests arch independent
From: Ilya Leoshkevich @ 2019-07-08 20:14 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Y Song, Stanislav Fomichev, netdev, bpf, David Miller,
Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <20190708161338.GC29524@mini-arch>
> Am 08.07.2019 um 18:13 schrieb Stanislav Fomichev <sdf@fomichev.me>:
>
> On 07/03, Y Song wrote:
>> On Wed, Jul 3, 2019 at 1:51 PM Stanislav Fomichev <sdf@google.com> wrote:
>>>
>>> Take the first x bytes of pt_regs for scalability tests, there is
>>> no real reason we need x86 specific rax.
>>>
>>> Signed-off-by: Stanislav Fomichev <sdf@google.com>
>>> ---
>>> tools/testing/selftests/bpf/progs/loop1.c | 3 ++-
>>> tools/testing/selftests/bpf/progs/loop2.c | 3 ++-
>>> tools/testing/selftests/bpf/progs/loop3.c | 3 ++-
>>> 3 files changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/bpf/progs/loop1.c b/tools/testing/selftests/bpf/progs/loop1.c
>>> index dea395af9ea9..d530c61d2517 100644
>>> --- a/tools/testing/selftests/bpf/progs/loop1.c
>>> +++ b/tools/testing/selftests/bpf/progs/loop1.c
>>> @@ -14,11 +14,12 @@ SEC("raw_tracepoint/kfree_skb")
>>> int nested_loops(volatile struct pt_regs* ctx)
>>> {
>>> int i, j, sum = 0, m;
>>> + volatile int *any_reg = (volatile int *)ctx;
>>>
>>> for (j = 0; j < 300; j++)
>>> for (i = 0; i < j; i++) {
>>> if (j & 1)
>>> - m = ctx->rax;
>>> + m = *any_reg;
>>
>> I agree. ctx->rax here is only to generate some operations, which
>> cannot be optimized away by the compiler. dereferencing a volatile
>> pointee may just serve that purpose.
>>
>> Comparing the byte code generated with ctx->rax and *any_reg, they are
>> slightly different. Using *any_reg is slighly worse, but this should
>> be still okay for the test.
>>
>>> else
>>> m = j;
>>> sum += i * m;
>>> diff --git a/tools/testing/selftests/bpf/progs/loop2.c b/tools/testing/selftests/bpf/progs/loop2.c
>>> index 0637bd8e8bcf..91bb89d901e3 100644
>>> --- a/tools/testing/selftests/bpf/progs/loop2.c
>>> +++ b/tools/testing/selftests/bpf/progs/loop2.c
>>> @@ -14,9 +14,10 @@ SEC("raw_tracepoint/consume_skb")
>>> int while_true(volatile struct pt_regs* ctx)
>>> {
>>> int i = 0;
>>> + volatile int *any_reg = (volatile int *)ctx;
>>>
>>> while (true) {
>>> - if (ctx->rax & 1)
>>> + if (*any_reg & 1)
>>> i += 3;
>>> else
>>> i += 7;
>>> diff --git a/tools/testing/selftests/bpf/progs/loop3.c b/tools/testing/selftests/bpf/progs/loop3.c
>>> index 30a0f6cba080..3a7f12d7186c 100644
>>> --- a/tools/testing/selftests/bpf/progs/loop3.c
>>> +++ b/tools/testing/selftests/bpf/progs/loop3.c
>>> @@ -14,9 +14,10 @@ SEC("raw_tracepoint/consume_skb")
>>> int while_true(volatile struct pt_regs* ctx)
>>> {
>>> __u64 i = 0, sum = 0;
>>> + volatile __u64 *any_reg = (volatile __u64 *)ctx;
>>> do {
>>> i++;
>>> - sum += ctx->rax;
>>> + sum += *any_reg;
>>> } while (i < 0x100000000ULL);
>>> return sum;
>>> }
>>> --
>>> 2.22.0.410.gd8fdbe21b5-goog
>>
>> Ilya Leoshkevich (iii@linux.ibm.com, cc'ed) has another patch set
>> trying to solve this problem by introducing s360 arch register access
>> macros. I guess for now that patch set is not needed any more?
> Oh, I missed them. Do they fix the tests for other (non-s360) arches as
> well? I was trying to fix the issue by not depending on any arch
> specific stuff because the test really doesn't care :-)
They are supposed to work for everything that defines PT_REGS_RC in
bpf_helpers.h, but I have to admit I tested only x86_64 and s390.
The main source of problems with my approach were mismatching definitions
of struct pt_regs for userspace and kernel, and because of that there was
some tweaking required for both arches. I will double check how it looks
for others (arm, mips, ppc, sparc) tomorrow.
Best regards,
Ilya
^ permalink raw reply
* Re: Kernel BUG: epoll_wait() (and epoll_pwait) stall for 206 ms per call on sockets with a small-ish snd/rcv buffer.
From: Neal Cardwell @ 2019-07-08 20:11 UTC (permalink / raw)
To: Carlo Wood; +Cc: David Miller, Netdev
In-Reply-To: <20190706201912.435a2198@hikaru>
On Sat, Jul 6, 2019 at 2:19 PM Carlo Wood <carlo@alinoe.com> wrote:
>
> While investigating this further, I read on
> http://www.masterraghu.com/subjects/np/introduction/unix_network_programming_v1.3/ch07lev1sec5.html
> under "SO_RCVBUF and SO_SNDBUF Socket Options":
>
> When setting the size of the TCP socket receive buffer, the
> ordering of the function calls is important. This is because of
> TCP's window scale option (Section 2.6), which is exchanged with
> the peer on the SYN segments when the connection is established.
> For a client, this means the SO_RCVBUF socket option must be set
> before calling connect. For a server, this means the socket option
> must be set for the listening socket before calling listen. Setting
> this option for the connected socket will have no effect whatsoever
> on the possible window scale option because accept does not return
> with the connected socket until TCP's three-way handshake is
> complete. That is why this option must be set for the listening
> socket. (The sizes of the socket buffers are always inherited from
> the listening socket by the newly created connected socket: pp.
> 462–463 of TCPv2.)
>
> As mentioned in a previous post, I had already discovered about
> needing to set the socket buffers before connect, but I didn't know
> about setting them before the call to listen() in order to get the
> buffer sizes inherited by the accepted sockets.
>
> After fixing this in my test program, all problems disappeared when
> keeping the send and receive buffers the same on both sides.
>
> However, when only setting the send and receive buffers on the client
> socket (not on the (accepted or) listen socket), epoll_wait() still
> stalls 43ms. When the SO_SNDBUF is smaller than 33182 bytes.
>
> Here is the latest version of my test program:
>
> https://github.com/CarloWood/ai-evio-testsuite/blob/master/src/epoll_bug.c
>
> I have to retract most of my "bug" report, it might even not really be
> a bug then... but nevertheless, what remains strange is the fact
> that setting the socket buffer sizes on the accepted sockets can lead
> to so much crippling effect, while the quoted website states:
>
> Setting this option for the connected socket will have no effect
> whatsoever on the possible window scale option because accept does
> not return with the connected socket until TCP's three-way
> handshake is complete.
>
> And when only setting the socket buffer sizes for the client socket
> (that I use to send back received data; so this is the sending
> side now) then why does epoll_wait() stall 43 ms per call when the
> receiving side is using the default (much larger) socket buffer sizes?
>
> That 43 ms is STILL crippling-- slowing down the transmission of the
> data to a trickling speed compared to what it should be.
Based on the magic numbers you cite, including the fact that this test
program seems to send traffic over a loopback device (presumably
MTU=65536), epoll_wait() stalling 43 ms (slightly longer than the
typical Linux delayed ACK timer), and the problem only happening if
SO_SNDBUF is smaller than 33182 bytes (half the MTU)... a guess would
be that when you artificially make the SO_SNDBUF that low, then you
are asking the kernel to only allow your sending sockets to buffer
less than a single MTU of data, which means that the packets the
sender sends are going to be less than the MTU, which means that the
receiver may tend to eventually (after the initial quick ACKs expire)
delay its ACKs in hopes of eventually receiving a full MSS of data
(see __tcp_ack_snd_check()). Since the SO_SNDBUF is so small in this
case, the sender then would not be able to write() or transmit
anything else until the receiver sends a delayed ACK for the data
~40ms later, allowing the sending socket to free the previously sent
data and trigger the sender's next EPOLLOUT and write().
You could try grabbing a packet capture of the traffic over your
loopback device during the test to see if it matches that theory:
tcpdump -i lo -s 100 -w /tmp/test.pcap
cheers,
neal
^ permalink raw reply
* Re: [PATCH v3 6/7] dt-bindings: net: realtek: Add property to configure LED mode
From: Matthias Kaehlcke @ 2019-07-08 20:07 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S . Miller, Rob Herring, Mark Rutland, Florian Fainelli,
Heiner Kallweit, netdev, devicetree, linux-kernel,
Douglas Anderson
In-Reply-To: <20190708194834.GI9027@lunn.ch>
Hi Andrew,
On Mon, Jul 08, 2019 at 09:48:34PM +0200, Andrew Lunn wrote:
> On Mon, Jul 08, 2019 at 12:24:58PM -0700, Matthias Kaehlcke wrote:
> > The LED behavior of some Realtek PHYs is configurable. Add the
> > property 'realtek,led-modes' to specify the configuration of the
> > LEDs.
> >
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
>
> Hi Matthias
>
> Humm. I thought you were going to drop this and the next patch?
It wasn't clear to me whether not introducing a generic interface is a
definitive NACK and tought I could at least post a version with the
review comments addressed. If there is no way forward without a
generic interface I'll drop the two patches.
^ permalink raw reply
* Re: [PATCH v3 net-next 19/19] ionic: Add basic devlink interface
From: Jiri Pirko @ 2019-07-08 20:03 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev
In-Reply-To: <af206309-514d-9619-1455-efc93af8431e@pensando.io>
Mon, Jul 08, 2019 at 09:58:09PM CEST, snelson@pensando.io wrote:
>On 7/8/19 12:34 PM, Jiri Pirko wrote:
>> Mon, Jul 08, 2019 at 09:25:32PM CEST, snelson@pensando.io wrote:
>> > Add a devlink interface for access to information that isn't
>> > normally available through ethtool or the iplink interface.
>> >
>> > Example:
>> > $ ./devlink -j -p dev info pci/0000:b6:00.0
>> > {
>> > "info": {
>> > "pci/0000:b6:00.0": {
>> > "driver": "ionic",
>> > "serial_number": "FLM18420073",
>> > "versions": {
>> > "fixed": {
>> > "fw_version": "0.11.0-50",
>> > "fw_status": "0x1",
>> > "fw_heartbeat": "0x716ce",
>> > "asic_type": "0x0",
>> > "asic_rev": "0x0"
>> > }
>> > }
>> > }
>> > }
>> > }
>> >
>> > Signed-off-by: Shannon Nelson <snelson@pensando.io>
>> > ---
>> > drivers/net/ethernet/pensando/ionic/Makefile | 2 +-
>> > drivers/net/ethernet/pensando/ionic/ionic.h | 1 +
>> > .../ethernet/pensando/ionic/ionic_bus_pci.c | 7 ++
>> > .../ethernet/pensando/ionic/ionic_devlink.c | 89 +++++++++++++++++++
>> > .../ethernet/pensando/ionic/ionic_devlink.h | 12 +++
>> > 5 files changed, 110 insertions(+), 1 deletion(-)
>> > create mode 100644 drivers/net/ethernet/pensando/ionic/ionic_devlink.c
>> > create mode 100644 drivers/net/ethernet/pensando/ionic/ionic_devlink.h
>> >
>> > diff --git a/drivers/net/ethernet/pensando/ionic/Makefile b/drivers/net/ethernet/pensando/ionic/Makefile
>> > index 4f3cfbf36c23..ce187c7b33a8 100644
>> > --- a/drivers/net/ethernet/pensando/ionic/Makefile
>> > +++ b/drivers/net/ethernet/pensando/ionic/Makefile
>> > @@ -5,4 +5,4 @@ obj-$(CONFIG_IONIC) := ionic.o
>> >
>> > ionic-y := ionic_main.o ionic_bus_pci.o ionic_dev.o ionic_ethtool.o \
>> > ionic_lif.o ionic_rx_filter.o ionic_txrx.o ionic_debugfs.o \
>> > - ionic_stats.o
>> > + ionic_stats.o ionic_devlink.o
>> > diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
>> > index cd08166f73a9..a0034bc5b4a1 100644
>> > --- a/drivers/net/ethernet/pensando/ionic/ionic.h
>> > +++ b/drivers/net/ethernet/pensando/ionic/ionic.h
>> > @@ -44,6 +44,7 @@ struct ionic {
>> > DECLARE_BITMAP(intrs, INTR_CTRL_REGS_MAX);
>> > struct work_struct nb_work;
>> > struct notifier_block nb;
>> > + struct devlink *dl;
>> > };
>> >
>> > struct ionic_admin_ctx {
>> > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> > index 98c12b770c7f..a8c99254489f 100644
>> > --- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> > +++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> > @@ -10,6 +10,7 @@
>> > #include "ionic_bus.h"
>> > #include "ionic_lif.h"
>> > #include "ionic_debugfs.h"
>> > +#include "ionic_devlink.h"
>> >
>> > /* Supported devices */
>> > static const struct pci_device_id ionic_id_table[] = {
>> > @@ -212,9 +213,14 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>> > goto err_out_deinit_lifs;
>> > }
>> >
>> > + err = ionic_devlink_register(ionic);
>> > + if (err)
>> > + dev_err(dev, "Cannot register devlink (ignored): %d\n", err);
>> > +
>> > return 0;
>> >
>> > err_out_deinit_lifs:
>> > + ionic_devlink_unregister(ionic);
>> > ionic_lifs_deinit(ionic);
>> > err_out_free_lifs:
>> > ionic_lifs_free(ionic);
>> > @@ -247,6 +253,7 @@ static void ionic_remove(struct pci_dev *pdev)
>> > struct ionic *ionic = pci_get_drvdata(pdev);
>> >
>> > if (ionic) {
>> > + ionic_devlink_unregister(ionic);
>> > ionic_lifs_unregister(ionic);
>> > ionic_lifs_deinit(ionic);
>> > ionic_lifs_free(ionic);
>> > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
>> > new file mode 100644
>> > index 000000000000..fbbfcdde292f
>> > --- /dev/null
>> > +++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
>> > @@ -0,0 +1,89 @@
>> > +// SPDX-License-Identifier: GPL-2.0
>> > +/* Copyright(c) 2017 - 2019 Pensando Systems, Inc */
>> > +
>> > +#include <linux/module.h>
>> > +#include <linux/netdevice.h>
>> > +
>> > +#include "ionic.h"
>> > +#include "ionic_bus.h"
>> > +#include "ionic_lif.h"
>> > +#include "ionic_devlink.h"
>> > +
>> > +struct ionic_devlink {
>> > + struct ionic *ionic;
>> > +};
>> > +
>> > +static int ionic_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
>> > + struct netlink_ext_ack *extack)
>> > +{
>> > + struct ionic *ionic = *(struct ionic **)devlink_priv(dl);
>> > + struct ionic_dev *idev = &ionic->idev;
>> > + char buf[16];
>> > + u32 val;
>> > +
>> > + devlink_info_driver_name_put(req, DRV_NAME);
>> > +
>> > + devlink_info_version_fixed_put(req, "fw_version",
>> > + idev->dev_info.fw_version);
>> > +
>> > + val = ioread8(&idev->dev_info_regs->fw_status);
>> > + snprintf(buf, sizeof(buf), "0x%x", val);
>> > + devlink_info_version_fixed_put(req, "fw_status", buf);
>> > +
>> > + val = ioread32(&idev->dev_info_regs->fw_heartbeat);
>> > + snprintf(buf, sizeof(buf), "0x%x", val);
>> > + devlink_info_version_fixed_put(req, "fw_heartbeat", buf);
>> > +
>> > + snprintf(buf, sizeof(buf), "0x%x", idev->dev_info.asic_type);
>> > + devlink_info_version_fixed_put(req, "asic_type", buf);
>> > +
>> > + snprintf(buf, sizeof(buf), "0x%x", idev->dev_info.asic_rev);
>> > + devlink_info_version_fixed_put(req, "asic_rev", buf);
>> > +
>> > + devlink_info_serial_number_put(req, idev->dev_info.serial_num);
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static const struct devlink_ops ionic_dl_ops = {
>> > + .info_get = ionic_dl_info_get,
>> > +};
>> > +
>> > +int ionic_devlink_register(struct ionic *ionic)
>> > +{
>> > + struct devlink *dl;
>> > + struct ionic **ip;
>> > + int err;
>> > +
>> > + dl = devlink_alloc(&ionic_dl_ops, sizeof(struct ionic *));
>> Oups. Something is wrong with your flow. The devlink alloc is allocating
>> the structure that holds private data (per-device data) for you. This is
>> misuse :/
>>
>> You are missing one parent device struct apparently.
>>
>> Oh, I think I see something like it. The unused "struct ionic_devlink".
>
>If I'm not mistaken, the alloc is only allocating enough for a pointer, not
>the whole per device struct, and a few lines down from here the pointer to
>the new devlink struct is assigned to ionic->dl. This was based on what I
>found in the qed driver's qed_devlink_register(), and it all seems to work.
I'm not saying your code won't work. What I say is that you should have
a struct for device that would be allocated by devlink_alloc()
The ionic struct should be associated with devlink_port. That you are
missing too.
>
>That unused struct ionic_devlink does need to go away, it was superfluous
>after working out a better typecast off of devlink_priv().
>
>I'll remove the unused struct ionic_devlink, but I think the rest is okay.
>
>sln
>
>>
>>
>> > + if (!dl) {
>> > + dev_warn(ionic->dev, "devlink_alloc failed");
>> > + return -ENOMEM;
>> > + }
>> > +
>> > + ip = (struct ionic **)devlink_priv(dl);
>> > + *ip = ionic;
>> > + ionic->dl = dl;
>> > +
>> > + err = devlink_register(dl, ionic->dev);
>> > + if (err) {
>> > + dev_warn(ionic->dev, "devlink_register failed: %d\n", err);
>> > + goto err_dl_free;
>> > + }
>> > +
>> > + return 0;
>> > +
>> > +err_dl_free:
>> > + ionic->dl = NULL;
>> > + devlink_free(dl);
>> > + return err;
>> > +}
>> > +
>> > +void ionic_devlink_unregister(struct ionic *ionic)
>> > +{
>> > + if (!ionic->dl)
>> > + return;
>> > +
>> > + devlink_unregister(ionic->dl);
>> > + devlink_free(ionic->dl);
>> > +}
>> > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.h b/drivers/net/ethernet/pensando/ionic/ionic_devlink.h
>> > new file mode 100644
>> > index 000000000000..35528884e29f
>> > --- /dev/null
>> > +++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.h
>> > @@ -0,0 +1,12 @@
>> > +/* SPDX-License-Identifier: GPL-2.0 */
>> > +/* Copyright(c) 2017 - 2019 Pensando Systems, Inc */
>> > +
>> > +#ifndef _IONIC_DEVLINK_H_
>> > +#define _IONIC_DEVLINK_H_
>> > +
>> > +#include <net/devlink.h>
>> > +
>> > +int ionic_devlink_register(struct ionic *ionic);
>> > +void ionic_devlink_unregister(struct ionic *ionic);
>> > +
>> > +#endif /* _IONIC_DEVLINK_H_ */
>> > --
>> > 2.17.1
>> >
>
^ permalink raw reply
* Re: [PATCH v3 1/7] dt-bindings: net: Add bindings for Realtek PHYs
From: Matthias Kaehlcke @ 2019-07-08 20:01 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S . Miller, Rob Herring, Mark Rutland, Florian Fainelli,
Heiner Kallweit, netdev, devicetree, linux-kernel,
Douglas Anderson
In-Reply-To: <20190708194615.GH9027@lunn.ch>
On Mon, Jul 08, 2019 at 09:46:15PM +0200, Andrew Lunn wrote:
> On Mon, Jul 08, 2019 at 12:24:53PM -0700, Matthias Kaehlcke wrote:
> > Add the 'realtek,eee-led-mode-disable' property to disable EEE
> > LED mode on Realtek PHYs that support it.
> >
> > Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> > ---
> > TODO: adapt PHY core to deal with optional compatible strings
>
> Yes. Does this even work at the moment? I would expect
> of_mdiobus_child_is_phy() to return false, indicating the device is
> not actually a PHY.
Indeed, it currently doesn't work atm. I found that removing the check
for dev->of_node in of_mdiobus_link_mdiodev() helps, but I imagine
doing (only) this might have undesired side-effects.
^ permalink raw reply
* Re: [PATCH v3 net-next 19/19] ionic: Add basic devlink interface
From: Shannon Nelson @ 2019-07-08 19:58 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev
In-Reply-To: <20190708193454.GF2282@nanopsycho.orion>
On 7/8/19 12:34 PM, Jiri Pirko wrote:
> Mon, Jul 08, 2019 at 09:25:32PM CEST, snelson@pensando.io wrote:
>> Add a devlink interface for access to information that isn't
>> normally available through ethtool or the iplink interface.
>>
>> Example:
>> $ ./devlink -j -p dev info pci/0000:b6:00.0
>> {
>> "info": {
>> "pci/0000:b6:00.0": {
>> "driver": "ionic",
>> "serial_number": "FLM18420073",
>> "versions": {
>> "fixed": {
>> "fw_version": "0.11.0-50",
>> "fw_status": "0x1",
>> "fw_heartbeat": "0x716ce",
>> "asic_type": "0x0",
>> "asic_rev": "0x0"
>> }
>> }
>> }
>> }
>> }
>>
>> Signed-off-by: Shannon Nelson <snelson@pensando.io>
>> ---
>> drivers/net/ethernet/pensando/ionic/Makefile | 2 +-
>> drivers/net/ethernet/pensando/ionic/ionic.h | 1 +
>> .../ethernet/pensando/ionic/ionic_bus_pci.c | 7 ++
>> .../ethernet/pensando/ionic/ionic_devlink.c | 89 +++++++++++++++++++
>> .../ethernet/pensando/ionic/ionic_devlink.h | 12 +++
>> 5 files changed, 110 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/net/ethernet/pensando/ionic/ionic_devlink.c
>> create mode 100644 drivers/net/ethernet/pensando/ionic/ionic_devlink.h
>>
>> diff --git a/drivers/net/ethernet/pensando/ionic/Makefile b/drivers/net/ethernet/pensando/ionic/Makefile
>> index 4f3cfbf36c23..ce187c7b33a8 100644
>> --- a/drivers/net/ethernet/pensando/ionic/Makefile
>> +++ b/drivers/net/ethernet/pensando/ionic/Makefile
>> @@ -5,4 +5,4 @@ obj-$(CONFIG_IONIC) := ionic.o
>>
>> ionic-y := ionic_main.o ionic_bus_pci.o ionic_dev.o ionic_ethtool.o \
>> ionic_lif.o ionic_rx_filter.o ionic_txrx.o ionic_debugfs.o \
>> - ionic_stats.o
>> + ionic_stats.o ionic_devlink.o
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
>> index cd08166f73a9..a0034bc5b4a1 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic.h
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic.h
>> @@ -44,6 +44,7 @@ struct ionic {
>> DECLARE_BITMAP(intrs, INTR_CTRL_REGS_MAX);
>> struct work_struct nb_work;
>> struct notifier_block nb;
>> + struct devlink *dl;
>> };
>>
>> struct ionic_admin_ctx {
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> index 98c12b770c7f..a8c99254489f 100644
>> --- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>> @@ -10,6 +10,7 @@
>> #include "ionic_bus.h"
>> #include "ionic_lif.h"
>> #include "ionic_debugfs.h"
>> +#include "ionic_devlink.h"
>>
>> /* Supported devices */
>> static const struct pci_device_id ionic_id_table[] = {
>> @@ -212,9 +213,14 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>> goto err_out_deinit_lifs;
>> }
>>
>> + err = ionic_devlink_register(ionic);
>> + if (err)
>> + dev_err(dev, "Cannot register devlink (ignored): %d\n", err);
>> +
>> return 0;
>>
>> err_out_deinit_lifs:
>> + ionic_devlink_unregister(ionic);
>> ionic_lifs_deinit(ionic);
>> err_out_free_lifs:
>> ionic_lifs_free(ionic);
>> @@ -247,6 +253,7 @@ static void ionic_remove(struct pci_dev *pdev)
>> struct ionic *ionic = pci_get_drvdata(pdev);
>>
>> if (ionic) {
>> + ionic_devlink_unregister(ionic);
>> ionic_lifs_unregister(ionic);
>> ionic_lifs_deinit(ionic);
>> ionic_lifs_free(ionic);
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
>> new file mode 100644
>> index 000000000000..fbbfcdde292f
>> --- /dev/null
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
>> @@ -0,0 +1,89 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright(c) 2017 - 2019 Pensando Systems, Inc */
>> +
>> +#include <linux/module.h>
>> +#include <linux/netdevice.h>
>> +
>> +#include "ionic.h"
>> +#include "ionic_bus.h"
>> +#include "ionic_lif.h"
>> +#include "ionic_devlink.h"
>> +
>> +struct ionic_devlink {
>> + struct ionic *ionic;
>> +};
>> +
>> +static int ionic_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
>> + struct netlink_ext_ack *extack)
>> +{
>> + struct ionic *ionic = *(struct ionic **)devlink_priv(dl);
>> + struct ionic_dev *idev = &ionic->idev;
>> + char buf[16];
>> + u32 val;
>> +
>> + devlink_info_driver_name_put(req, DRV_NAME);
>> +
>> + devlink_info_version_fixed_put(req, "fw_version",
>> + idev->dev_info.fw_version);
>> +
>> + val = ioread8(&idev->dev_info_regs->fw_status);
>> + snprintf(buf, sizeof(buf), "0x%x", val);
>> + devlink_info_version_fixed_put(req, "fw_status", buf);
>> +
>> + val = ioread32(&idev->dev_info_regs->fw_heartbeat);
>> + snprintf(buf, sizeof(buf), "0x%x", val);
>> + devlink_info_version_fixed_put(req, "fw_heartbeat", buf);
>> +
>> + snprintf(buf, sizeof(buf), "0x%x", idev->dev_info.asic_type);
>> + devlink_info_version_fixed_put(req, "asic_type", buf);
>> +
>> + snprintf(buf, sizeof(buf), "0x%x", idev->dev_info.asic_rev);
>> + devlink_info_version_fixed_put(req, "asic_rev", buf);
>> +
>> + devlink_info_serial_number_put(req, idev->dev_info.serial_num);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct devlink_ops ionic_dl_ops = {
>> + .info_get = ionic_dl_info_get,
>> +};
>> +
>> +int ionic_devlink_register(struct ionic *ionic)
>> +{
>> + struct devlink *dl;
>> + struct ionic **ip;
>> + int err;
>> +
>> + dl = devlink_alloc(&ionic_dl_ops, sizeof(struct ionic *));
> Oups. Something is wrong with your flow. The devlink alloc is allocating
> the structure that holds private data (per-device data) for you. This is
> misuse :/
>
> You are missing one parent device struct apparently.
>
> Oh, I think I see something like it. The unused "struct ionic_devlink".
If I'm not mistaken, the alloc is only allocating enough for a pointer,
not the whole per device struct, and a few lines down from here the
pointer to the new devlink struct is assigned to ionic->dl. This was
based on what I found in the qed driver's qed_devlink_register(), and it
all seems to work.
That unused struct ionic_devlink does need to go away, it was
superfluous after working out a better typecast off of devlink_priv().
I'll remove the unused struct ionic_devlink, but I think the rest is okay.
sln
>
>
>> + if (!dl) {
>> + dev_warn(ionic->dev, "devlink_alloc failed");
>> + return -ENOMEM;
>> + }
>> +
>> + ip = (struct ionic **)devlink_priv(dl);
>> + *ip = ionic;
>> + ionic->dl = dl;
>> +
>> + err = devlink_register(dl, ionic->dev);
>> + if (err) {
>> + dev_warn(ionic->dev, "devlink_register failed: %d\n", err);
>> + goto err_dl_free;
>> + }
>> +
>> + return 0;
>> +
>> +err_dl_free:
>> + ionic->dl = NULL;
>> + devlink_free(dl);
>> + return err;
>> +}
>> +
>> +void ionic_devlink_unregister(struct ionic *ionic)
>> +{
>> + if (!ionic->dl)
>> + return;
>> +
>> + devlink_unregister(ionic->dl);
>> + devlink_free(ionic->dl);
>> +}
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.h b/drivers/net/ethernet/pensando/ionic/ionic_devlink.h
>> new file mode 100644
>> index 000000000000..35528884e29f
>> --- /dev/null
>> +++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.h
>> @@ -0,0 +1,12 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/* Copyright(c) 2017 - 2019 Pensando Systems, Inc */
>> +
>> +#ifndef _IONIC_DEVLINK_H_
>> +#define _IONIC_DEVLINK_H_
>> +
>> +#include <net/devlink.h>
>> +
>> +int ionic_devlink_register(struct ionic *ionic);
>> +void ionic_devlink_unregister(struct ionic *ionic);
>> +
>> +#endif /* _IONIC_DEVLINK_H_ */
>> --
>> 2.17.1
>>
^ permalink raw reply
* Re: [PATCH v3 bpf-next 4/9] libbpf: add kprobe/uprobe attach API
From: Andrii Nakryiko @ 2019-07-08 19:55 UTC (permalink / raw)
To: Matt Hart
Cc: Andrii Nakryiko, Alexei Starovoitov, bpf, Daniel Borkmann,
Kernel Team, Networking, Stanislav Fomichev
In-Reply-To: <CAH+k93G=qGLfEKe+3dSZPKhmxrc8JiPqDppGa-yLSwaQYRJU=Q@mail.gmail.com>
On Mon, Jul 8, 2019 at 12:27 PM Matt Hart <matthew.hart@linaro.org> wrote:
>
> On Mon, 8 Jul 2019 at 18:58, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Mon, Jul 8, 2019 at 8:11 AM Matt Hart <matthew.hart@linaro.org> wrote:
> > >
> > > Hi all,
> > >
> > > I bisected a perf build error on ARMv7 to this patch:
> > > libbpf.c: In function ‘perf_event_open_probe’:
> > > libbpf.c:4112:17: error: cast from pointer to integer of different
> > > size [-Werror=pointer-to-int-cast]
> > > attr.config1 = (uint64_t)(void *)name; /* kprobe_func or uprobe_path */
> > > ^
> > >
> > > Is this a known issue?
> >
> > No, thanks for reporting!
> >
> > It should be
> >
> > attr.config1 = (uint64_t)(uintptr_t)(void *)name;
> >
> > to avoid warning on 32-bit architectures.
>
> Tested with manual change and can confirm perf now builds without errors.
Thanks for testing!
I'll add Tested-by: Matt Hart <matthew.hart@linaro.org> when posting a fix.
>
> >
> > I'll post a fix later today, but if you could verify this fixes
> > warning for you, I'd really appreciate that! Thanks!
^ permalink raw reply
* Re: [PATCH v3 6/7] dt-bindings: net: realtek: Add property to configure LED mode
From: Andrew Lunn @ 2019-07-08 19:48 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: David S . Miller, Rob Herring, Mark Rutland, Florian Fainelli,
Heiner Kallweit, netdev, devicetree, linux-kernel,
Douglas Anderson
In-Reply-To: <20190708192459.187984-7-mka@chromium.org>
On Mon, Jul 08, 2019 at 12:24:58PM -0700, Matthias Kaehlcke wrote:
> The LED behavior of some Realtek PHYs is configurable. Add the
> property 'realtek,led-modes' to specify the configuration of the
> LEDs.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Hi Matthias
Humm. I thought you were going to drop this and the next patch?
Andrew
^ permalink raw reply
* Re: [PATCH v3 1/7] dt-bindings: net: Add bindings for Realtek PHYs
From: Andrew Lunn @ 2019-07-08 19:46 UTC (permalink / raw)
To: Matthias Kaehlcke
Cc: David S . Miller, Rob Herring, Mark Rutland, Florian Fainelli,
Heiner Kallweit, netdev, devicetree, linux-kernel,
Douglas Anderson
In-Reply-To: <20190708192459.187984-2-mka@chromium.org>
On Mon, Jul 08, 2019 at 12:24:53PM -0700, Matthias Kaehlcke wrote:
> Add the 'realtek,eee-led-mode-disable' property to disable EEE
> LED mode on Realtek PHYs that support it.
>
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> TODO: adapt PHY core to deal with optional compatible strings
Yes. Does this even work at the moment? I would expect
of_mdiobus_child_is_phy() to return false, indicating the device is
not actually a PHY.
Andrew
^ permalink raw reply
* Re: [PATCH] [net-next] macb: fix build warning for !CONFIG_OF
From: David Miller @ 2019-07-08 19:43 UTC (permalink / raw)
To: arnd
Cc: nicolas.ferre, palmer, paul.walmsley, yash.shah, harini.katakam,
claudiu.beznea, netdev, linux-kernel, linux-riscv
In-Reply-To: <20190708124840.3616530-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Mon, 8 Jul 2019 14:48:23 +0200
> When CONFIG_OF is disabled, we get a harmless warning about the
> newly added variable:
>
> drivers/net/ethernet/cadence/macb_main.c:48:39: error: 'mgmt' defined but not used [-Werror=unused-variable]
> static struct sifive_fu540_macb_mgmt *mgmt;
>
> Move the variable closer to its use inside of the #ifdef.
>
> Fixes: c218ad559020 ("macb: Add support for SiFive FU540-C000")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied, thanks Arnd.
^ permalink raw reply
* Re: [PATCH] [net-next] gve: fix unused variable/label warnings
From: David Miller @ 2019-07-08 19:41 UTC (permalink / raw)
To: arnd; +Cc: csully, sagis, jonolson, willemb, lrizzo, netdev, linux-kernel
In-Reply-To: <20190708124350.3470436-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Mon, 8 Jul 2019 14:43:39 +0200
> On unusual page sizes, we get harmless warnings:
>
> drivers/net/ethernet/google/gve/gve_rx.c:283:6: error: unused variable 'pagecount' [-Werror,-Wunused-variable]
> drivers/net/ethernet/google/gve/gve_rx.c:336:1: error: unused label 'have_skb' [-Werror,-Wunused-label]
>
> Change the preprocessor #if to regular if() to avoid this.
>
> Fixes: f5cedc84a30d ("gve: Add transmit and receive support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied.
^ permalink raw reply
* Re: [PATCH net] net: stmmac: Re-work the queue selection for TSO packets
From: David Miller @ 2019-07-08 19:41 UTC (permalink / raw)
To: Jose.Abreu
Cc: netdev, Joao.Pinto, peppe.cavallaro, alexandre.torgue,
mcoquelin.stm32, linux-stm32, linux-arm-kernel, linux-kernel, ben
In-Reply-To: <36018491f47206728e04d67a9e6263635e64f721.1562588640.git.joabreu@synopsys.com>
From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Mon, 8 Jul 2019 14:26:28 +0200
> Ben Hutchings says:
> "This is the wrong place to change the queue mapping.
> stmmac_xmit() is called with a specific TX queue locked,
> and accessing a different TX queue results in a data race
> for all of that queue's state.
>
> I think this commit should be reverted upstream and in all
> stable branches. Instead, the driver should implement the
> ndo_select_queue operation and override the queue mapping there."
>
> Fixes: c5acdbee22a1 ("net: stmmac: Send TSO packets always from Queue 0")
> Suggested-by: Ben Hutchings <ben@decadent.org.uk>
> Signed-off-by: Jose Abreu <joabreu@synopsys.com>
Applied and queued up for -stable.
^ permalink raw reply
* possible deadlock in rtnl_lock (6)
From: syzbot @ 2019-07-08 19:37 UTC (permalink / raw)
To: ast, bjorn.topel, bpf, christian, daniel, davem, dsahern,
edumazet, hawk, i.maximets, idosch, jakub.kicinski, johannes.berg,
john.fastabend, jonathan.lemon, kafai, linux-kernel,
magnus.karlsson, netdev, petrm, roopa, songliubraving,
syzkaller-bugs, xdp-newbies, yhs
Hello,
syzbot found the following crash on:
HEAD commit: 537de0c8 ipv4: Fix NULL pointer dereference in ipv4_neigh_..
git tree: net
console output: https://syzkaller.appspot.com/x/log.txt?x=14521cc3a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=90f5d2d9c1e7421c
dashboard link: https://syzkaller.appspot.com/bug?extid=174ce29c2308dec5bc68
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1777debba00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16969b53a00000
The bug was bisected to:
commit 455302d1c9ae9318660aaeb9748a01ff414c9741
Author: Ilya Maximets <i.maximets@samsung.com>
Date: Fri Jun 28 08:04:07 2019 +0000
xdp: fix hang while unregistering device bound to xdp socket
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1179943da00000
final crash: https://syzkaller.appspot.com/x/report.txt?x=1379943da00000
console output: https://syzkaller.appspot.com/x/log.txt?x=1579943da00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+174ce29c2308dec5bc68@syzkaller.appspotmail.com
Fixes: 455302d1c9ae ("xdp: fix hang while unregistering device bound to xdp
socket")
======================================================
WARNING: possible circular locking dependency detected
5.2.0-rc6+ #76 Not tainted
------------------------------------------------------
syz-executor613/9114 is trying to acquire lock:
000000002c564901 (rtnl_mutex){+.+.}, at: rtnl_lock+0x17/0x20
net/core/rtnetlink.c:72
but task is already holding lock:
0000000039d6ee9b (&xs->mutex){+.+.}, at: xsk_bind+0x16a/0xe70
net/xdp/xsk.c:422
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #2 (&xs->mutex){+.+.}:
__mutex_lock_common kernel/locking/mutex.c:926 [inline]
__mutex_lock+0xf7/0x1310 kernel/locking/mutex.c:1073
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1088
xsk_notifier+0x149/0x290 net/xdp/xsk.c:730
notifier_call_chain+0xc2/0x230 kernel/notifier.c:95
__raw_notifier_call_chain kernel/notifier.c:396 [inline]
raw_notifier_call_chain+0x2e/0x40 kernel/notifier.c:403
call_netdevice_notifiers_info+0x3f/0x90 net/core/dev.c:1749
call_netdevice_notifiers_extack net/core/dev.c:1761 [inline]
call_netdevice_notifiers net/core/dev.c:1775 [inline]
rollback_registered_many+0x9b9/0xfc0 net/core/dev.c:8206
rollback_registered+0x109/0x1d0 net/core/dev.c:8248
unregister_netdevice_queue net/core/dev.c:9295 [inline]
unregister_netdevice_queue+0x1ee/0x2c0 net/core/dev.c:9288
br_dev_delete+0x145/0x1a0 net/bridge/br_if.c:383
br_del_bridge+0xd7/0x120 net/bridge/br_if.c:483
br_ioctl_deviceless_stub+0x2a4/0x7b0 net/bridge/br_ioctl.c:376
sock_ioctl+0x44b/0x780 net/socket.c:1141
vfs_ioctl fs/ioctl.c:46 [inline]
file_ioctl fs/ioctl.c:509 [inline]
do_vfs_ioctl+0xd5f/0x1380 fs/ioctl.c:696
ksys_ioctl+0xab/0xd0 fs/ioctl.c:713
__do_sys_ioctl fs/ioctl.c:720 [inline]
__se_sys_ioctl fs/ioctl.c:718 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:718
do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
-> #1 (&net->xdp.lock){+.+.}:
__mutex_lock_common kernel/locking/mutex.c:926 [inline]
__mutex_lock+0xf7/0x1310 kernel/locking/mutex.c:1073
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1088
xsk_notifier+0xa7/0x290 net/xdp/xsk.c:726
notifier_call_chain+0xc2/0x230 kernel/notifier.c:95
__raw_notifier_call_chain kernel/notifier.c:396 [inline]
raw_notifier_call_chain+0x2e/0x40 kernel/notifier.c:403
call_netdevice_notifiers_info+0x3f/0x90 net/core/dev.c:1749
call_netdevice_notifiers_extack net/core/dev.c:1761 [inline]
call_netdevice_notifiers net/core/dev.c:1775 [inline]
rollback_registered_many+0x9b9/0xfc0 net/core/dev.c:8206
rollback_registered+0x109/0x1d0 net/core/dev.c:8248
unregister_netdevice_queue net/core/dev.c:9295 [inline]
unregister_netdevice_queue+0x1ee/0x2c0 net/core/dev.c:9288
br_dev_delete+0x145/0x1a0 net/bridge/br_if.c:383
br_del_bridge+0xd7/0x120 net/bridge/br_if.c:483
br_ioctl_deviceless_stub+0x2a4/0x7b0 net/bridge/br_ioctl.c:376
sock_ioctl+0x44b/0x780 net/socket.c:1141
vfs_ioctl fs/ioctl.c:46 [inline]
file_ioctl fs/ioctl.c:509 [inline]
do_vfs_ioctl+0xd5f/0x1380 fs/ioctl.c:696
ksys_ioctl+0xab/0xd0 fs/ioctl.c:713
__do_sys_ioctl fs/ioctl.c:720 [inline]
__se_sys_ioctl fs/ioctl.c:718 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:718
do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
-> #0 (rtnl_mutex){+.+.}:
lock_acquire+0x16f/0x3f0 kernel/locking/lockdep.c:4303
__mutex_lock_common kernel/locking/mutex.c:926 [inline]
__mutex_lock+0xf7/0x1310 kernel/locking/mutex.c:1073
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1088
rtnl_lock+0x17/0x20 net/core/rtnetlink.c:72
xdp_umem_assign_dev+0xbe/0x8b0 net/xdp/xdp_umem.c:96
xsk_bind+0x4d7/0xe70 net/xdp/xsk.c:488
__sys_bind+0x239/0x290 net/socket.c:1653
__do_sys_bind net/socket.c:1664 [inline]
__se_sys_bind net/socket.c:1662 [inline]
__x64_sys_bind+0x73/0xb0 net/socket.c:1662
do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
other info that might help us debug this:
Chain exists of:
rtnl_mutex --> &net->xdp.lock --> &xs->mutex
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&xs->mutex);
lock(&net->xdp.lock);
lock(&xs->mutex);
lock(rtnl_mutex);
*** DEADLOCK ***
1 lock held by syz-executor613/9114:
#0: 0000000039d6ee9b (&xs->mutex){+.+.}, at: xsk_bind+0x16a/0xe70
net/xdp/xsk.c:422
stack backtrace:
CPU: 1 PID: 9114 Comm: syz-executor613 Not tainted 5.2.0-rc6+ #76
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+0x172/0x1f0 lib/dump_stack.c:113
print_circular_bug.cold+0x1cc/0x28f kernel/locking/lockdep.c:1565
check_prev_add kernel/locking/lockdep.c:2310 [inline]
check_prevs_add kernel/locking/lockdep.c:2418 [inline]
validate_chain kernel/locking/lockdep.c:2800 [inline]
__lock_acquire+0x3755/0x5490 kernel/locking/lockdep.c:3793
lock_acquire+0x16f/0x3f0 kernel/locking/lockdep.c:4303
__mutex_lock_common kernel/locking/mutex.c:926 [inline]
__mutex_lock+0xf7/0x1310 kernel/locking/mutex.c:1073
mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1088
rtnl_lock+0x17/0x20 net/core/rtnetlink.c:72
xdp_umem_assign_dev+0xbe/0x8b0 net/xdp/xdp_umem.c:96
xsk_bind+0x4d7/0xe70 net/xdp/xsk.c:488
__sys_bind+0x239/0x290 net/socket.c:1653
__do_sys_bind net/socket.c:1664 [inline]
__se_sys_bind net/socket.c:1662 [inline]
__x64_sys_bind+0x73/0xb0 net/socket.c:1662
do_syscall_64+0xfd/0x680 arch/x86/entry/common.c:301
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x447909
Code: e8 cc e7 ff ff 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 3b 08 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fb2a478fd98 EFLAGS: 00000246 ORIG_RAX: 0000000000000031
RAX: ffffffffffffffda RBX: 00000000006dcc58 RCX: 0000000000447909
RDX: 0000000000000010 RSI: 0000000020000040 RDI: 0000000000000005
RBP: 00000000006dcc50 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000006dcc5c
R13: 0000003066736362 R14: 0000000000000000 R15: 0000003066736362
---
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#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* WARNING: held lock freed! (2)
From: syzbot @ 2019-07-08 19:37 UTC (permalink / raw)
To: davem, linux-hams, linux-kernel, netdev, ralf, syzkaller-bugs,
xiyou.wangcong
Hello,
syzbot found the following crash on:
HEAD commit: 9d1bc24b bonding: validate ip header before check IPPROTO_..
git tree: net
console output: https://syzkaller.appspot.com/x/log.txt?x=152fab25a00000
kernel config: https://syzkaller.appspot.com/x/.config?x=e7c31a94f66cc0aa
dashboard link: https://syzkaller.appspot.com/bug?extid=e54ed2cb16c6da22c549
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11ad60bba00000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=165d2453a00000
The bug was bisected to:
commit c8c8218ec5af5d2598381883acbefbf604e56b5e
Author: Cong Wang <xiyou.wangcong@gmail.com>
Date: Thu Jun 27 21:30:58 2019 +0000
netrom: fix a memory leak in nr_rx_frame()
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1489854ba00000
final crash: https://syzkaller.appspot.com/x/report.txt?x=1689854ba00000
console output: https://syzkaller.appspot.com/x/log.txt?x=1289854ba00000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+e54ed2cb16c6da22c549@syzkaller.appspotmail.com
Fixes: c8c8218ec5af ("netrom: fix a memory leak in nr_rx_frame()")
=========================
WARNING: held lock freed!
5.2.0-rc6+ #75 Not tainted
-------------------------
syz-executor315/8559 is freeing memory ffff88809faed2c0-ffff88809faedabf,
with a lock still held there!
00000000cf45dbdb (sk_lock-AF_NETROM){+.+.}, at: lock_sock
include/net/sock.h:1522 [inline]
00000000cf45dbdb (sk_lock-AF_NETROM){+.+.}, at: nr_release+0x11a/0x3b0
net/netrom/af_netrom.c:522
2 locks held by syz-executor315/8559:
#0: 00000000c0a19dcd (&sb->s_type->i_mutex_key#11){+.+.}, at: inode_lock
include/linux/fs.h:778 [inline]
#0: 00000000c0a19dcd (&sb->s_type->i_mutex_key#11){+.+.}, at:
__sock_release+0x89/0x2a0 net/socket.c:600
#1: 00000000cf45dbdb (sk_lock-AF_NETROM){+.+.}, at: lock_sock
include/net/sock.h:1522 [inline]
#1: 00000000cf45dbdb (sk_lock-AF_NETROM){+.+.}, at: nr_release+0x11a/0x3b0
net/netrom/af_netrom.c:522
stack backtrace:
CPU: 0 PID: 8559 Comm: syz-executor315 Not tainted 5.2.0-rc6+ #75
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+0x172/0x1f0 lib/dump_stack.c:113
print_freed_lock_bug kernel/locking/lockdep.c:5077 [inline]
debug_check_no_locks_freed.cold+0x9d/0xa9 kernel/locking/lockdep.c:5110
kfree+0xb1/0x220 mm/slab.c:3752
sk_prot_free net/core/sock.c:1636 [inline]
__sk_destruct+0x4f7/0x6e0 net/core/sock.c:1722
sk_destruct+0x7b/0x90 net/core/sock.c:1730
__sk_free+0xce/0x300 net/core/sock.c:1741
sk_free+0x42/0x50 net/core/sock.c:1752
sock_put include/net/sock.h:1725 [inline]
nr_destroy_socket+0x3df/0x4a0 net/netrom/af_netrom.c:288
nr_release+0x323/0x3b0 net/netrom/af_netrom.c:530
__sock_release+0xce/0x2a0 net/socket.c:601
sock_close+0x1b/0x30 net/socket.c:1273
__fput+0x2ff/0x890 fs/file_table.c:280
____fput+0x16/0x20 fs/file_table.c:313
task_work_run+0x145/0x1c0 kernel/task_work.c:113
tracehook_notify_resume include/linux/tracehook.h:185 [inline]
exit_to_usermode_loop+0x273/0x2c0 arch/x86/entry/common.c:168
prepare_exit_to_usermode arch/x86/entry/common.c:199 [inline]
syscall_return_slowpath arch/x86/entry/common.c:279 [inline]
do_syscall_64+0x58e/0x680 arch/x86/entry/common.c:304
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x447269
Code: e8 7c 14 03 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 cb 0e fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f653a7bed88 EFLAGS: 00000246 ORIG_RAX: 000000000000002d
RAX: ffffffffffffff95 RBX: 00000000006dcc48 RCX: 0000000000447269
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003
RBP: 00000000006dcc40 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000006dcc4c
R13: 0000003066736362 R14: 002cc7eb47000000 R15: 0000003066736362
==================================================================
BUG: KASAN: use-after-free in debug_spin_lock_before
kernel/locking/spinlock_debug.c:83 [inline]
BUG: KASAN: use-after-free in do_raw_spin_lock+0x28a/0x2e0
kernel/locking/spinlock_debug.c:112
Read of size 4 at addr ffff88809faed34c by task syz-executor315/8559
CPU: 1 PID: 8559 Comm: syz-executor315 Not tainted 5.2.0-rc6+ #75
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+0x172/0x1f0 lib/dump_stack.c:113
print_address_description.cold+0x7c/0x20d mm/kasan/report.c:188
__kasan_report.cold+0x1b/0x40 mm/kasan/report.c:317
kasan_report+0x12/0x20 mm/kasan/common.c:614
__asan_report_load4_noabort+0x14/0x20 mm/kasan/generic_report.c:131
debug_spin_lock_before kernel/locking/spinlock_debug.c:83 [inline]
do_raw_spin_lock+0x28a/0x2e0 kernel/locking/spinlock_debug.c:112
__raw_spin_lock_bh include/linux/spinlock_api_smp.h:136 [inline]
_raw_spin_lock_bh+0x3b/0x50 kernel/locking/spinlock.c:175
spin_lock_bh include/linux/spinlock.h:343 [inline]
release_sock+0x20/0x1c0 net/core/sock.c:2928
nr_release+0x2df/0x3b0 net/netrom/af_netrom.c:553
__sock_release+0xce/0x2a0 net/socket.c:601
sock_close+0x1b/0x30 net/socket.c:1273
__fput+0x2ff/0x890 fs/file_table.c:280
____fput+0x16/0x20 fs/file_table.c:313
task_work_run+0x145/0x1c0 kernel/task_work.c:113
tracehook_notify_resume include/linux/tracehook.h:185 [inline]
exit_to_usermode_loop+0x273/0x2c0 arch/x86/entry/common.c:168
prepare_exit_to_usermode arch/x86/entry/common.c:199 [inline]
syscall_return_slowpath arch/x86/entry/common.c:279 [inline]
do_syscall_64+0x58e/0x680 arch/x86/entry/common.c:304
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x447269
Code: e8 7c 14 03 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 cb 0e fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f653a7bed88 EFLAGS: 00000246 ORIG_RAX: 000000000000002d
RAX: ffffffffffffff95 RBX: 00000000006dcc48 RCX: 0000000000447269
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003
RBP: 00000000006dcc40 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000006dcc4c
R13: 0000003066736362 R14: 002cc7eb47000000 R15: 0000003066736362
Allocated by task 8562:
save_stack+0x23/0x90 mm/kasan/common.c:71
set_track mm/kasan/common.c:79 [inline]
__kasan_kmalloc mm/kasan/common.c:489 [inline]
__kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:462
kasan_kmalloc+0x9/0x10 mm/kasan/common.c:503
__do_kmalloc mm/slab.c:3660 [inline]
__kmalloc+0x15c/0x740 mm/slab.c:3669
kmalloc include/linux/slab.h:552 [inline]
sk_prot_alloc+0x19c/0x2e0 net/core/sock.c:1599
sk_alloc+0x39/0xf70 net/core/sock.c:1653
nr_make_new net/netrom/af_netrom.c:476 [inline]
nr_rx_frame+0x733/0x1e70 net/netrom/af_netrom.c:959
nr_loopback_timer+0x7b/0x170 net/netrom/nr_loopback.c:59
call_timer_fn+0x193/0x720 kernel/time/timer.c:1322
expire_timers kernel/time/timer.c:1366 [inline]
__run_timers kernel/time/timer.c:1685 [inline]
__run_timers kernel/time/timer.c:1653 [inline]
run_timer_softirq+0x66f/0x1740 kernel/time/timer.c:1698
__do_softirq+0x25c/0x94c kernel/softirq.c:292
Freed by task 8559:
save_stack+0x23/0x90 mm/kasan/common.c:71
set_track mm/kasan/common.c:79 [inline]
__kasan_slab_free+0x102/0x150 mm/kasan/common.c:451
kasan_slab_free+0xe/0x10 mm/kasan/common.c:459
__cache_free mm/slab.c:3432 [inline]
kfree+0xcf/0x220 mm/slab.c:3755
sk_prot_free net/core/sock.c:1636 [inline]
__sk_destruct+0x4f7/0x6e0 net/core/sock.c:1722
sk_destruct+0x7b/0x90 net/core/sock.c:1730
__sk_free+0xce/0x300 net/core/sock.c:1741
sk_free+0x42/0x50 net/core/sock.c:1752
sock_put include/net/sock.h:1725 [inline]
nr_destroy_socket+0x3df/0x4a0 net/netrom/af_netrom.c:288
nr_release+0x323/0x3b0 net/netrom/af_netrom.c:530
__sock_release+0xce/0x2a0 net/socket.c:601
sock_close+0x1b/0x30 net/socket.c:1273
__fput+0x2ff/0x890 fs/file_table.c:280
____fput+0x16/0x20 fs/file_table.c:313
task_work_run+0x145/0x1c0 kernel/task_work.c:113
tracehook_notify_resume include/linux/tracehook.h:185 [inline]
exit_to_usermode_loop+0x273/0x2c0 arch/x86/entry/common.c:168
prepare_exit_to_usermode arch/x86/entry/common.c:199 [inline]
syscall_return_slowpath arch/x86/entry/common.c:279 [inline]
do_syscall_64+0x58e/0x680 arch/x86/entry/common.c:304
entry_SYSCALL_64_after_hwframe+0x49/0xbe
The buggy address belongs to the object at ffff88809faed2c0
which belongs to the cache kmalloc-2k of size 2048
The buggy address is located 140 bytes inside of
2048-byte region [ffff88809faed2c0, ffff88809faedac0)
The buggy address belongs to the page:
page:ffffea00027ebb00 refcount:1 mapcount:0 mapping:ffff8880aa400c40
index:0x0 compound_mapcount: 0
flags: 0x1fffc0000010200(slab|head)
raw: 01fffc0000010200 ffffea000242cd08 ffffea00023df908 ffff8880aa400c40
raw: 0000000000000000 ffff88809faec1c0 0000000100000003 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff88809faed200: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
ffff88809faed280: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
> ffff88809faed300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88809faed380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88809faed400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH v3 net-next 19/19] ionic: Add basic devlink interface
From: Jiri Pirko @ 2019-07-08 19:34 UTC (permalink / raw)
To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190708192532.27420-20-snelson@pensando.io>
Mon, Jul 08, 2019 at 09:25:32PM CEST, snelson@pensando.io wrote:
>Add a devlink interface for access to information that isn't
>normally available through ethtool or the iplink interface.
>
>Example:
> $ ./devlink -j -p dev info pci/0000:b6:00.0
> {
> "info": {
> "pci/0000:b6:00.0": {
> "driver": "ionic",
> "serial_number": "FLM18420073",
> "versions": {
> "fixed": {
> "fw_version": "0.11.0-50",
> "fw_status": "0x1",
> "fw_heartbeat": "0x716ce",
> "asic_type": "0x0",
> "asic_rev": "0x0"
> }
> }
> }
> }
> }
>
>Signed-off-by: Shannon Nelson <snelson@pensando.io>
>---
> drivers/net/ethernet/pensando/ionic/Makefile | 2 +-
> drivers/net/ethernet/pensando/ionic/ionic.h | 1 +
> .../ethernet/pensando/ionic/ionic_bus_pci.c | 7 ++
> .../ethernet/pensando/ionic/ionic_devlink.c | 89 +++++++++++++++++++
> .../ethernet/pensando/ionic/ionic_devlink.h | 12 +++
> 5 files changed, 110 insertions(+), 1 deletion(-)
> create mode 100644 drivers/net/ethernet/pensando/ionic/ionic_devlink.c
> create mode 100644 drivers/net/ethernet/pensando/ionic/ionic_devlink.h
>
>diff --git a/drivers/net/ethernet/pensando/ionic/Makefile b/drivers/net/ethernet/pensando/ionic/Makefile
>index 4f3cfbf36c23..ce187c7b33a8 100644
>--- a/drivers/net/ethernet/pensando/ionic/Makefile
>+++ b/drivers/net/ethernet/pensando/ionic/Makefile
>@@ -5,4 +5,4 @@ obj-$(CONFIG_IONIC) := ionic.o
>
> ionic-y := ionic_main.o ionic_bus_pci.o ionic_dev.o ionic_ethtool.o \
> ionic_lif.o ionic_rx_filter.o ionic_txrx.o ionic_debugfs.o \
>- ionic_stats.o
>+ ionic_stats.o ionic_devlink.o
>diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
>index cd08166f73a9..a0034bc5b4a1 100644
>--- a/drivers/net/ethernet/pensando/ionic/ionic.h
>+++ b/drivers/net/ethernet/pensando/ionic/ionic.h
>@@ -44,6 +44,7 @@ struct ionic {
> DECLARE_BITMAP(intrs, INTR_CTRL_REGS_MAX);
> struct work_struct nb_work;
> struct notifier_block nb;
>+ struct devlink *dl;
> };
>
> struct ionic_admin_ctx {
>diff --git a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>index 98c12b770c7f..a8c99254489f 100644
>--- a/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>+++ b/drivers/net/ethernet/pensando/ionic/ionic_bus_pci.c
>@@ -10,6 +10,7 @@
> #include "ionic_bus.h"
> #include "ionic_lif.h"
> #include "ionic_debugfs.h"
>+#include "ionic_devlink.h"
>
> /* Supported devices */
> static const struct pci_device_id ionic_id_table[] = {
>@@ -212,9 +213,14 @@ static int ionic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> goto err_out_deinit_lifs;
> }
>
>+ err = ionic_devlink_register(ionic);
>+ if (err)
>+ dev_err(dev, "Cannot register devlink (ignored): %d\n", err);
>+
> return 0;
>
> err_out_deinit_lifs:
>+ ionic_devlink_unregister(ionic);
> ionic_lifs_deinit(ionic);
> err_out_free_lifs:
> ionic_lifs_free(ionic);
>@@ -247,6 +253,7 @@ static void ionic_remove(struct pci_dev *pdev)
> struct ionic *ionic = pci_get_drvdata(pdev);
>
> if (ionic) {
>+ ionic_devlink_unregister(ionic);
> ionic_lifs_unregister(ionic);
> ionic_lifs_deinit(ionic);
> ionic_lifs_free(ionic);
>diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
>new file mode 100644
>index 000000000000..fbbfcdde292f
>--- /dev/null
>+++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
>@@ -0,0 +1,89 @@
>+// SPDX-License-Identifier: GPL-2.0
>+/* Copyright(c) 2017 - 2019 Pensando Systems, Inc */
>+
>+#include <linux/module.h>
>+#include <linux/netdevice.h>
>+
>+#include "ionic.h"
>+#include "ionic_bus.h"
>+#include "ionic_lif.h"
>+#include "ionic_devlink.h"
>+
>+struct ionic_devlink {
>+ struct ionic *ionic;
>+};
>+
>+static int ionic_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
>+ struct netlink_ext_ack *extack)
>+{
>+ struct ionic *ionic = *(struct ionic **)devlink_priv(dl);
>+ struct ionic_dev *idev = &ionic->idev;
>+ char buf[16];
>+ u32 val;
>+
>+ devlink_info_driver_name_put(req, DRV_NAME);
>+
>+ devlink_info_version_fixed_put(req, "fw_version",
>+ idev->dev_info.fw_version);
>+
>+ val = ioread8(&idev->dev_info_regs->fw_status);
>+ snprintf(buf, sizeof(buf), "0x%x", val);
>+ devlink_info_version_fixed_put(req, "fw_status", buf);
>+
>+ val = ioread32(&idev->dev_info_regs->fw_heartbeat);
>+ snprintf(buf, sizeof(buf), "0x%x", val);
>+ devlink_info_version_fixed_put(req, "fw_heartbeat", buf);
>+
>+ snprintf(buf, sizeof(buf), "0x%x", idev->dev_info.asic_type);
>+ devlink_info_version_fixed_put(req, "asic_type", buf);
>+
>+ snprintf(buf, sizeof(buf), "0x%x", idev->dev_info.asic_rev);
>+ devlink_info_version_fixed_put(req, "asic_rev", buf);
>+
>+ devlink_info_serial_number_put(req, idev->dev_info.serial_num);
>+
>+ return 0;
>+}
>+
>+static const struct devlink_ops ionic_dl_ops = {
>+ .info_get = ionic_dl_info_get,
>+};
>+
>+int ionic_devlink_register(struct ionic *ionic)
>+{
>+ struct devlink *dl;
>+ struct ionic **ip;
>+ int err;
>+
>+ dl = devlink_alloc(&ionic_dl_ops, sizeof(struct ionic *));
Oups. Something is wrong with your flow. The devlink alloc is allocating
the structure that holds private data (per-device data) for you. This is
misuse :/
You are missing one parent device struct apparently.
Oh, I think I see something like it. The unused "struct ionic_devlink".
>+ if (!dl) {
>+ dev_warn(ionic->dev, "devlink_alloc failed");
>+ return -ENOMEM;
>+ }
>+
>+ ip = (struct ionic **)devlink_priv(dl);
>+ *ip = ionic;
>+ ionic->dl = dl;
>+
>+ err = devlink_register(dl, ionic->dev);
>+ if (err) {
>+ dev_warn(ionic->dev, "devlink_register failed: %d\n", err);
>+ goto err_dl_free;
>+ }
>+
>+ return 0;
>+
>+err_dl_free:
>+ ionic->dl = NULL;
>+ devlink_free(dl);
>+ return err;
>+}
>+
>+void ionic_devlink_unregister(struct ionic *ionic)
>+{
>+ if (!ionic->dl)
>+ return;
>+
>+ devlink_unregister(ionic->dl);
>+ devlink_free(ionic->dl);
>+}
>diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.h b/drivers/net/ethernet/pensando/ionic/ionic_devlink.h
>new file mode 100644
>index 000000000000..35528884e29f
>--- /dev/null
>+++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.h
>@@ -0,0 +1,12 @@
>+/* SPDX-License-Identifier: GPL-2.0 */
>+/* Copyright(c) 2017 - 2019 Pensando Systems, Inc */
>+
>+#ifndef _IONIC_DEVLINK_H_
>+#define _IONIC_DEVLINK_H_
>+
>+#include <net/devlink.h>
>+
>+int ionic_devlink_register(struct ionic *ionic);
>+void ionic_devlink_unregister(struct ionic *ionic);
>+
>+#endif /* _IONIC_DEVLINK_H_ */
>--
>2.17.1
>
^ 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