* Need help with mdiobus_register and phy
From: Timur Tabi @ 2016-10-13 23:15 UTC (permalink / raw)
To: netdev
I need help with a program I've discovered with my driver
(ethernet/qualcomm/emac/) that occurs in the following process:
1. Load the driver
1a. Driver calls mdiobus_register (see emac_phy_config), which calls
mdiobus_scan(), which calls get_phy_device()
1b. get_phy_device() performs MDIO bus operations, which means the bus
and the external phy need to be active.
2. Bring up the interface
3. Bring down the interface. This calls phy_disconnect() which calls
phy_detach() which calls phy_suspend(). This puts the external phy to
sleep.
4. Unload the driver
5. Re-load the driver.
5a. Driver calls mdiobus_register again, but this time the call to
get_phy_device() fails because the external phy is asleep and does not
respond to MDIO transactions.
I don't know what my driver is doing wrong. I'm concerned that
mdiobus_register() is causing MDIO transactions to occur before the rest
of the EMAC has a chance to initialize. I've examined many other
drivers, and I think I'm initializing things in the proper order.
The call to mdiobus_register() occurs before I know what phydev is.
Since I need the phydev in order to talk to the phy driver, I don't see
how I can "wake up" the external phy before calling mdiobus_register().
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [mac80211] BUG_ON with current -git (4.8.0-11417-g24532f7)
From: Andy Lutomirski @ 2016-10-13 21:49 UTC (permalink / raw)
To: Johannes Berg
Cc: Stephen Rothwell, linux-next@vger.kernel.org, Sergey Senozhatsky,
Network Development, Sergey Senozhatsky, Herbert Xu,
David S. Miller, Linux Wireless List,
linux-kernel@vger.kernel.org
In-Reply-To: <1476366354.4904.31.camel@sipsolutions.net>
On Oct 13, 2016 6:46 AM, "Johannes Berg" <johannes@sipsolutions.net> wrote:
>
> On Thu, 2016-10-13 at 22:42 +0900, Sergey Senozhatsky wrote:
> >
> > > > https://git.kernel.org/cgit/linux/kernel/git/luto/linux.git/commi
> > > > t/?h=x86/vmap_stack&id=0a39cfa6fbb5d5635c85253cc7d6b44b54822afd
> > > > https://git.kernel.org/cgit/linux/kernel/git/luto/linux.git/commi
> > > > t/?h=x86/vmap_stack&id=bf8cfa200b5a01383ea39fc8ce2f32909767baa8
> > >
> > > That truly sounds like something we'd rather avoid in the TX/RX
> > > paths though, which should perform well.
> >
> > didn't fix.
>
> It couldn't, since the new helpers weren't used in mac80211 in those
> patches yet.
>
> > so I finally had some time to do a better bug-reporter job.
> >
> > I added a bunch of printk-s and several virt_addr_valid()-s
> > to ieee80211_aes_ccm_encrypt().
> >
> > and right befoe the Oops I see the following report from
> > virt_addr_valid()
> >
> >
> > FAIL: 00004100002cba02 > ffffc900802cba02 || 1 -> (00004100002cba02
> > >> 39) == 130
>
> Yeah, we already know that in this function the aad variable is on the
> stack, it explicitly is.
>
> The question, though, is why precisely that fails in the crypto code.
> Can you send the Oops report itself?
>
It's failing before that. With CONFIG_VMAP_STACK=y, the stack may not
be physically contiguous and can't be used for DMA, so putting it in a
scatterlist is bogus in general, and the crypto code mostly wants a
scatterlist.
There are a couple (faster!) APIs for crypto that don't use
scatterlists, but I don't think AEAD works with them.
--Andy
^ permalink raw reply
* Re: [PATCH] net: Require exact match for TCP socket lookups if dif is l3mdev
From: David Ahern @ 2016-10-13 21:39 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1476394191.5650.29.camel@edumazet-glaptop3.roam.corp.google.com>
On 10/13/16 3:29 PM, Eric Dumazet wrote:
> Since netif_index_is_l3_master() is not cheap, can you reorder the
> test ?
>
> if (!net->ipv4.sysctl_tcp_l3mdev_accept)
> return netif_index_is_l3_master(net, dif);
sure. Since this use case is called under rcu_read_lock I can make a netif_index_is_l3_master_rcu which saves a few cycles.
^ permalink raw reply
* Re: slab corruption with current -git
From: Al Viro @ 2016-10-13 21:32 UTC (permalink / raw)
To: Linus Torvalds
Cc: Markus Trippelsdorf, Christoph Lameter, Jens Axboe,
Linux Kernel Mailing List, Aaron Conole, David Miller,
Pablo Neira Ayuso, linux-fsdevel, NetFilter, Network Development,
Andrew Morton, Florian Westphal, Theodore Ts'o
In-Reply-To: <CA+55aFynWiz1UWwQVvFrRsVvgcYZ=ctCPou9BL1b9_DOVTqKOQ@mail.gmail.com>
On Thu, Oct 13, 2016 at 12:49:33PM -0700, Linus Torvalds wrote:
> That said, xt_hook_ops_alloc() itself is odd. Lookie here, this is the
> loop that initializes things:
>
> for (i = 0, hooknum = 0; i < num_hooks && hook_mask != 0;
> hook_mask >>= 1, ++hooknum) {
>
> and it makes no sense to me how that tests *both* "i < num_hools" and
> "hook_mask != 0".
>
> Why? Because
>
> num_hooks = hweight32(hook_mask);
>
> so it's entirely redundant. num_hooks is already how many bits are on
> in hook_mask, so that test is just duplicating the same thing twice
> ("have we done less than that number of bits" and "do we have any bits
> less").
>
> I don't know. There's something odd going on. Regardless, thsi is a
> different problem from the nf_register_net_hook() list handling, so
> I'll leave it to the networking people. David?
Hey, I remember looking through that stuff. <checks> There it is, in
a thread started by Krause Randomness(tm)... Short version: nf_hook_ops
is a mess - it's embedded into different objects, with different subsets
of fields used depending on the containing object and I would seriously
suggest moving some of those into those containing objects.
--------------------------------------------------------------------------
On Thu, Sep 01, 2016 at 08:10:44AM -0500, Eric Sandeen wrote:
> On 8/4/16 8:57 AM, Al Viro wrote:
>
> > Don't feed the troll. On all paths leading to that place we have
> > result->name = kname;
> > len = strncpy_from_user(kname, filename, EMBEDDED_NAME_MAX);
> > or
> > result->name = kname;
> > len = strncpy_from_user(kname, filename, PATH_MAX);
> > with failure exits taken if strncpy_from_user() returns an error, which means
> > that the damn thing has already been copied into.
> >
> > FWIW, it looks a lot like buggered kmemcheck; as usual, he can't be bothered
> > to mention which kernel version would it be (let alone how to reproduce it
> > on the kernel in question), but IIRC davej had run into some instrumentation
> > breakage lately.
>
> The original report is in https://bugzilla.kernel.org/show_bug.cgi?id=120651
> if anyone is interested in it.
What the hell does that one have to getname_flags(), other than having
attracted the same... something on the edge of failing the Turing Test?
FWIW, looking at the netfilter one... That's nf_register_net_hook()
hitting
entry->ops = *reg;
with reg pointing to something uninitialized (according to kmemcheck, that is,
and presuming that it's not an instrumentation bug). With the callchain
in report, it came (all in the same assumptions) from
nf_register_net_hooks(net, ops, hweight32(table->valid_hooks))
with hweight32(table->valid_hooks) being greater than the amount of
initialized entries in ops[] (call site in ipt_register_table()).
This "ops" ought to be net/ipv4/netfilter/iptable_filter.c:filter_ops,
allocated by
filter_ops = xt_hook_ops_alloc(&packet_filter, iptable_filter_hook);
in iptable_filter_init(). "table" is &packet_filter and its contents ought
to be unchanged, so ->valid_hooks in there is FILTER_VALID_HOOKS, i.e.
((1 << NF_INET_LOCAL_IN) | (1 << NF_INET_FORWARD) | (1 << NF_INET_LOCAL_OUT)).
Which is to say, filter_ops[] had fewer than 3 initialized elements
when it got to the call of iptable_filter_table_init()... Since filter_ops
hadn't been NULL, the xt_hook_ops_alloc() call above must've already been
done. Said xt_hook_ops_alloc() should've allocated a 3-element array and
hooked through all of it, so it's not a wholesale uninitialized element, it's
uninitialized parts of one...
What gets initialized is ->hook, ->pf, ->hooknum and ->priority.
Let's figure out the offsets:
0: list (two pointers, i.e. 16 bytes)
0x10: hook (8)
0x18: dev (8)
0x20: priv (8)
0x28: pf (1)
0x29: padding (3)
0x2c: hooknum (4)
0x30: priority (4)
0x34: padding (8)
OK... The address of the damn thing is apparently ffff880037b4bd80 and
we see complaint about the accesses at offsets 0, 0x18, 8, 0x20 and then
the same pattern with 0x38 and 0x70 added (i.e. the same fields in the next
two elements of the same array). Then there are similar complaints, but
with a different call chain (iptable_mangle instead of iptable_filter).
These offsets are ->list, ->dev and ->priv, and those are exactly the ones
not initialized by xt_hook_ops_alloc(). Looking at the nf_register_net_hook(),
we have
list_add_rcu(&entry->ops.list, elem->list.prev);
a bit further down the road. ->dev and ->priv are left uninitialized (and
very likely - unused).
I would say it's a false positive. struct nf_hook_ops is embedded into a
bunch of different objects, with different subsets of fields getting used.
IMO it's a bad idea (in particular, I really wonder if ->list would've
been better off moved into (some of) the containing suckers), but it's
not a bug per se, just a design choice asking for trouble. One way of
getting kmemcheck off your back would be to switch xt_hook_ops_alloc() from
ops = kmalloc(sizeof(*ops) * num_hooks, GFP_KERNEL);
to
ops = kcalloc(num_hooks, sizeof(*ops), GFP_KERNEL);
which might have some merits beyond making kmemcheck STFU...
--------------------------------------------------------------------------
^ permalink raw reply
* Re: [PATCH] net: Require exact match for TCP socket lookups if dif is l3mdev
From: Eric Dumazet @ 2016-10-13 21:29 UTC (permalink / raw)
To: David Ahern; +Cc: netdev
In-Reply-To: <1476389690-31211-1-git-send-email-dsa@cumulusnetworks.com>
On Thu, 2016-10-13 at 13:14 -0700, David Ahern wrote:
> Currently, socket lookups for l3mdev (vrf) use cases can match a socket
> that is bound to a port but not a device (ie., a global socket). If the
> sysctl tcp_l3mdev_accept is not set this leads to ack packets going out
> based on the main table even though the packet came in from an L3 domain.
> The end result is that the connection does not establish creating
> confusion for users since the service is running and a socket shows in
> ss output. Fix by requiring an exact dif to sk_bound_dev_if match if the
> dif is an l3mdev device and the tcp_l3mdev_accept is not set.
>
> Fixes: 193125dbd8eb ("net: Introduce VRF device driver")
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
> ---
> include/net/inet_sock.h | 10 ++++++++++
> net/ipv4/inet_hashtables.c | 7 ++++---
> net/ipv6/inet6_hashtables.c | 7 ++++---
> 3 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
> index 236a81034fef..abb4e720a769 100644
> --- a/include/net/inet_sock.h
> +++ b/include/net/inet_sock.h
> @@ -132,6 +132,16 @@ static inline int inet_request_bound_dev_if(const struct sock *sk,
> return sk->sk_bound_dev_if;
> }
>
> +static inline bool inet_exact_dif_match(struct net *net, int dif)
> +{
> +#ifdef CONFIG_NET_L3_MASTER_DEV
> + if (netif_index_is_l3_master(net, dif) &&
> + !net->ipv4.sysctl_tcp_l3mdev_accept)
> + return true;
Since netif_index_is_l3_master() is not cheap, can you reorder the
test ?
if (!net->ipv4.sysctl_tcp_l3mdev_accept)
return netif_index_is_l3_master(net, dif);
^ permalink raw reply
* Re: [PATCH net] net_sched: do not broadcast RTM_GETTFILTER result
From: Eric Dumazet @ 2016-10-13 21:08 UTC (permalink / raw)
To: David Miller; +Cc: netdev, jhs
In-Reply-To: <20161013.095403.1806984759673575482.davem@davemloft.net>
On Thu, 2016-10-13 at 09:54 -0400, David Miller wrote:
> Applied, thanks Eric.
>
> Want me to queue this up for -stable too?
No thanks, it just occurred to me during a debugging session.
^ permalink raw reply
* Re: [PATCH] net: limit a number of namespaces which can be cleaned up concurrently
From: Andrei Vagin @ 2016-10-13 20:44 UTC (permalink / raw)
To: Eric W. Biederman
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Andrei Vagin, David S. Miller
In-Reply-To: <871szk9rl9.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
On Thu, Oct 13, 2016 at 10:49:38AM -0500, Eric W. Biederman wrote:
> Andrei Vagin <avagin@openvz.org> writes:
>
> > From: Andrey Vagin <avagin@openvz.org>
> >
> > The operation of destroying netns is heavy and it is executed under
> > net_mutex. If many namespaces are destroyed concurrently, net_mutex can
> > be locked for a long time. It is impossible to create a new netns during
> > this period of time.
>
> This may be the right approach or at least the right approach to bound
> net_mutex hold times but I have to take exception to calling network
> namespace cleanup heavy.
>
> The only particularly time consuming operation I have ever found are calls to
> synchronize_rcu/sycrhonize_sched/synchronize_net.
I booted the kernel with maxcpus=1, in this case these functions work
very fast and the problem is there any way.
Accoding to perf, we spend a lot of time in kobject_uevent:
- 99.96% 0.00% kworker/u4:1 [kernel.kallsyms] [k] unregister_netdevice_many ▒
- unregister_netdevice_many ◆
- 99.95% rollback_registered_many ▒
- 99.64% netdev_unregister_kobject ▒
- 33.43% netdev_queue_update_kobjects ▒
- 33.40% kobject_put ▒
- kobject_release ▒
+ 33.37% kobject_uevent ▒
+ 0.03% kobject_del ▒
+ 0.03% sysfs_remove_group ▒
- 33.13% net_rx_queue_update_kobjects ▒
- kobject_put ▒
- kobject_release ▒
+ 33.11% kobject_uevent ▒
+ 0.01% kobject_del ▒
0.00% rx_queue_release ▒
- 33.08% device_del ▒
+ 32.75% kobject_uevent ▒
+ 0.17% device_remove_attrs ▒
+ 0.07% dpm_sysfs_remove ▒
+ 0.04% device_remove_class_symlinks ▒
+ 0.01% kobject_del ▒
+ 0.01% device_pm_remove ▒
+ 0.01% sysfs_remove_file_ns ▒
+ 0.00% klist_del ▒
+ 0.00% driver_deferred_probe_del ▒
0.00% cleanup_glue_dir.isra.14.part.15 ▒
0.00% to_acpi_device_node ▒
0.00% sysfs_remove_group ▒
0.00% klist_del ▒
0.00% device_remove_attrs ▒
+ 0.26% call_netdevice_notifiers_info ▒
+ 0.04% rtmsg_ifinfo_build_skb ▒
+ 0.01% rtmsg_ifinfo_send ▒
0.00% dev_uc_flush ▒
0.00% netif_reset_xps_queues_gt
Someone can listen these uevents, so we can't stop sending them without
breaking backward compatibility. We can try to optimize kobject_uevent...
>
> Ideally we can search those out calls in the network namespace cleanup
> operations and figuroue out how to eliminate those operations or how to
> stack them.
>
> > In our days when userns allows to create network namespaces to
> > unprivilaged users, it may be a real problem.
>
> Sorting out syncrhonize_rcu calls will be a much larger
> and much more effective improvement than your patch here.
>
> > On my laptop (fedora 24, i5-5200U, 12GB) 1000 namespaces requires about
> > 300MB of RAM and are being destroyed for 8 seconds.
> >
> > In this patch, a number of namespaces which can be cleaned up
> > concurrently is limited by 32. net_mutex is released after handling each
> > portion of net namespaces and then it is locked again to handle the next
> > one. It allows other users to lock it without waiting for a long
> > time.
> >
> > I am not sure whether we need to add a sysctl to costomize this limit.
> > Let me know if you think it's required.
>
> We definitely don't need an extra sysctl.
Thanks,
Andrei
>
> Eric
>
>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: "Eric W. Biederman" <ebiederm@xmission.com>
> > Signed-off-by: Andrei Vagin <avagin@openvz.org>
> > ---
> > net/core/net_namespace.c | 12 +++++++++++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> > index 989434f..33dd3b7 100644
> > --- a/net/core/net_namespace.c
> > +++ b/net/core/net_namespace.c
> > @@ -406,10 +406,20 @@ static void cleanup_net(struct work_struct *work)
> > struct net *net, *tmp;
> > struct list_head net_kill_list;
> > LIST_HEAD(net_exit_list);
> > + int i = 0;
> >
> > /* Atomically snapshot the list of namespaces to cleanup */
> > spin_lock_irq(&cleanup_list_lock);
> > - list_replace_init(&cleanup_list, &net_kill_list);
> > + list_for_each_entry_safe(net, tmp, &cleanup_list, cleanup_list)
> > + if (++i == 32)
> > + break;
> > + if (i == 32) {
> > + list_cut_position(&net_kill_list,
> > + &cleanup_list, &net->cleanup_list);
> > + queue_work(netns_wq, work);
> > + } else {
> > + list_replace_init(&cleanup_list, &net_kill_list);
> > + }
> > spin_unlock_irq(&cleanup_list_lock);
> >
> > mutex_lock(&net_mutex);
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers
^ permalink raw reply
* Re: [PATCH net 2/2] conntrack: enable to tune gc parameters
From: Florian Westphal @ 2016-10-13 20:43 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: Florian Westphal, davem, pablo, netdev, netfilter-devel
In-Reply-To: <cdda5290-c49c-e841-6ba5-10b30fd04fc0@6wind.com>
Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> Le 10/10/2016 à 16:04, Florian Westphal a écrit :
> > Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
> >> After commit b87a2f9199ea ("netfilter: conntrack: add gc worker to remove
> >> timed-out entries"), netlink conntrack deletion events may be sent with a
> >> huge delay. It could be interesting to let the user tweak gc parameters
> >> depending on its use case.
> >
> > Hmm, care to elaborate?
> >
> > I am not against doing this but I'd like to hear/read your use case.
> >
> > The expectation is that in almot all cases eviction will happen from
> > packet path. The gc worker is jusdt there for case where a busy system
> > goes idle.
> It was precisely that case. After a period of activity, the event is sent a long
> time after the timeout. If the router does not manage a lot of flows, why not
> trying to parse more entries instead of the default 1/64 of the table?
> In fact, I don't understand why using GC_MAX_BUCKETS_DIV instead of using always
> GC_MAX_BUCKETS whatever the size of the table is.
I wanted to make sure that we have a known upper bound on the number of
buckets we process so that we do not block other pending kworker items
for too long.
(Or cause too many useless scans)
Another idea worth trying might be to get rid of the max cap and
instead break early in case too many jiffies expired.
I don't want to add sysctl knobs for this unless absolutely needed; its already
possible to 'force' eviction cycle by running 'conntrack -L'.
^ permalink raw reply
* Re: slab corruption with current -git
From: Florian Westphal @ 2016-10-13 20:43 UTC (permalink / raw)
To: Linus Torvalds
Cc: Markus Trippelsdorf, Christoph Lameter, Jens Axboe,
Linux Kernel Mailing List, Aaron Conole, David Miller,
Pablo Neira Ayuso, linux-fsdevel, Al Viro, NetFilter,
Network Development, Andrew Morton, Florian Westphal,
Theodore Ts'o
In-Reply-To: <CA+55aFynWiz1UWwQVvFrRsVvgcYZ=ctCPou9BL1b9_DOVTqKOQ@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Wed, Oct 12, 2016 at 11:27 PM, Markus Trippelsdorf
> <markus@trippelsdorf.de> wrote:
> >
> > Yeah.
> >
> > 105 entry->orig_ops = reg;
> > 106 entry->ops = *reg;
> > 107 entry->next = NULL;
>
> So ipt_register_table() does:
>
> ret = nf_register_net_hooks(net, ops, hweight32(table->valid_hooks));
>
> and then nf_register_net_hooks() just does
>
> for (i = 0; i < n; i++) {
> err = nf_register_net_hook(net, ®[i]);
>
> so if the *reg is uninitialized, it means that it's the 'ops[]' array
> that isn't actually really valid in "valid_hooks". Odd. They should
> all be initialized by xt_hook_ops_alloc(), no?
Its only partially initialized. Looking at Markus' splat
its complaining about first 16 bytes (list_head), whose contents are indeed
undefined when it gets copied to entry->ops.
For the time being this seems like the most simple "fix", until we
disentangle the hook description (which should be const) from run-time
allocated data.
diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c
index e0aa7c1d0224..fc4977456c30 100644
--- a/net/netfilter/x_tables.c
+++ b/net/netfilter/x_tables.c
@@ -1513,7 +1513,7 @@ xt_hook_ops_alloc(const struct xt_table *table,
nf_hookfn *fn)
if (!num_hooks)
return ERR_PTR(-EINVAL);
- ops = kmalloc(sizeof(*ops) * num_hooks, GFP_KERNEL);
+ ops = kcalloc(num_hooks, sizeof(*ops), GFP_KERNEL);
if (ops == NULL)
return ERR_PTR(-ENOMEM);
I'll pass such a patch to Pablo.
> That said, xt_hook_ops_alloc() itself is odd. Lookie here, this is the
> loop that initializes things:
>
> for (i = 0, hooknum = 0; i < num_hooks && hook_mask != 0;
> hook_mask >>= 1, ++hooknum) {
>
> and it makes no sense to me how that tests *both* "i < num_hools" and
> "hook_mask != 0".
Right, one of these is enough.
^ permalink raw reply related
* Re: igb driver can cause cache invalidation of non-owned memory?
From: Alexander Duyck @ 2016-10-13 20:32 UTC (permalink / raw)
To: Nikita Yushchenko
Cc: David Laight, Eric Dumazet, David Miller, Jeff Kirsher,
intel-wired-lan, Netdev, linux-kernel@vger.kernel.org,
Chris Healy
In-Reply-To: <070385e5-7cfb-4f87-bb53-9b8549ea2c89@cogentembedded.com>
On Thu, Oct 13, 2016 at 4:00 AM, Nikita Yushchenko
<nikita.yoush@cogentembedded.com> wrote:
>>>> It would make more sense to update the DMA API for
>>>> __dma_page_cpu_to_dev on ARM so that you don't invalidate the cache if
>>>> the direction is DMA_FROM_DEVICE.
>>>
>>> No, in generic case it's unsafe.
>>>
>>> If CPU issued a write to a location, and sometime later that location is
>>> used as DMA buffer, there is danger that write is still in cache only,
>>> and writeback is pending. Later this writeback can overwrite data
>>> written to memory via DMA, causing corruption.
>>
>> Okay so if I understand it correctly then the invalidation in
>> sync_for_device is to force any writes to be flushed out, and the
>> invalidation in sync_for_cpu is to flush out any speculative reads.
>> So without speculative reads then the sync_for_cpu portion is not
>> needed. You might want to determine if the core you are using
>> supports the speculative reads, if not you might be able to get away
>> without having to do the sync_for_cpu at all.
>
> pl310 L2 cache controller does support prefetching - and I think most
> arm systems use pl310.
>
>
>>>> Changing the driver code for this won't necessarily work on all
>>>> architectures, and on top of it we have some changes planned which
>>>> will end up making the pages writable in the future to support the
>>>> ongoing XDP effort. That is one of the reasons why I would not be
>>>> okay with changing the driver to make this work.
>>>
>>> Well I was not really serious about removing that sync_for_device() in
>>> mainline :) Although >20% throughput win that this provides is
>>> impressive...
>>
>> I agree the improvement is pretty impressive. The think is there are
>> similar gains we can generate on x86 by stripping out bits and pieces
>> that are needed for other architectures. I'm just wanting to make
>> certain we aren't optimizing for one architecture at the detriment of
>> others.
>
> Well in ideal world needs of other architectures should not limit x86 -
> and if they do then that's a bug and should be fixed - by designing
> proper set of abstractions :)
>
> Perhaps issue is that "do whatever needed for device to perform DMA
> correctly" semantics of dma_map() / sync_for_device() - and symmetric
> for dma_unmap() / sync_for_cpu() - is too abstract and that hurts
> performance. In particular, "setup i/o" and "sync caches" is different
> activity with conflicting performance properties: for better
> performance, one wants to setup i/o for larger blocks, but sync caches
> for smaller blocks. Probably separation of these activities into
> different calls is the way for better performance.
>
>
>>> But what about doing something safer, e.g. adding a bit of tracking and
>>> only sync_for_device() what was previously sync_for_cpu()ed? Will you
>>> accept that?
>>
>> The problem is that as we move things over for XDP we will be looking
>> at having the CPU potentially write to any spot in the region that was
>> mapped as we could append headers to the front or pad data onto the
>> end of the frame. It is probably safest for us to invalidate the
>> entire region just to make sure we don't have a collision with
>> something that is writing to the page.
>
> Can't comment without knowning particular access patterns that XDP will
> cause. Still rule is simple - "invalidate more" does hurt performance,
> thus need to invalidate minimal required area. To avoid this
> invalidation thing hurting performance on x86 that does not need
> invalidation at all, good idea is to use some compile-time magic - just
> to compile out unneeded things completely.
On x86 we don't perform any invalidation. We just have the function
pointers usually referencing functions that don't do anything unless
we can't handle the DMA address for some reason as we usually default
to swiotlb setup on x86_64.
> Still, XDP is future question, currently igb does not use it. Why not
> improve sync_for_cpu() / sync_for_device() pairing in the current code?
> I can propare such a patch. If XDP will make it irrelevant in future,
> perhaps it could be just undone (and if this will cause performance
> degradation, then it will be something to work on)
So the plan is to update the code path in the near future. I'm
working on rewriting the DMA APIs for the drivers now, will update the
page count handling for the pages, and in the near future we should be
able to support XDP and the use of build_skb. Once we get to that
point there should be a modest bump in performance since we can drop
the memcpy for headers.
>> So for example in the near future I am planning to expand out the
>> DMA_ATTR_SKIP_CPU_SYNC DMA attribute beyond just the ARM architecture
>> to see if I can expand it for use with SWIOTLB. If we can use this on
>> unmap we might be able to solve some of the existing problems that
>> required us to make the page read-only since we could unmap the page
>> without invalidating any existing writes on the page.
>
> Ack. Actually it is the same decoupling between "setup i/o" and "sync
> caches" I've mentioned above.
Right. That is kind of what I was thinking as well. We need to be
able to create and destroy DMA mappable pages without corrupting
things once we hand the pages up the stack. If we can get away from
having to worry about unmap invalidating any writes from the CPU then
we can get away with using things like build_skb which should give us
a pretty good increase in throughput.
- Alex
^ permalink raw reply
* [PATCH] net: Require exact match for TCP socket lookups if dif is l3mdev
From: David Ahern @ 2016-10-13 20:14 UTC (permalink / raw)
To: netdev; +Cc: David Ahern
Currently, socket lookups for l3mdev (vrf) use cases can match a socket
that is bound to a port but not a device (ie., a global socket). If the
sysctl tcp_l3mdev_accept is not set this leads to ack packets going out
based on the main table even though the packet came in from an L3 domain.
The end result is that the connection does not establish creating
confusion for users since the service is running and a socket shows in
ss output. Fix by requiring an exact dif to sk_bound_dev_if match if the
dif is an l3mdev device and the tcp_l3mdev_accept is not set.
Fixes: 193125dbd8eb ("net: Introduce VRF device driver")
Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
---
include/net/inet_sock.h | 10 ++++++++++
net/ipv4/inet_hashtables.c | 7 ++++---
net/ipv6/inet6_hashtables.c | 7 ++++---
3 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index 236a81034fef..abb4e720a769 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -132,6 +132,16 @@ static inline int inet_request_bound_dev_if(const struct sock *sk,
return sk->sk_bound_dev_if;
}
+static inline bool inet_exact_dif_match(struct net *net, int dif)
+{
+#ifdef CONFIG_NET_L3_MASTER_DEV
+ if (netif_index_is_l3_master(net, dif) &&
+ !net->ipv4.sysctl_tcp_l3mdev_accept)
+ return true;
+#endif
+ return false;
+}
+
struct inet_cork {
unsigned int flags;
__be32 addr;
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 77c20a489218..08b3598c43f9 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -172,7 +172,7 @@ EXPORT_SYMBOL_GPL(__inet_inherit_port);
static inline int compute_score(struct sock *sk, struct net *net,
const unsigned short hnum, const __be32 daddr,
- const int dif)
+ const int dif, bool exact_dif)
{
int score = -1;
struct inet_sock *inet = inet_sk(sk);
@@ -186,7 +186,7 @@ static inline int compute_score(struct sock *sk, struct net *net,
return -1;
score += 4;
}
- if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if || exact_dif) {
if (sk->sk_bound_dev_if != dif)
return -1;
score += 4;
@@ -215,11 +215,12 @@ struct sock *__inet_lookup_listener(struct net *net,
unsigned int hash = inet_lhashfn(net, hnum);
struct inet_listen_hashbucket *ilb = &hashinfo->listening_hash[hash];
int score, hiscore = 0, matches = 0, reuseport = 0;
+ bool exact_dif = inet_exact_dif_match(net, dif);
struct sock *sk, *result = NULL;
u32 phash = 0;
sk_for_each_rcu(sk, &ilb->head) {
- score = compute_score(sk, net, hnum, daddr, dif);
+ score = compute_score(sk, net, hnum, daddr, dif, exact_dif);
if (score > hiscore) {
reuseport = sk->sk_reuseport;
if (reuseport) {
diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c
index 00cf28ad4565..f85924883056 100644
--- a/net/ipv6/inet6_hashtables.c
+++ b/net/ipv6/inet6_hashtables.c
@@ -96,7 +96,7 @@ EXPORT_SYMBOL(__inet6_lookup_established);
static inline int compute_score(struct sock *sk, struct net *net,
const unsigned short hnum,
const struct in6_addr *daddr,
- const int dif)
+ const int dif, bool exact_dif)
{
int score = -1;
@@ -109,7 +109,7 @@ static inline int compute_score(struct sock *sk, struct net *net,
return -1;
score++;
}
- if (sk->sk_bound_dev_if) {
+ if (sk->sk_bound_dev_if || exact_dif) {
if (sk->sk_bound_dev_if != dif)
return -1;
score++;
@@ -131,11 +131,12 @@ struct sock *inet6_lookup_listener(struct net *net,
unsigned int hash = inet_lhashfn(net, hnum);
struct inet_listen_hashbucket *ilb = &hashinfo->listening_hash[hash];
int score, hiscore = 0, matches = 0, reuseport = 0;
+ bool exact_dif = inet_exact_dif_match(net, dif);
struct sock *sk, *result = NULL;
u32 phash = 0;
sk_for_each(sk, &ilb->head) {
- score = compute_score(sk, net, hnum, daddr, dif);
+ score = compute_score(sk, net, hnum, daddr, dif, exact_dif);
if (score > hiscore) {
reuseport = sk->sk_reuseport;
if (reuseport) {
--
2.1.4
^ permalink raw reply related
* [PATCH net] net/mlx4_en: fixup xdp tx irq to match rx
From: Brenden Blanco @ 2016-10-13 20:13 UTC (permalink / raw)
To: davem, netdev
Cc: Brenden Blanco, Alexei Starovoitov, Tariq Toukan, Or Gerlitz,
Saeed Mahameed, Jesper Dangaard Brouer
In cases where the number of tx rings is not a multiple of the number of
rx rings, the tx completion event will be handled on a different core
from the transmit and population of the ring. Races on the ring will
lead to a double-free of the page, and possibly other corruption.
The rings are initialized by default with a valid multiple of rings,
based on the number of cpus, therefore an invalid configuration requires
ethtool to change the ring layout. For instance 'ethtool -L eth0 rx 9 tx
8' will cause packets received on rx0, and XDP_TX'd to tx48, to be
completed on cpu3 (48 % 9 == 3).
Resolve this discrepancy by shifting the irq for the xdp tx queues to
start again from 0, modulo rx_ring_num.
Fixes: 9ecc2d86171a ("net/mlx4_en: add xdp forwarding and data write support")
Reported-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Brenden Blanco <bblanco@plumgrid.com>
---
drivers/net/ethernet/mellanox/mlx4/en_cq.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_cq.c b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
index 132cea6..e3be7e4 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_cq.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_cq.c
@@ -127,7 +127,15 @@ int mlx4_en_activate_cq(struct mlx4_en_priv *priv, struct mlx4_en_cq *cq,
/* For TX we use the same irq per
ring we assigned for the RX */
struct mlx4_en_cq *rx_cq;
-
+ int xdp_index;
+
+ /* The xdp tx irq must align with the rx ring that forwards to
+ * it, so reindex these from 0. This should only happen when
+ * tx_ring_num is not a multiple of rx_ring_num.
+ */
+ xdp_index = (priv->xdp_ring_num - priv->tx_ring_num) + cq_idx;
+ if (xdp_index >= 0)
+ cq_idx = xdp_index;
cq_idx = cq_idx % priv->rx_ring_num;
rx_cq = priv->rx_cq[cq_idx];
cq->vector = rx_cq->vector;
--
2.9.3
^ permalink raw reply related
* Re: [PATCH net-next 0/5] udp: Flow dissection for tunnels
From: Tom Herbert @ 2016-10-13 19:29 UTC (permalink / raw)
To: David Miller; +Cc: Linux Kernel Network Developers, Kernel Team
In-Reply-To: <20161013.151700.1071876162706356206.davem@davemloft.net>
On Thu, Oct 13, 2016 at 12:17 PM, David Miller <davem@davemloft.net> wrote:
> From: Tom Herbert <tom@herbertland.com>
> Date: Wed, 12 Oct 2016 16:25:42 -0700
>
>> Since performing a UDP lookup on every packet might be expensive I
>> added a static key check to bypass the lookup if there are no
>> sockets with flow_dissect set. I should mention that doing the
>> lookup wasn't particularly a big hit anyway.
>
> I think this new static key is unnecessary, as it is equivalent
> to: (udp_encap_needed + udpv6_encap_needed)
>
Good point.
> This socket lookup is very heavy handed, and I realize that you
> need this because we no longer store the encapsulation socket in
> skb->sk these days.
>
I don't quite understand your point about the encapsulation socket.
The reason we do the socket lookup is the same as why we do it this
way in GRO already, identifying UDP encapsulation by just port is
insufficient and may lead to incorrect results. Once we removed the
atomic operation from the UDP socket lookup (thanks Eric!) it becomes
feasible to do the lookup in critical paths and so the special per
port offloads lookup was removed. In my testing I didn't see a
noticeable hit in performing the lookup, but it still seems prudent to
use the static key since UDP tunnels are a relatively narrow use case.
> Can you talk about the various code paths that lead into the
> flow dissector and why the UDP socket lookup is needed? Maybe
> we can propagate it another way, at least on TX.
>
The code path I'm interested in simple RX, particularly in the case
where UDP is being used as static tunnel for many flows in IPv4
(assuming in IPv6 we have flow label to provide entropy). For getting
the flow hash on TX we are already using sk->tx_hash if it's
available.
Tom
> Thanks.
^ permalink raw reply
* [PATCH net 3/3] qed: Additional work toward cleaning C=1
From: Yuval Mintz @ 2016-10-13 19:57 UTC (permalink / raw)
To: davem, netdev; +Cc: arnd, dan.carpenter, Yuval Mintz
In-Reply-To: <1476388623-31032-1-git-send-email-Yuval.Mintz@caviumnetworks.com>
This cleans many of the warnings that would arise in qed as a
result of compilations with C=1; Most of those are the addition
of missing 'static' to functions, although there are several other
fixes as well.
Signed-off-by: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
---
drivers/net/ethernet/qlogic/qed/qed_cxt.c | 8 +-
drivers/net/ethernet/qlogic/qed/qed_debug.c | 53 +++++++------
drivers/net/ethernet/qlogic/qed/qed_dev.c | 5 +-
drivers/net/ethernet/qlogic/qed/qed_ll2.c | 15 ++--
drivers/net/ethernet/qlogic/qed/qed_main.c | 2 +-
drivers/net/ethernet/qlogic/qed/qed_roce.c | 118 ++++------------------------
drivers/net/ethernet/qlogic/qed/qed_roce.h | 20 -----
drivers/net/ethernet/qlogic/qed/qed_sp.h | 1 -
8 files changed, 58 insertions(+), 164 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_cxt.c b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
index 277db78..0c42c24 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_cxt.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
@@ -344,14 +344,14 @@ static struct qed_tid_seg *qed_cxt_tid_seg_info(struct qed_hwfn *p_hwfn,
return NULL;
}
-void qed_cxt_set_srq_count(struct qed_hwfn *p_hwfn, u32 num_srqs)
+static void qed_cxt_set_srq_count(struct qed_hwfn *p_hwfn, u32 num_srqs)
{
struct qed_cxt_mngr *p_mgr = p_hwfn->p_cxt_mngr;
p_mgr->srq_count = num_srqs;
}
-u32 qed_cxt_get_srq_count(struct qed_hwfn *p_hwfn)
+static u32 qed_cxt_get_srq_count(struct qed_hwfn *p_hwfn)
{
struct qed_cxt_mngr *p_mgr = p_hwfn->p_cxt_mngr;
@@ -1799,8 +1799,8 @@ int qed_cxt_get_cid_info(struct qed_hwfn *p_hwfn, struct qed_cxt_info *p_info)
return 0;
}
-void qed_rdma_set_pf_params(struct qed_hwfn *p_hwfn,
- struct qed_rdma_pf_params *p_params)
+static void qed_rdma_set_pf_params(struct qed_hwfn *p_hwfn,
+ struct qed_rdma_pf_params *p_params)
{
u32 num_cons, num_tasks, num_qps, num_mrs, num_srqs;
enum protocol_type proto;
diff --git a/drivers/net/ethernet/qlogic/qed/qed_debug.c b/drivers/net/ethernet/qlogic/qed/qed_debug.c
index 88e7d5b..68f19ca 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_debug.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_debug.c
@@ -405,7 +405,7 @@ struct phy_defs {
/***************************** Constant Arrays *******************************/
/* Debug arrays */
-static struct dbg_array s_dbg_arrays[MAX_BIN_DBG_BUFFER_TYPE] = { {0} };
+static struct dbg_array s_dbg_arrays[MAX_BIN_DBG_BUFFER_TYPE] = { {NULL} };
/* Chip constant definitions array */
static struct chip_defs s_chip_defs[MAX_CHIP_IDS] = {
@@ -4028,10 +4028,10 @@ static enum dbg_status qed_mcp_trace_read_meta(struct qed_hwfn *p_hwfn,
}
/* Dump MCP Trace */
-enum dbg_status qed_mcp_trace_dump(struct qed_hwfn *p_hwfn,
- struct qed_ptt *p_ptt,
- u32 *dump_buf,
- bool dump, u32 *num_dumped_dwords)
+static enum dbg_status qed_mcp_trace_dump(struct qed_hwfn *p_hwfn,
+ struct qed_ptt *p_ptt,
+ u32 *dump_buf,
+ bool dump, u32 *num_dumped_dwords)
{
u32 trace_data_grc_addr, trace_data_size_bytes, trace_data_size_dwords;
u32 trace_meta_size_dwords, running_bundle_id, offset = 0;
@@ -4130,10 +4130,10 @@ enum dbg_status qed_mcp_trace_dump(struct qed_hwfn *p_hwfn,
}
/* Dump GRC FIFO */
-enum dbg_status qed_reg_fifo_dump(struct qed_hwfn *p_hwfn,
- struct qed_ptt *p_ptt,
- u32 *dump_buf,
- bool dump, u32 *num_dumped_dwords)
+static enum dbg_status qed_reg_fifo_dump(struct qed_hwfn *p_hwfn,
+ struct qed_ptt *p_ptt,
+ u32 *dump_buf,
+ bool dump, u32 *num_dumped_dwords)
{
u32 offset = 0, dwords_read, size_param_offset;
bool fifo_has_data;
@@ -4192,10 +4192,10 @@ enum dbg_status qed_reg_fifo_dump(struct qed_hwfn *p_hwfn,
}
/* Dump IGU FIFO */
-enum dbg_status qed_igu_fifo_dump(struct qed_hwfn *p_hwfn,
- struct qed_ptt *p_ptt,
- u32 *dump_buf,
- bool dump, u32 *num_dumped_dwords)
+static enum dbg_status qed_igu_fifo_dump(struct qed_hwfn *p_hwfn,
+ struct qed_ptt *p_ptt,
+ u32 *dump_buf,
+ bool dump, u32 *num_dumped_dwords)
{
u32 offset = 0, dwords_read, size_param_offset;
bool fifo_has_data;
@@ -4255,10 +4255,11 @@ enum dbg_status qed_igu_fifo_dump(struct qed_hwfn *p_hwfn,
}
/* Protection Override dump */
-enum dbg_status qed_protection_override_dump(struct qed_hwfn *p_hwfn,
- struct qed_ptt *p_ptt,
- u32 *dump_buf,
- bool dump, u32 *num_dumped_dwords)
+static enum dbg_status qed_protection_override_dump(struct qed_hwfn *p_hwfn,
+ struct qed_ptt *p_ptt,
+ u32 *dump_buf,
+ bool dump,
+ u32 *num_dumped_dwords)
{
u32 offset = 0, size_param_offset, override_window_dwords;
@@ -6339,10 +6340,11 @@ enum dbg_status qed_print_fw_asserts_results(struct qed_hwfn *p_hwfn,
}
/* Wrapper for unifying the idle_chk and mcp_trace api */
-enum dbg_status qed_print_idle_chk_results_wrapper(struct qed_hwfn *p_hwfn,
- u32 *dump_buf,
- u32 num_dumped_dwords,
- char *results_buf)
+static enum dbg_status
+qed_print_idle_chk_results_wrapper(struct qed_hwfn *p_hwfn,
+ u32 *dump_buf,
+ u32 num_dumped_dwords,
+ char *results_buf)
{
u32 num_errors, num_warnnings;
@@ -6413,8 +6415,8 @@ static void qed_dbg_print_feature(u8 *p_text_buf, u32 text_size)
#define QED_RESULTS_BUF_MIN_SIZE 16
/* Generic function for decoding debug feature info */
-enum dbg_status format_feature(struct qed_hwfn *p_hwfn,
- enum qed_dbg_features feature_idx)
+static enum dbg_status format_feature(struct qed_hwfn *p_hwfn,
+ enum qed_dbg_features feature_idx)
{
struct qed_dbg_feature *feature =
&p_hwfn->cdev->dbg_params.features[feature_idx];
@@ -6480,8 +6482,9 @@ enum dbg_status format_feature(struct qed_hwfn *p_hwfn,
}
/* Generic function for performing the dump of a debug feature. */
-enum dbg_status qed_dbg_dump(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
- enum qed_dbg_features feature_idx)
+static enum dbg_status qed_dbg_dump(struct qed_hwfn *p_hwfn,
+ struct qed_ptt *p_ptt,
+ enum qed_dbg_features feature_idx)
{
struct qed_dbg_feature *feature =
&p_hwfn->cdev->dbg_params.features[feature_idx];
diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c
index 21adf52..edae5fc 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_dev.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c
@@ -497,12 +497,13 @@ int qed_resc_alloc(struct qed_dev *cdev)
if (p_hwfn->hw_info.personality == QED_PCI_ETH_ROCE) {
num_cons = qed_cxt_get_proto_cid_count(p_hwfn,
PROTOCOLID_ROCE,
- 0) * 2;
+ NULL) * 2;
n_eqes += num_cons + 2 * MAX_NUM_VFS_BB;
} else if (p_hwfn->hw_info.personality == QED_PCI_ISCSI) {
num_cons =
qed_cxt_get_proto_cid_count(p_hwfn,
- PROTOCOLID_ISCSI, 0);
+ PROTOCOLID_ISCSI,
+ NULL);
n_eqes += 2 * num_cons;
}
diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c b/drivers/net/ethernet/qlogic/qed/qed_ll2.c
index 7856e52..ce171a8 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c
@@ -141,11 +141,11 @@ static void qed_ll2_kill_buffers(struct qed_dev *cdev)
qed_ll2_dealloc_buffer(cdev, buffer);
}
-void qed_ll2b_complete_rx_packet(struct qed_hwfn *p_hwfn,
- u8 connection_handle,
- struct qed_ll2_rx_packet *p_pkt,
- struct core_rx_fast_path_cqe *p_cqe,
- bool b_last_packet)
+static void qed_ll2b_complete_rx_packet(struct qed_hwfn *p_hwfn,
+ u8 connection_handle,
+ struct qed_ll2_rx_packet *p_pkt,
+ struct core_rx_fast_path_cqe *p_cqe,
+ bool b_last_packet)
{
u16 packet_length = le16_to_cpu(p_cqe->packet_length);
struct qed_ll2_buffer *buffer = p_pkt->cookie;
@@ -516,7 +516,7 @@ static int qed_ll2_rxq_completion(struct qed_hwfn *p_hwfn, void *cookie)
return rc;
}
-void qed_ll2_rxq_flush(struct qed_hwfn *p_hwfn, u8 connection_handle)
+static void qed_ll2_rxq_flush(struct qed_hwfn *p_hwfn, u8 connection_handle)
{
struct qed_ll2_info *p_ll2_conn = NULL;
struct qed_ll2_rx_packet *p_pkt = NULL;
@@ -1124,9 +1124,6 @@ static void qed_ll2_prepare_tx_packet_set_bd(struct qed_hwfn *p_hwfn,
DMA_REGPAIR_LE(start_bd->addr, first_frag);
start_bd->nbytes = cpu_to_le16(first_frag_len);
- SET_FIELD(start_bd->bd_flags.as_bitfield, CORE_TX_BD_FLAGS_ROCE_FLAV,
- type);
-
DP_VERBOSE(p_hwfn,
(NETIF_MSG_TX_QUEUED | QED_MSG_LL2),
"LL2 [q 0x%02x cid 0x%08x type 0x%08x] Tx Producer at [0x%04x] - set with a %04x bytes %02x BDs buffer at %08x:%08x\n",
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index 6eb2401..8dc3f46 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -1430,7 +1430,7 @@ static int qed_set_led(struct qed_dev *cdev, enum qed_led_mode mode)
return status;
}
-struct qed_selftest_ops qed_selftest_ops_pass = {
+static struct qed_selftest_ops qed_selftest_ops_pass = {
.selftest_memory = &qed_selftest_memory,
.selftest_interrupt = &qed_selftest_interrupt,
.selftest_register = &qed_selftest_register,
diff --git a/drivers/net/ethernet/qlogic/qed/qed_roce.c b/drivers/net/ethernet/qlogic/qed/qed_roce.c
index 187df38..f3a825a 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_roce.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_roce.c
@@ -157,7 +157,8 @@ static int qed_rdma_alloc(struct qed_hwfn *p_hwfn,
p_hwfn->p_rdma_info = p_rdma_info;
p_rdma_info->proto = PROTOCOLID_ROCE;
- num_cons = qed_cxt_get_proto_cid_count(p_hwfn, p_rdma_info->proto, 0);
+ num_cons = qed_cxt_get_proto_cid_count(p_hwfn, p_rdma_info->proto,
+ NULL);
p_rdma_info->num_qps = num_cons / 2;
@@ -831,7 +832,7 @@ static int qed_rdma_alloc_pd(void *rdma_cxt, u16 *pd)
return rc;
}
-void qed_rdma_free_pd(void *rdma_cxt, u16 pd)
+static void qed_rdma_free_pd(void *rdma_cxt, u16 pd)
{
struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
@@ -868,8 +869,9 @@ void qed_rdma_free_pd(void *rdma_cxt, u16 pd)
return toggle_bit;
}
-int qed_rdma_create_cq(void *rdma_cxt,
- struct qed_rdma_create_cq_in_params *params, u16 *icid)
+static int qed_rdma_create_cq(void *rdma_cxt,
+ struct qed_rdma_create_cq_in_params *params,
+ u16 *icid)
{
struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
struct qed_rdma_info *p_info = p_hwfn->p_rdma_info;
@@ -952,98 +954,10 @@ int qed_rdma_create_cq(void *rdma_cxt,
return rc;
}
-int qed_rdma_resize_cq(void *rdma_cxt,
- struct qed_rdma_resize_cq_in_params *in_params,
- struct qed_rdma_resize_cq_out_params *out_params)
-{
- struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
- struct rdma_resize_cq_output_params *p_ramrod_res;
- struct rdma_resize_cq_ramrod_data *p_ramrod;
- enum qed_rdma_toggle_bit toggle_bit;
- struct qed_sp_init_data init_data;
- struct qed_spq_entry *p_ent;
- dma_addr_t ramrod_res_phys;
- u8 fw_return_code;
- int rc = -ENOMEM;
-
- DP_VERBOSE(p_hwfn, QED_MSG_RDMA, "icid = %08x\n", in_params->icid);
-
- p_ramrod_res =
- (struct rdma_resize_cq_output_params *)
- dma_alloc_coherent(&p_hwfn->cdev->pdev->dev,
- sizeof(struct rdma_resize_cq_output_params),
- &ramrod_res_phys, GFP_KERNEL);
- if (!p_ramrod_res) {
- DP_NOTICE(p_hwfn,
- "qed resize cq failed: cannot allocate memory (ramrod)\n");
- return rc;
- }
-
- /* Get SPQ entry */
- memset(&init_data, 0, sizeof(init_data));
- init_data.cid = in_params->icid;
- init_data.opaque_fid = p_hwfn->hw_info.opaque_fid;
- init_data.comp_mode = QED_SPQ_MODE_EBLOCK;
-
- rc = qed_sp_init_request(p_hwfn, &p_ent,
- RDMA_RAMROD_RESIZE_CQ,
- p_hwfn->p_rdma_info->proto, &init_data);
- if (rc)
- goto err;
-
- p_ramrod = &p_ent->ramrod.rdma_resize_cq;
-
- p_ramrod->flags = 0;
-
- /* toggle the bit for every resize or create cq for a given icid */
- toggle_bit = qed_rdma_toggle_bit_create_resize_cq(p_hwfn,
- in_params->icid);
-
- SET_FIELD(p_ramrod->flags,
- RDMA_RESIZE_CQ_RAMROD_DATA_TOGGLE_BIT, toggle_bit);
-
- SET_FIELD(p_ramrod->flags,
- RDMA_RESIZE_CQ_RAMROD_DATA_IS_TWO_LEVEL_PBL,
- in_params->pbl_two_level);
-
- p_ramrod->pbl_log_page_size = in_params->pbl_page_size_log - 12;
- p_ramrod->pbl_num_pages = cpu_to_le16(in_params->pbl_num_pages);
- p_ramrod->max_cqes = cpu_to_le32(in_params->cq_size);
- DMA_REGPAIR_LE(p_ramrod->pbl_addr, in_params->pbl_ptr);
- DMA_REGPAIR_LE(p_ramrod->output_params_addr, ramrod_res_phys);
-
- rc = qed_spq_post(p_hwfn, p_ent, &fw_return_code);
- if (rc)
- goto err;
-
- if (fw_return_code != RDMA_RETURN_OK) {
- DP_NOTICE(p_hwfn, "fw_return_code = %d\n", fw_return_code);
- rc = -EINVAL;
- goto err;
- }
-
- out_params->prod = le32_to_cpu(p_ramrod_res->old_cq_prod);
- out_params->cons = le32_to_cpu(p_ramrod_res->old_cq_cons);
-
- dma_free_coherent(&p_hwfn->cdev->pdev->dev,
- sizeof(struct rdma_resize_cq_output_params),
- p_ramrod_res, ramrod_res_phys);
-
- DP_VERBOSE(p_hwfn, QED_MSG_RDMA, "Resized CQ, rc = %d\n", rc);
-
- return rc;
-
-err: dma_free_coherent(&p_hwfn->cdev->pdev->dev,
- sizeof(struct rdma_resize_cq_output_params),
- p_ramrod_res, ramrod_res_phys);
- DP_NOTICE(p_hwfn, "Resized CQ, Failed - rc = %d\n", rc);
-
- return rc;
-}
-
-int qed_rdma_destroy_cq(void *rdma_cxt,
- struct qed_rdma_destroy_cq_in_params *in_params,
- struct qed_rdma_destroy_cq_out_params *out_params)
+static int
+qed_rdma_destroy_cq(void *rdma_cxt,
+ struct qed_rdma_destroy_cq_in_params *in_params,
+ struct qed_rdma_destroy_cq_out_params *out_params)
{
struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
struct rdma_destroy_cq_output_params *p_ramrod_res;
@@ -1164,7 +1078,7 @@ static enum roce_flavor qed_roce_mode_to_flavor(enum roce_mode roce_mode)
return flavor;
}
-int qed_roce_alloc_cid(struct qed_hwfn *p_hwfn, u16 *cid)
+static int qed_roce_alloc_cid(struct qed_hwfn *p_hwfn, u16 *cid)
{
struct qed_rdma_info *p_rdma_info = p_hwfn->p_rdma_info;
u32 responder_icid;
@@ -1788,9 +1702,9 @@ static int qed_roce_sp_destroy_qp_requester(struct qed_hwfn *p_hwfn,
return rc;
}
-int qed_roce_query_qp(struct qed_hwfn *p_hwfn,
- struct qed_rdma_qp *qp,
- struct qed_rdma_query_qp_out_params *out_params)
+static int qed_roce_query_qp(struct qed_hwfn *p_hwfn,
+ struct qed_rdma_qp *qp,
+ struct qed_rdma_query_qp_out_params *out_params)
{
struct roce_query_qp_resp_output_params *p_resp_ramrod_res;
struct roce_query_qp_req_output_params *p_req_ramrod_res;
@@ -1931,7 +1845,7 @@ int qed_roce_query_qp(struct qed_hwfn *p_hwfn,
return rc;
}
-int qed_roce_destroy_qp(struct qed_hwfn *p_hwfn, struct qed_rdma_qp *qp)
+static int qed_roce_destroy_qp(struct qed_hwfn *p_hwfn, struct qed_rdma_qp *qp)
{
u32 num_invalidated_mw = 0;
u32 num_bound_mw = 0;
@@ -2033,7 +1947,7 @@ static int qed_rdma_destroy_qp(void *rdma_cxt, struct qed_rdma_qp *qp)
return rc;
}
-struct qed_rdma_qp *
+static struct qed_rdma_qp *
qed_rdma_create_qp(void *rdma_cxt,
struct qed_rdma_create_qp_in_params *in_params,
struct qed_rdma_create_qp_out_params *out_params)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_roce.h b/drivers/net/ethernet/qlogic/qed/qed_roce.h
index 6914131..279f342 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_roce.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_roce.h
@@ -95,26 +95,6 @@ struct qed_rdma_info {
enum protocol_type proto;
};
-struct qed_rdma_resize_cq_in_params {
- u16 icid;
- u32 cq_size;
- bool pbl_two_level;
- u64 pbl_ptr;
- u16 pbl_num_pages;
- u8 pbl_page_size_log;
-};
-
-struct qed_rdma_resize_cq_out_params {
- u32 prod;
- u32 cons;
-};
-
-struct qed_rdma_resize_cnq_in_params {
- u32 cnq_id;
- u32 pbl_page_size_log;
- u64 pbl_ptr;
-};
-
struct qed_rdma_qp {
struct regpair qp_handle;
struct regpair qp_handle_async;
diff --git a/drivers/net/ethernet/qlogic/qed/qed_sp.h b/drivers/net/ethernet/qlogic/qed/qed_sp.h
index 652c908..b2c08e4 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_sp.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_sp.h
@@ -80,7 +80,6 @@ int qed_eth_cqe_completion(struct qed_hwfn *p_hwfn,
struct roce_destroy_qp_resp_ramrod_data roce_destroy_qp_resp;
struct roce_destroy_qp_req_ramrod_data roce_destroy_qp_req;
struct rdma_create_cq_ramrod_data rdma_create_cq;
- struct rdma_resize_cq_ramrod_data rdma_resize_cq;
struct rdma_destroy_cq_ramrod_data rdma_destroy_cq;
struct rdma_srq_create_ramrod_data rdma_create_srq;
struct rdma_srq_destroy_ramrod_data rdma_destroy_srq;
--
1.9.3
^ permalink raw reply related
* [PATCH net 2/3] qed*: Fix Kconfig dependencies with INFINIBAND_QEDR
From: Yuval Mintz @ 2016-10-13 19:57 UTC (permalink / raw)
To: davem, netdev; +Cc: arnd, dan.carpenter, Yuval Mintz
In-Reply-To: <1476388623-31032-1-git-send-email-Yuval.Mintz@caviumnetworks.com>
The qedr driver would require a tristate Kconfig option [to allow
it to compile as a module], and toward that end we've added the
INFINIBAND_QEDR option. But as we've made the compilation of the
qed/qede infrastructure required for RoCE dependent on the option
we'd be facing linking difficulties in case that QED=y or QEDE=y,
and INFINIBAND_QEDR=m.
To resolve this, we seperate between the INFINIBAND_QEDR option
and the infrastructure support in qed/qede by introducing a new
QED_RDMA option which would be selected by INFINIBAND_QEDR but would
be a boolean instead of a tristate; Following that, the qed/qede is
fixed based on this new option so that all config combinations would
be supported.
Fixes: cee9fbd8e2e9 ("qede: add qedr framework")
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
---
drivers/net/ethernet/qlogic/Kconfig | 4 ++
drivers/net/ethernet/qlogic/qed/Makefile | 2 +-
drivers/net/ethernet/qlogic/qed/qed_cxt.c | 7 +--
drivers/net/ethernet/qlogic/qed/qed_dev.c | 14 ++---
drivers/net/ethernet/qlogic/qed/qed_ll2.c | 1 +
drivers/net/ethernet/qlogic/qed/qed_ll2.h | 20 -------
drivers/net/ethernet/qlogic/qed/qed_main.c | 28 +++++----
drivers/net/ethernet/qlogic/qed/qed_roce.c | 91 +++++++++++++++---------------
drivers/net/ethernet/qlogic/qed/qed_roce.h | 75 +++++++++++++++---------
drivers/net/ethernet/qlogic/qed/qed_spq.c | 4 --
drivers/net/ethernet/qlogic/qede/Makefile | 2 +-
include/linux/qed/qede_roce.h | 2 +-
12 files changed, 120 insertions(+), 130 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/Kconfig b/drivers/net/ethernet/qlogic/Kconfig
index 0df1391f9..7756772 100644
--- a/drivers/net/ethernet/qlogic/Kconfig
+++ b/drivers/net/ethernet/qlogic/Kconfig
@@ -107,10 +107,14 @@ config QEDE
---help---
This enables the support for ...
+config QED_RDMA
+ bool
+
config INFINIBAND_QEDR
tristate "QLogic qede RoCE sources [debug]"
depends on QEDE && 64BIT
select QED_LL2
+ select QED_RDMA
default n
---help---
This provides a temporary node that allows the compilation
diff --git a/drivers/net/ethernet/qlogic/qed/Makefile b/drivers/net/ethernet/qlogic/qed/Makefile
index cda0af7..967acf3 100644
--- a/drivers/net/ethernet/qlogic/qed/Makefile
+++ b/drivers/net/ethernet/qlogic/qed/Makefile
@@ -5,4 +5,4 @@ qed-y := qed_cxt.o qed_dev.o qed_hw.o qed_init_fw_funcs.o qed_init_ops.o \
qed_selftest.o qed_dcbx.o qed_debug.o
qed-$(CONFIG_QED_SRIOV) += qed_sriov.o qed_vf.o
qed-$(CONFIG_QED_LL2) += qed_ll2.o
-qed-$(CONFIG_INFINIBAND_QEDR) += qed_roce.o
+qed-$(CONFIG_QED_RDMA) += qed_roce.o
diff --git a/drivers/net/ethernet/qlogic/qed/qed_cxt.c b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
index 82370a1..277db78 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_cxt.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
@@ -47,13 +47,8 @@
#define TM_ALIGN BIT(TM_SHIFT)
#define TM_ELEM_SIZE 4
-/* ILT constants */
-#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
/* For RoCE we configure to 64K to cover for RoCE max tasks 256K purpose. */
-#define ILT_DEFAULT_HW_P_SIZE 4
-#else
-#define ILT_DEFAULT_HW_P_SIZE 3
-#endif
+#define ILT_DEFAULT_HW_P_SIZE (IS_ENABLED(CONFIG_QED_RDMA) ? 4 : 3)
#define ILT_PAGE_IN_BYTES(hw_p_size) (1U << ((hw_p_size) + 12))
#define ILT_CFG_REG(cli, reg) PSWRQ2_REG_ ## cli ## _ ## reg ## _RT_OFFSET
diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c
index 754f6a9..21adf52 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_dev.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c
@@ -1422,19 +1422,19 @@ static void qed_hw_set_feat(struct qed_hwfn *p_hwfn)
u32 *feat_num = p_hwfn->hw_info.feat_num;
int num_features = 1;
-#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
- /* Roce CNQ each requires: 1 status block + 1 CNQ. We divide the
- * status blocks equally between L2 / RoCE but with consideration as
- * to how many l2 queues / cnqs we have
- */
- if (p_hwfn->hw_info.personality == QED_PCI_ETH_ROCE) {
+ if (IS_ENABLED(CONFIG_QED_RDMA) &&
+ p_hwfn->hw_info.personality == QED_PCI_ETH_ROCE) {
+ /* Roce CNQ each requires: 1 status block + 1 CNQ. We divide
+ * the status blocks equally between L2 / RoCE but with
+ * consideration as to how many l2 queues / cnqs we have.
+ */
num_features++;
feat_num[QED_RDMA_CNQ] =
min_t(u32, RESC_NUM(p_hwfn, QED_SB) / num_features,
RESC_NUM(p_hwfn, QED_RDMA_CNQ_RAM));
}
-#endif
+
feat_num[QED_PF_L2_QUE] = min_t(u32, RESC_NUM(p_hwfn, QED_SB) /
num_features,
RESC_NUM(p_hwfn, QED_L2_QUEUE));
diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c b/drivers/net/ethernet/qlogic/qed/qed_ll2.c
index 02a8be2..7856e52 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c
@@ -38,6 +38,7 @@
#include "qed_mcp.h"
#include "qed_reg_addr.h"
#include "qed_sp.h"
+#include "qed_roce.h"
#define QED_LL2_RX_REGISTERED(ll2) ((ll2)->rx_queue.b_cb_registred)
#define QED_LL2_TX_REGISTERED(ll2) ((ll2)->tx_queue.b_cb_registred)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.h b/drivers/net/ethernet/qlogic/qed/qed_ll2.h
index 80a5dc2..4e3d62a 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_ll2.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.h
@@ -293,24 +293,4 @@ void qed_ll2_setup(struct qed_hwfn *p_hwfn,
*/
void qed_ll2_free(struct qed_hwfn *p_hwfn,
struct qed_ll2_info *p_ll2_connections);
-void qed_ll2b_complete_rx_gsi_packet(struct qed_hwfn *p_hwfn,
- u8 connection_handle,
- void *cookie,
- dma_addr_t rx_buf_addr,
- u16 data_length,
- u8 data_length_error,
- u16 parse_flags,
- u16 vlan,
- u32 src_mac_addr_hi,
- u16 src_mac_addr_lo, bool b_last_packet);
-void qed_ll2b_complete_tx_gsi_packet(struct qed_hwfn *p_hwfn,
- u8 connection_handle,
- void *cookie,
- dma_addr_t first_frag_addr,
- bool b_last_fragment, bool b_last_packet);
-void qed_ll2b_release_tx_gsi_packet(struct qed_hwfn *p_hwfn,
- u8 connection_handle,
- void *cookie,
- dma_addr_t first_frag_addr,
- bool b_last_fragment, bool b_last_packet);
#endif
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index 4ee3151..6eb2401 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -33,10 +33,8 @@
#include "qed_hw.h"
#include "qed_selftest.h"
-#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
#define QED_ROCE_QPS (8192)
#define QED_ROCE_DPIS (8)
-#endif
static char version[] =
"QLogic FastLinQ 4xxxx Core Module qed " DRV_MODULE_VERSION "\n";
@@ -682,9 +680,7 @@ static int qed_slowpath_setup_int(struct qed_dev *cdev,
enum qed_int_mode int_mode)
{
struct qed_sb_cnt_info sb_cnt_info;
-#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
- int num_l2_queues;
-#endif
+ int num_l2_queues = 0;
int rc;
int i;
@@ -715,8 +711,9 @@ static int qed_slowpath_setup_int(struct qed_dev *cdev,
cdev->int_params.fp_msix_cnt = cdev->int_params.out.num_vectors -
cdev->num_hwfns;
-#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
- num_l2_queues = 0;
+ if (!IS_ENABLED(CONFIG_QED_RDMA))
+ return 0;
+
for_each_hwfn(cdev, i)
num_l2_queues += FEAT_NUM(&cdev->hwfns[i], QED_PF_L2_QUE);
@@ -738,7 +735,6 @@ static int qed_slowpath_setup_int(struct qed_dev *cdev,
DP_VERBOSE(cdev, QED_MSG_RDMA, "roce_msix_cnt=%d roce_msix_base=%d\n",
cdev->int_params.rdma_msix_cnt,
cdev->int_params.rdma_msix_base);
-#endif
return 0;
}
@@ -843,18 +839,20 @@ static void qed_update_pf_params(struct qed_dev *cdev,
{
int i;
-#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
- params->rdma_pf_params.num_qps = QED_ROCE_QPS;
- params->rdma_pf_params.min_dpis = QED_ROCE_DPIS;
- /* divide by 3 the MRs to avoid MF ILT overflow */
- params->rdma_pf_params.num_mrs = RDMA_MAX_TIDS;
- params->rdma_pf_params.gl_pi = QED_ROCE_PROTOCOL_INDEX;
-#endif
for (i = 0; i < cdev->num_hwfns; i++) {
struct qed_hwfn *p_hwfn = &cdev->hwfns[i];
p_hwfn->pf_params = *params;
}
+
+ if (!IS_ENABLED(CONFIG_QED_RDMA))
+ return;
+
+ params->rdma_pf_params.num_qps = QED_ROCE_QPS;
+ params->rdma_pf_params.min_dpis = QED_ROCE_DPIS;
+ /* divide by 3 the MRs to avoid MF ILT overflow */
+ params->rdma_pf_params.num_mrs = RDMA_MAX_TIDS;
+ params->rdma_pf_params.gl_pi = QED_ROCE_PROTOCOL_INDEX;
}
static int qed_slowpath_start(struct qed_dev *cdev,
diff --git a/drivers/net/ethernet/qlogic/qed/qed_roce.c b/drivers/net/ethernet/qlogic/qed/qed_roce.c
index c73638c..187df38 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_roce.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_roce.c
@@ -129,17 +129,12 @@ static void qed_bmap_release_id(struct qed_hwfn *p_hwfn,
}
}
-u32 qed_rdma_get_sb_id(void *p_hwfn, u32 rel_sb_id)
+static u32 qed_rdma_get_sb_id(void *p_hwfn, u32 rel_sb_id)
{
/* First sb id for RoCE is after all the l2 sb */
return FEAT_NUM((struct qed_hwfn *)p_hwfn, QED_PF_L2_QUE) + rel_sb_id;
}
-u32 qed_rdma_query_cau_timer_res(void *rdma_cxt)
-{
- return QED_CAU_DEF_RX_TIMER_RES;
-}
-
static int qed_rdma_alloc(struct qed_hwfn *p_hwfn,
struct qed_ptt *p_ptt,
struct qed_rdma_start_in_params *params)
@@ -275,7 +270,7 @@ static int qed_rdma_alloc(struct qed_hwfn *p_hwfn,
return rc;
}
-void qed_rdma_resc_free(struct qed_hwfn *p_hwfn)
+static void qed_rdma_resc_free(struct qed_hwfn *p_hwfn)
{
struct qed_rdma_info *p_rdma_info = p_hwfn->p_rdma_info;
@@ -527,6 +522,26 @@ static int qed_rdma_start_fw(struct qed_hwfn *p_hwfn,
return qed_spq_post(p_hwfn, p_ent, NULL);
}
+static int qed_rdma_alloc_tid(void *rdma_cxt, u32 *itid)
+{
+ struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
+ int rc;
+
+ DP_VERBOSE(p_hwfn, QED_MSG_RDMA, "Allocate TID\n");
+
+ spin_lock_bh(&p_hwfn->p_rdma_info->lock);
+ rc = qed_rdma_bmap_alloc_id(p_hwfn,
+ &p_hwfn->p_rdma_info->tid_map, itid);
+ spin_unlock_bh(&p_hwfn->p_rdma_info->lock);
+ if (rc)
+ goto out;
+
+ rc = qed_cxt_dynamic_ilt_alloc(p_hwfn, QED_ELEM_TASK, *itid);
+out:
+ DP_VERBOSE(p_hwfn, QED_MSG_RDMA, "Allocate TID - done, rc = %d\n", rc);
+ return rc;
+}
+
static int qed_rdma_reserve_lkey(struct qed_hwfn *p_hwfn)
{
struct qed_rdma_device *dev = p_hwfn->p_rdma_info->dev;
@@ -573,7 +588,7 @@ static int qed_rdma_setup(struct qed_hwfn *p_hwfn,
return qed_rdma_start_fw(p_hwfn, params, p_ptt);
}
-int qed_rdma_stop(void *rdma_cxt)
+static int qed_rdma_stop(void *rdma_cxt)
{
struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
struct rdma_close_func_ramrod_data *p_ramrod;
@@ -629,8 +644,8 @@ int qed_rdma_stop(void *rdma_cxt)
return rc;
}
-int qed_rdma_add_user(void *rdma_cxt,
- struct qed_rdma_add_user_out_params *out_params)
+static int qed_rdma_add_user(void *rdma_cxt,
+ struct qed_rdma_add_user_out_params *out_params)
{
struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
u32 dpi_start_offset;
@@ -664,7 +679,7 @@ int qed_rdma_add_user(void *rdma_cxt,
return rc;
}
-struct qed_rdma_port *qed_rdma_query_port(void *rdma_cxt)
+static struct qed_rdma_port *qed_rdma_query_port(void *rdma_cxt)
{
struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
struct qed_rdma_port *p_port = p_hwfn->p_rdma_info->port;
@@ -680,7 +695,7 @@ struct qed_rdma_port *qed_rdma_query_port(void *rdma_cxt)
return p_port;
}
-struct qed_rdma_device *qed_rdma_query_device(void *rdma_cxt)
+static struct qed_rdma_device *qed_rdma_query_device(void *rdma_cxt)
{
struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
@@ -690,7 +705,7 @@ struct qed_rdma_device *qed_rdma_query_device(void *rdma_cxt)
return p_hwfn->p_rdma_info->dev;
}
-void qed_rdma_free_tid(void *rdma_cxt, u32 itid)
+static void qed_rdma_free_tid(void *rdma_cxt, u32 itid)
{
struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
@@ -701,27 +716,7 @@ void qed_rdma_free_tid(void *rdma_cxt, u32 itid)
spin_unlock_bh(&p_hwfn->p_rdma_info->lock);
}
-int qed_rdma_alloc_tid(void *rdma_cxt, u32 *itid)
-{
- struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
- int rc;
-
- DP_VERBOSE(p_hwfn, QED_MSG_RDMA, "Allocate TID\n");
-
- spin_lock_bh(&p_hwfn->p_rdma_info->lock);
- rc = qed_rdma_bmap_alloc_id(p_hwfn,
- &p_hwfn->p_rdma_info->tid_map, itid);
- spin_unlock_bh(&p_hwfn->p_rdma_info->lock);
- if (rc)
- goto out;
-
- rc = qed_cxt_dynamic_ilt_alloc(p_hwfn, QED_ELEM_TASK, *itid);
-out:
- DP_VERBOSE(p_hwfn, QED_MSG_RDMA, "Allocate TID - done, rc = %d\n", rc);
- return rc;
-}
-
-void qed_rdma_cnq_prod_update(void *rdma_cxt, u8 qz_offset, u16 prod)
+static void qed_rdma_cnq_prod_update(void *rdma_cxt, u8 qz_offset, u16 prod)
{
struct qed_hwfn *p_hwfn;
u16 qz_num;
@@ -816,7 +811,7 @@ static int qed_rdma_get_int(struct qed_dev *cdev, struct qed_int_info *info)
return 0;
}
-int qed_rdma_alloc_pd(void *rdma_cxt, u16 *pd)
+static int qed_rdma_alloc_pd(void *rdma_cxt, u16 *pd)
{
struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
u32 returned_id;
@@ -1985,9 +1980,9 @@ int qed_roce_destroy_qp(struct qed_hwfn *p_hwfn, struct qed_rdma_qp *qp)
return 0;
}
-int qed_rdma_query_qp(void *rdma_cxt,
- struct qed_rdma_qp *qp,
- struct qed_rdma_query_qp_out_params *out_params)
+static int qed_rdma_query_qp(void *rdma_cxt,
+ struct qed_rdma_qp *qp,
+ struct qed_rdma_query_qp_out_params *out_params)
{
struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
int rc;
@@ -2022,7 +2017,7 @@ int qed_rdma_query_qp(void *rdma_cxt,
return rc;
}
-int qed_rdma_destroy_qp(void *rdma_cxt, struct qed_rdma_qp *qp)
+static int qed_rdma_destroy_qp(void *rdma_cxt, struct qed_rdma_qp *qp)
{
struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
int rc = 0;
@@ -2215,9 +2210,9 @@ static int qed_roce_modify_qp(struct qed_hwfn *p_hwfn,
return rc;
}
-int qed_rdma_modify_qp(void *rdma_cxt,
- struct qed_rdma_qp *qp,
- struct qed_rdma_modify_qp_in_params *params)
+static int qed_rdma_modify_qp(void *rdma_cxt,
+ struct qed_rdma_qp *qp,
+ struct qed_rdma_modify_qp_in_params *params)
{
struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
enum qed_roce_qp_state prev_state;
@@ -2312,8 +2307,9 @@ int qed_rdma_modify_qp(void *rdma_cxt,
return rc;
}
-int qed_rdma_register_tid(void *rdma_cxt,
- struct qed_rdma_register_tid_in_params *params)
+static int
+qed_rdma_register_tid(void *rdma_cxt,
+ struct qed_rdma_register_tid_in_params *params)
{
struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
struct rdma_register_tid_ramrod_data *p_ramrod;
@@ -2450,7 +2446,7 @@ int qed_rdma_register_tid(void *rdma_cxt,
return rc;
}
-int qed_rdma_deregister_tid(void *rdma_cxt, u32 itid)
+static int qed_rdma_deregister_tid(void *rdma_cxt, u32 itid)
{
struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
struct rdma_deregister_tid_ramrod_data *p_ramrod;
@@ -2561,7 +2557,8 @@ void qed_rdma_dpm_bar(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
qed_rdma_dpm_conf(p_hwfn, p_ptt);
}
-int qed_rdma_start(void *rdma_cxt, struct qed_rdma_start_in_params *params)
+static int qed_rdma_start(void *rdma_cxt,
+ struct qed_rdma_start_in_params *params)
{
struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
struct qed_ptt *p_ptt;
@@ -2601,7 +2598,7 @@ static int qed_rdma_init(struct qed_dev *cdev,
return qed_rdma_start(QED_LEADING_HWFN(cdev), params);
}
-void qed_rdma_remove_user(void *rdma_cxt, u16 dpi)
+static void qed_rdma_remove_user(void *rdma_cxt, u16 dpi)
{
struct qed_hwfn *p_hwfn = (struct qed_hwfn *)rdma_cxt;
diff --git a/drivers/net/ethernet/qlogic/qed/qed_roce.h b/drivers/net/ethernet/qlogic/qed/qed_roce.h
index 2f091e8..6914131 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_roce.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_roce.h
@@ -181,36 +181,55 @@ struct qed_rdma_qp {
dma_addr_t shared_queue_phys_addr;
};
-int
-qed_rdma_add_user(void *rdma_cxt,
- struct qed_rdma_add_user_out_params *out_params);
-int qed_rdma_alloc_pd(void *rdma_cxt, u16 *pd);
-int qed_rdma_alloc_tid(void *rdma_cxt, u32 *tid);
-int qed_rdma_deregister_tid(void *rdma_cxt, u32 tid);
-void qed_rdma_free_tid(void *rdma_cxt, u32 tid);
-struct qed_rdma_device *qed_rdma_query_device(void *rdma_cxt);
-struct qed_rdma_port *qed_rdma_query_port(void *rdma_cxt);
-int
-qed_rdma_register_tid(void *rdma_cxt,
- struct qed_rdma_register_tid_in_params *params);
-void qed_rdma_remove_user(void *rdma_cxt, u16 dpi);
-int qed_rdma_start(void *p_hwfn, struct qed_rdma_start_in_params *params);
-int qed_rdma_stop(void *rdma_cxt);
-u32 qed_rdma_get_sb_id(void *p_hwfn, u32 rel_sb_id);
-u32 qed_rdma_query_cau_timer_res(void *p_hwfn);
-void qed_rdma_cnq_prod_update(void *rdma_cxt, u8 cnq_index, u16 prod);
-void qed_rdma_resc_free(struct qed_hwfn *p_hwfn);
+#if IS_ENABLED(CONFIG_QED_RDMA)
+void qed_rdma_dpm_bar(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt);
void qed_async_roce_event(struct qed_hwfn *p_hwfn,
struct event_ring_entry *p_eqe);
-int qed_rdma_destroy_qp(void *rdma_cxt, struct qed_rdma_qp *qp);
-int qed_rdma_modify_qp(void *rdma_cxt, struct qed_rdma_qp *qp,
- struct qed_rdma_modify_qp_in_params *params);
-int qed_rdma_query_qp(void *rdma_cxt, struct qed_rdma_qp *qp,
- struct qed_rdma_query_qp_out_params *out_params);
-
-#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
-void qed_rdma_dpm_bar(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt);
+void qed_ll2b_complete_tx_gsi_packet(struct qed_hwfn *p_hwfn,
+ u8 connection_handle,
+ void *cookie,
+ dma_addr_t first_frag_addr,
+ bool b_last_fragment, bool b_last_packet);
+void qed_ll2b_release_tx_gsi_packet(struct qed_hwfn *p_hwfn,
+ u8 connection_handle,
+ void *cookie,
+ dma_addr_t first_frag_addr,
+ bool b_last_fragment, bool b_last_packet);
+void qed_ll2b_complete_rx_gsi_packet(struct qed_hwfn *p_hwfn,
+ u8 connection_handle,
+ void *cookie,
+ dma_addr_t rx_buf_addr,
+ u16 data_length,
+ u8 data_length_error,
+ u16 parse_flags,
+ u16 vlan,
+ u32 src_mac_addr_hi,
+ u16 src_mac_addr_lo, bool b_last_packet);
#else
-void qed_rdma_dpm_bar(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) {}
+static inline void qed_rdma_dpm_bar(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) {}
+static inline void qed_async_roce_event(struct qed_hwfn *p_hwfn, struct event_ring_entry *p_eqe) {}
+static inline void qed_ll2b_complete_tx_gsi_packet(struct qed_hwfn *p_hwfn,
+ u8 connection_handle,
+ void *cookie,
+ dma_addr_t first_frag_addr,
+ bool b_last_fragment,
+ bool b_last_packet) {}
+static inline void qed_ll2b_release_tx_gsi_packet(struct qed_hwfn *p_hwfn,
+ u8 connection_handle,
+ void *cookie,
+ dma_addr_t first_frag_addr,
+ bool b_last_fragment,
+ bool b_last_packet) {}
+static inline void qed_ll2b_complete_rx_gsi_packet(struct qed_hwfn *p_hwfn,
+ u8 connection_handle,
+ void *cookie,
+ dma_addr_t rx_buf_addr,
+ u16 data_length,
+ u8 data_length_error,
+ u16 parse_flags,
+ u16 vlan,
+ u32 src_mac_addr_hi,
+ u16 src_mac_addr_lo,
+ bool b_last_packet) {}
#endif
#endif
diff --git a/drivers/net/ethernet/qlogic/qed/qed_spq.c b/drivers/net/ethernet/qlogic/qed/qed_spq.c
index caff415..9fbaf94 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_spq.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_spq.c
@@ -28,9 +28,7 @@
#include "qed_reg_addr.h"
#include "qed_sp.h"
#include "qed_sriov.h"
-#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
#include "qed_roce.h"
-#endif
/***************************************************************************
* Structures & Definitions
@@ -240,11 +238,9 @@ static int qed_spq_hw_post(struct qed_hwfn *p_hwfn,
struct event_ring_entry *p_eqe)
{
switch (p_eqe->protocol_id) {
-#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
case PROTOCOLID_ROCE:
qed_async_roce_event(p_hwfn, p_eqe);
return 0;
-#endif
case PROTOCOLID_COMMON:
return qed_sriov_eqe_event(p_hwfn,
p_eqe->opcode,
diff --git a/drivers/net/ethernet/qlogic/qede/Makefile b/drivers/net/ethernet/qlogic/qede/Makefile
index 28dc589..048a230 100644
--- a/drivers/net/ethernet/qlogic/qede/Makefile
+++ b/drivers/net/ethernet/qlogic/qede/Makefile
@@ -2,4 +2,4 @@ obj-$(CONFIG_QEDE) := qede.o
qede-y := qede_main.o qede_ethtool.o
qede-$(CONFIG_DCB) += qede_dcbnl.o
-qede-$(CONFIG_INFINIBAND_QEDR) += qede_roce.o
+qede-$(CONFIG_QED_RDMA) += qede_roce.o
diff --git a/include/linux/qed/qede_roce.h b/include/linux/qed/qede_roce.h
index 99fbe6d..f48d64b 100644
--- a/include/linux/qed/qede_roce.h
+++ b/include/linux/qed/qede_roce.h
@@ -68,7 +68,7 @@ struct qedr_driver {
bool qede_roce_supported(struct qede_dev *dev);
-#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
+#if IS_ENABLED(CONFIG_QED_RDMA)
int qede_roce_dev_add(struct qede_dev *dev);
void qede_roce_dev_event_open(struct qede_dev *dev);
void qede_roce_dev_event_close(struct qede_dev *dev);
--
1.9.3
^ permalink raw reply related
* [PATCH net 1/3] qed: Fix static checker warning.
From: Yuval Mintz @ 2016-10-13 19:57 UTC (permalink / raw)
To: davem, netdev; +Cc: arnd, dan.carpenter, Yuval Mintz
In-Reply-To: <1476388623-31032-1-git-send-email-Yuval.Mintz@caviumnetworks.com>
Smatch compains about qed_roce_ll2_tx() dereference
of the 'cdev' variable while testing its validity later.
As the validation checking is an over-kill [variable would always
be set], simply remove it.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Fixes: abd49676c707 ("qed: Add RoCE ll2 & GSI support")
Signed-off-by: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
---
drivers/net/ethernet/qlogic/qed/qed_roce.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_roce.c b/drivers/net/ethernet/qlogic/qed/qed_roce.c
index 76831a3..c73638c 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_roce.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_roce.c
@@ -2809,11 +2809,6 @@ static int qed_roce_ll2_stop(struct qed_dev *cdev)
struct qed_roce_ll2_info *roce_ll2 = hwfn->ll2;
int rc;
- if (!cdev) {
- DP_ERR(cdev, "qed roce ll2 stop: invalid cdev\n");
- return -EINVAL;
- }
-
if (roce_ll2->handle == QED_LL2_UNUSED_HANDLE) {
DP_ERR(cdev, "qed roce ll2 stop: cannot stop an unused LL2\n");
return -EINVAL;
@@ -2850,7 +2845,7 @@ static int qed_roce_ll2_tx(struct qed_dev *cdev,
int rc;
int i;
- if (!cdev || !pkt || !params) {
+ if (!pkt || !params) {
DP_ERR(cdev,
"roce ll2 tx: failed tx because one of the following is NULL - drv=%p, pkt=%p, params=%p\n",
cdev, pkt, params);
--
1.9.3
^ permalink raw reply related
* [PATCH net 0/3] qed: Fix dependencies and warnings series
From: Yuval Mintz @ 2016-10-13 19:57 UTC (permalink / raw)
To: davem, netdev; +Cc: arnd, dan.carpenter, Yuval Mintz
Hi Dave,
The first patch in this series follows Dan Carpenter's reports about
Smatch warnings for recent qed additions and fixes those.
The second patch is the most significant one [and the reason this is
ntended for 'net'] - it's based on Arnd Bermann's suggestion for fixing
compilation issues that were introduced with the roce addition as a result
of certain combinations of qed, qede and qedr Kconfig options.
The third follows the discussion with Arnd and clears a lot of the warnings
that arise when compiling the drivers with "C=1".
Please consider applying this series to 'net'.
Thanks,
Yuval
Yuval Mintz (3):
qed: Fix static checker warning.
qed*: Fix Kconfig dependencies with INFINIBAND_QEDR
qed: Additional work toward cleaning C=1
drivers/net/ethernet/qlogic/Kconfig | 4 +
drivers/net/ethernet/qlogic/qed/Makefile | 2 +-
drivers/net/ethernet/qlogic/qed/qed_cxt.c | 15 +-
drivers/net/ethernet/qlogic/qed/qed_debug.c | 53 +++----
drivers/net/ethernet/qlogic/qed/qed_dev.c | 19 +--
drivers/net/ethernet/qlogic/qed/qed_ll2.c | 16 +--
drivers/net/ethernet/qlogic/qed/qed_ll2.h | 20 ---
drivers/net/ethernet/qlogic/qed/qed_main.c | 30 ++--
drivers/net/ethernet/qlogic/qed/qed_roce.c | 216 ++++++++--------------------
drivers/net/ethernet/qlogic/qed/qed_roce.h | 95 ++++++------
drivers/net/ethernet/qlogic/qed/qed_sp.h | 1 -
drivers/net/ethernet/qlogic/qed/qed_spq.c | 4 -
drivers/net/ethernet/qlogic/qede/Makefile | 2 +-
include/linux/qed/qede_roce.h | 2 +-
14 files changed, 179 insertions(+), 300 deletions(-)
--
1.9.3
^ permalink raw reply
* Re: slab corruption with current -git
From: Linus Torvalds @ 2016-10-13 19:49 UTC (permalink / raw)
To: Markus Trippelsdorf
Cc: Christoph Lameter, Jens Axboe, Linux Kernel Mailing List,
Aaron Conole, David Miller, Pablo Neira Ayuso, linux-fsdevel,
Al Viro, NetFilter, Network Development, Andrew Morton,
Florian Westphal, Theodore Ts'o
In-Reply-To: <20161013062703.GC310@x4>
On Wed, Oct 12, 2016 at 11:27 PM, Markus Trippelsdorf
<markus@trippelsdorf.de> wrote:
>
> Yeah.
>
> 105 entry->orig_ops = reg;
> 106 entry->ops = *reg;
> 107 entry->next = NULL;
So ipt_register_table() does:
ret = nf_register_net_hooks(net, ops, hweight32(table->valid_hooks));
and then nf_register_net_hooks() just does
for (i = 0; i < n; i++) {
err = nf_register_net_hook(net, ®[i]);
so if the *reg is uninitialized, it means that it's the 'ops[]' array
that isn't actually really valid in "valid_hooks". Odd. They should
all be initialized by xt_hook_ops_alloc(), no?
That said, xt_hook_ops_alloc() itself is odd. Lookie here, this is the
loop that initializes things:
for (i = 0, hooknum = 0; i < num_hooks && hook_mask != 0;
hook_mask >>= 1, ++hooknum) {
and it makes no sense to me how that tests *both* "i < num_hools" and
"hook_mask != 0".
Why? Because
num_hooks = hweight32(hook_mask);
so it's entirely redundant. num_hooks is already how many bits are on
in hook_mask, so that test is just duplicating the same thing twice
("have we done less than that number of bits" and "do we have any bits
less").
I don't know. There's something odd going on. Regardless, thsi is a
different problem from the nf_register_net_hook() list handling, so
I'll leave it to the networking people. David?
Linus
^ permalink raw reply
* Re: XDP_TX bug report on mlx4
From: Brenden Blanco @ 2016-10-13 19:46 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev@vger.kernel.org, Tariq Toukan, Tom Herbert, Saeed Mahameed,
Rana Shahout, Eran Ben Elisha
In-Reply-To: <20160918235943.GA18685@gmail.com>
On Sun, Sep 18, 2016 at 07:59:43PM -0400, Brenden Blanco wrote:
> On Fri, Sep 16, 2016 at 09:24:43PM +0200, Jesper Dangaard Brouer wrote:
> > On Fri, 16 Sep 2016 12:17:27 -0700
> > Brenden Blanco <bblanco@plumgrid.com> wrote:
> >
> > > On Fri, Sep 16, 2016 at 09:03:40PM +0200, Jesper Dangaard Brouer wrote:
> > > > Hi Brenden,
> > > >
> > > > I've discovered a bug with XDP_TX recycling of pages in the mlx4 driver.
> > > >
> > > > If I increase the number of RX and TX queues/channels via ethtool cmd:
> > > > ethtool -L mlx4p1 rx 10 tx 10
> > > >
> > > > Then when running the xdp2 program, which does XDP_TX, the kernel will
> > > > crash with page errors, because the page refcnt goes to zero or even
> > > > minus. I've noticed pages delivered to mlx4_en_rx_recycle() can have
> > > > a page refcnt of zero, which is wrong, they should always have 1 (for
> > > > XDP).
> > > >
> > > > Debugging it further, I find that this can happen when mlx4_en_rx_recycle()
> > > > is called from mlx4_en_recycle_tx_desc(). This is the TX cleanup function,
> > > > associated with TX ring queues used for XDP_TX only. No others than the
> > > > XDP_TX action should be able to place packets into these TX rings
> > > > which call mlx4_en_recycle_tx_desc().
> > >
> > > Sounds pretty straightforward, let me look into it.
> >
> > Here is some debug info I instrumented my kernel with, and I've
> > attached my minicom output with a warning and a panic.
>
> Thanks for the info.
>
> I've also spent this weekend trying to debug (pretty easy to reproduce),
> but with no conclusive answer. I was investigating the sequence in
> mlx4_en_stop_port to see if rx might still be running through the
> function, on the theory that the xdp tx might race with
> mlx4_en_free_tx_buf. For instance, I tried moving the napi_synchronize
> loop to be just below the msleep(10). No improvement.
>
> Unfortunately, I'm out of options, since my one test box has decided not
> to reboot itself automatically, and I'll be out of email communication
> (for 3 weeks) before anybody can physicially resuscitate it (tomorrow).
Sorry to leave this hanging for so long. I've just recently come back
from vacation and I think I now have a fix for this. I'll send a patch
to the list shortly.
> >
> > Enable some driver debug printks via::
> > ethtool -s mlx4p1 msglvl drv on
> >
> > Debug normal situation::
> >
> [...]
^ permalink raw reply
* Re: [PATCH net-next 0/5] udp: Flow dissection for tunnels
From: David Miller @ 2016-10-13 19:17 UTC (permalink / raw)
To: tom; +Cc: netdev, kernel-team
In-Reply-To: <20161012232547.2383961-1-tom@herbertland.com>
From: Tom Herbert <tom@herbertland.com>
Date: Wed, 12 Oct 2016 16:25:42 -0700
> Since performing a UDP lookup on every packet might be expensive I
> added a static key check to bypass the lookup if there are no
> sockets with flow_dissect set. I should mention that doing the
> lookup wasn't particularly a big hit anyway.
I think this new static key is unnecessary, as it is equivalent
to: (udp_encap_needed + udpv6_encap_needed)
This socket lookup is very heavy handed, and I realize that you
need this because we no longer store the encapsulation socket in
skb->sk these days.
Can you talk about the various code paths that lead into the
flow dissector and why the UDP socket lookup is needed? Maybe
we can propagate it another way, at least on TX.
Thanks.
^ permalink raw reply
* [PATCH net-next v11 1/1] net: phy: Cleanup the Edge-Rate feature in Microsemi PHYs.
From: Allan W. Nielsen @ 2016-10-13 18:21 UTC (permalink / raw)
To: netdev; +Cc: andrew, f.fainelli, raju.lakkaraju, allan.nielsen
In-Reply-To: <1476382890-7275-1-git-send-email-allan.nielsen@microsemi.com>
Edge-Rate cleanup include the following:
- Updated device tree bindings documentation for edge-rate
- The edge-rate is now specified as a "slowdown", meaning that it is now
being specified as positive values instead of negative (both
documentation and implementation wise).
- Only explicitly documented values for "vsc8531,vddmac" and
"vsc8531,edge-slowdown" are accepted by the device driver.
- Deleted include/dt-bindings/net/mscc-phy-vsc8531.h as it was not needed.
- Read/validate devicetree settings in probe instead of init
Signed-off-by: Allan W. Nielsen <allan.nielsen@microsemi.com>
Signed-off-by: Raju Lakkaraju <raju.lakkaraju@microsemi.com>
---
.../devicetree/bindings/net/mscc-phy-vsc8531.txt | 51 ++++----
drivers/net/phy/mscc.c | 135 ++++++++++-----------
include/dt-bindings/net/mscc-phy-vsc8531.h | 21 ----
3 files changed, 90 insertions(+), 117 deletions(-)
delete mode 100644 include/dt-bindings/net/mscc-phy-vsc8531.h
diff --git a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
index 99c7eb0..bdefefc6 100644
--- a/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
+++ b/Documentation/devicetree/bindings/net/mscc-phy-vsc8531.txt
@@ -6,22 +6,27 @@ Required properties:
Documentation/devicetree/bindings/net/phy.txt
Optional properties:
-- vsc8531,vddmac : The vddmac in mV.
+- vsc8531,vddmac : The vddmac in mV. Allowed values is listed
+ in the first row of Table 1 (below).
+ This property is only used in combination
+ with the 'edge-slowdown' property.
+ Default value is 3300.
- vsc8531,edge-slowdown : % the edge should be slowed down relative to
- the fastest possible edge time. Native sign
- need not enter.
+ the fastest possible edge time.
Edge rate sets the drive strength of the MAC
- interface output signals. Changing the drive
- strength will affect the edge rate of the output
- signal. The goal of this setting is to help
- reduce electrical emission (EMI) by being able
- to reprogram drive strength and in effect slow
- down the edge rate if desired. Table 1 shows the
- impact to the edge rate per VDDMAC supply for each
- drive strength setting.
- Ref: Table:1 - Edge rate change below.
-
-Note: see dt-bindings/net/mscc-phy-vsc8531.h for applicable values
+ interface output signals. Changing the
+ drive strength will affect the edge rate of
+ the output signal. The goal of this setting
+ is to help reduce electrical emission (EMI)
+ by being able to reprogram drive strength
+ and in effect slow down the edge rate if
+ desired.
+ To adjust the edge-slowdown, the 'vddmac'
+ must be specified. Table 1 lists the
+ supported edge-slowdown values for a given
+ 'vddmac'.
+ Default value is 0%.
+ Ref: Table:1 - Edge rate change (below).
Table: 1 - Edge rate change
----------------------------------------------------------------|
@@ -29,23 +34,23 @@ Table: 1 - Edge rate change
| |
| 3300 mV 2500 mV 1800 mV 1500 mV |
|---------------------------------------------------------------|
-| Default Deafult Default Default |
+| 0% 0% 0% 0% |
| (Fastest) (recommended) (recommended) |
|---------------------------------------------------------------|
-| -2% -3% -5% -6% |
+| 2% 3% 5% 6% |
|---------------------------------------------------------------|
-| -4% -6% -9% -14% |
+| 4% 6% 9% 14% |
|---------------------------------------------------------------|
-| -7% -10% -16% -21% |
+| 7% 10% 16% 21% |
|(recommended) (recommended) |
|---------------------------------------------------------------|
-| -10% -14% -23% -29% |
+| 10% 14% 23% 29% |
|---------------------------------------------------------------|
-| -17% -23% -35% -42% |
+| 17% 23% 35% 42% |
|---------------------------------------------------------------|
-| -29% -37% -52% -58% |
+| 29% 37% 52% 58% |
|---------------------------------------------------------------|
-| -53% -63% -76% -77% |
+| 53% 63% 76% 77% |
| (slowest) |
|---------------------------------------------------------------|
@@ -54,5 +59,5 @@ Example:
vsc8531_0: ethernet-phy@0 {
compatible = "ethernet-phy-id0007.0570";
vsc8531,vddmac = <3300>;
- vsc8531,edge-slowdown = <21>;
+ vsc8531,edge-slowdown = <7>;
};
diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index 77a6671..113616b 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -12,7 +12,6 @@
#include <linux/mii.h>
#include <linux/phy.h>
#include <linux/of.h>
-#include <dt-bindings/net/mscc-phy-vsc8531.h>
#include <linux/netdevice.h>
enum rgmii_rx_clock_delay {
@@ -69,23 +68,29 @@ enum rgmii_rx_clock_delay {
#define PHY_ID_VSC8531 0x00070570
#define PHY_ID_VSC8541 0x00070770
-struct edge_rate_table {
- u16 vddmac;
- int slowdown[MSCC_SLOWDOWN_MAX];
-};
+#define MSCC_VDDMAC_1500 1500
+#define MSCC_VDDMAC_1800 1800
+#define MSCC_VDDMAC_2500 2500
+#define MSCC_VDDMAC_3300 3300
-struct edge_rate_table edge_table[MSCC_VDDMAC_MAX] = {
- {3300, { 0, -2, -4, -7, -10, -17, -29, -53} },
- {2500, { 0, -3, -6, -10, -14, -23, -37, -63} },
- {1800, { 0, -5, -9, -16, -23, -35, -52, -76} },
- {1500, { 0, -6, -14, -21, -29, -42, -58, -77} },
+struct vsc8531_private {
+ int rate_magic;
};
-struct vsc8531_private {
- u8 edge_slowdown;
+#ifdef CONFIG_OF_MDIO
+struct vsc8531_edge_rate_table {
u16 vddmac;
+ u8 slowdown[8];
};
+static const struct vsc8531_edge_rate_table edge_table[] = {
+ {MSCC_VDDMAC_3300, { 0, 2, 4, 7, 10, 17, 29, 53} },
+ {MSCC_VDDMAC_2500, { 0, 3, 6, 10, 14, 23, 37, 63} },
+ {MSCC_VDDMAC_1800, { 0, 5, 9, 16, 23, 35, 52, 76} },
+ {MSCC_VDDMAC_1500, { 0, 6, 14, 21, 29, 42, 58, 77} },
+};
+#endif /* CONFIG_OF_MDIO */
+
static int vsc85xx_phy_page_set(struct phy_device *phydev, u8 page)
{
int rc;
@@ -205,29 +210,43 @@ static void vsc85xx_wol_get(struct phy_device *phydev,
mutex_unlock(&phydev->lock);
}
-static u8 edge_rate_magic_get(u16 vddmac,
- int slowdown)
+#ifdef CONFIG_OF_MDIO
+static int vsc85xx_edge_rate_magic_get(struct phy_device *phydev)
{
- int rc = (MSCC_SLOWDOWN_MAX - 1);
- u8 vdd;
u8 sd;
+ u16 vdd;
+ int rc, i, j;
+ struct device *dev = &phydev->mdio.dev;
+ struct device_node *of_node = dev->of_node;
+ u8 sd_array_size = ARRAY_SIZE(edge_table[0].slowdown);
- for (vdd = 0; vdd < MSCC_VDDMAC_MAX; vdd++) {
- if (edge_table[vdd].vddmac == vddmac) {
- for (sd = 0; sd < MSCC_SLOWDOWN_MAX; sd++) {
- if (edge_table[vdd].slowdown[sd] <= slowdown) {
- rc = (MSCC_SLOWDOWN_MAX - sd - 1);
- break;
- }
- }
- }
- }
+ if (!of_node)
+ return -ENODEV;
- return rc;
+ rc = of_property_read_u16(of_node, "vsc8531,vddmac", &vdd);
+ if (rc != 0)
+ vdd = MSCC_VDDMAC_3300;
+
+ rc = of_property_read_u8(of_node, "vsc8531,edge-slowdown", &sd);
+ if (rc != 0)
+ sd = 0;
+
+ for (i = 0; i < ARRAY_SIZE(edge_table); i++)
+ if (edge_table[i].vddmac == vdd)
+ for (j = 0; j < sd_array_size; j++)
+ if (edge_table[i].slowdown[j] == sd)
+ return (sd_array_size - j - 1);
+
+ return -EINVAL;
+}
+#else
+static int vsc85xx_edge_rate_magic_get(struct phy_device *phydev)
+{
+ return 0;
}
+#endif /* CONFIG_OF_MDIO */
-static int vsc85xx_edge_rate_cntl_set(struct phy_device *phydev,
- u8 edge_rate)
+static int vsc85xx_edge_rate_cntl_set(struct phy_device *phydev, u8 edge_rate)
{
int rc;
u16 reg_val;
@@ -308,45 +327,10 @@ static int vsc85xx_default_config(struct phy_device *phydev)
return rc;
}
-#ifdef CONFIG_OF_MDIO
-static int vsc8531_of_init(struct phy_device *phydev)
-{
- int rc;
- struct vsc8531_private *vsc8531 = phydev->priv;
- struct device *dev = &phydev->mdio.dev;
- struct device_node *of_node = dev->of_node;
-
- if (!of_node)
- return -ENODEV;
-
- rc = of_property_read_u16(of_node, "vsc8531,vddmac",
- &vsc8531->vddmac);
- if (rc == -EINVAL)
- vsc8531->vddmac = MSCC_VDDMAC_3300;
- rc = of_property_read_u8(of_node, "vsc8531,edge-slowdown",
- &vsc8531->edge_slowdown);
- if (rc == -EINVAL)
- vsc8531->edge_slowdown = 0;
-
- rc = 0;
- return rc;
-}
-#else
-static int vsc8531_of_init(struct phy_device *phydev)
-{
- return 0;
-}
-#endif /* CONFIG_OF_MDIO */
-
static int vsc85xx_config_init(struct phy_device *phydev)
{
int rc;
struct vsc8531_private *vsc8531 = phydev->priv;
- u8 edge_rate;
-
- rc = vsc8531_of_init(phydev);
- if (rc)
- return rc;
rc = vsc85xx_default_config(phydev);
if (rc)
@@ -356,9 +340,7 @@ static int vsc85xx_config_init(struct phy_device *phydev)
if (rc)
return rc;
- edge_rate = edge_rate_magic_get(vsc8531->vddmac,
- -(int)vsc8531->edge_slowdown);
- rc = vsc85xx_edge_rate_cntl_set(phydev, edge_rate);
+ rc = vsc85xx_edge_rate_cntl_set(phydev, vsc8531->rate_magic);
if (rc)
return rc;
@@ -396,14 +378,21 @@ static int vsc85xx_config_intr(struct phy_device *phydev)
static int vsc85xx_probe(struct phy_device *phydev)
{
+ int rate_magic;
struct vsc8531_private *vsc8531;
+ rate_magic = vsc85xx_edge_rate_magic_get(phydev);
+ if (rate_magic < 0)
+ return rate_magic;
+
vsc8531 = devm_kzalloc(&phydev->mdio.dev, sizeof(*vsc8531), GFP_KERNEL);
if (!vsc8531)
return -ENOMEM;
phydev->priv = vsc8531;
+ vsc8531->rate_magic = rate_magic;
+
return 0;
}
@@ -424,9 +413,9 @@ static struct phy_driver vsc85xx_driver[] = {
.config_intr = &vsc85xx_config_intr,
.suspend = &genphy_suspend,
.resume = &genphy_resume,
- .probe = &vsc85xx_probe,
- .set_wol = &vsc85xx_wol_set,
- .get_wol = &vsc85xx_wol_get,
+ .probe = &vsc85xx_probe,
+ .set_wol = &vsc85xx_wol_set,
+ .get_wol = &vsc85xx_wol_get,
},
{
.phy_id = PHY_ID_VSC8541,
@@ -443,9 +432,9 @@ static struct phy_driver vsc85xx_driver[] = {
.config_intr = &vsc85xx_config_intr,
.suspend = &genphy_suspend,
.resume = &genphy_resume,
- .probe = &vsc85xx_probe,
- .set_wol = &vsc85xx_wol_set,
- .get_wol = &vsc85xx_wol_get,
+ .probe = &vsc85xx_probe,
+ .set_wol = &vsc85xx_wol_set,
+ .get_wol = &vsc85xx_wol_get,
}
};
diff --git a/include/dt-bindings/net/mscc-phy-vsc8531.h b/include/dt-bindings/net/mscc-phy-vsc8531.h
deleted file mode 100644
index 2383dd2..0000000
--- a/include/dt-bindings/net/mscc-phy-vsc8531.h
+++ /dev/null
@@ -1,21 +0,0 @@
-/*
- * Device Tree constants for Microsemi VSC8531 PHY
- *
- * Author: Nagaraju Lakkaraju
- *
- * License: Dual MIT/GPL
- * Copyright (c) 2016 Microsemi Corporation
- */
-
-#ifndef _DT_BINDINGS_MSCC_VSC8531_H
-#define _DT_BINDINGS_MSCC_VSC8531_H
-
-/* MAC interface Edge rate control VDDMAC in milli Volts */
-#define MSCC_VDDMAC_3300 3300
-#define MSCC_VDDMAC_2500 2500
-#define MSCC_VDDMAC_1800 1800
-#define MSCC_VDDMAC_1500 1500
-#define MSCC_VDDMAC_MAX 4
-#define MSCC_SLOWDOWN_MAX 8
-
-#endif
--
2.7.3
^ permalink raw reply related
* [PATCH net-next v11 0/1] net: phy: Cleanup the Edge-Rate feature in Microsemi PHYs (rebase)
From: Allan W. Nielsen @ 2016-10-13 18:21 UTC (permalink / raw)
To: netdev; +Cc: andrew, f.fainelli, raju.lakkaraju, allan.nielsen
In-Reply-To: <20161013.100141.1588302672560551253.davem@davemloft.net>
Hi,
On 13/10/16 10:01, David Miller wrote:
> This doesn't apply cleanly to net-next, please respin.
Sorry about that... But, I have already send a rebased version (v11) which does
apply cleanly. This time I will try and see if I can get git-send-email to use
the same thread ID.
Anyway, here is a re-post of version 11 (which should apply cleanly on-top of
net-next)
v10 was reviewed by Florian and Andrew:
On 12/10/16 02:14, Florian Fainelli wrote:
> On 10/10/2016 07:13 AM, Allan W. Nielsen wrote:
> > Edge-Rate cleanup include the following:
> > ...
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
On 11/10/16 15:09, Andrew Lunn wrote:
> On Mon, Oct 10, 2016 at 04:13:45PM +0200, Allan W. Nielsen wrote:
> > Edge-Rate cleanup include the following:
> > ...
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Best regards
Allan W. Nielsen
^ permalink raw reply
* Re: [PATCH] iwlwifi: pcie: fix SPLC structure parsing
From: Luca Coelho @ 2016-10-13 17:49 UTC (permalink / raw)
To: Paul Bolle, linux-wireless, chris
Cc: linuxwifi, emmanuel.grumbach, johannes, kvalo, oren.givon, netdev,
linux-kernel
In-Reply-To: <1476363353.1999.17.camel@tiscali.nl>
On Thu, 2016-10-13 at 14:55 +0200, Paul Bolle wrote:
> On Thu, 2016-10-13 at 15:44 +0300, Luca Coelho wrote:
> > Even though there is apparently something wrong with this part of the
> > ACPI table on you laptop, since it doesn't match our specifications.
> > In any case, it's mostly harmless.
>
>
> Would a correct implementation by Dell have any benefits for the users
> of these laptops? In other words: should I bother somehow contacting
> Dell and point them to this discussion in order to have them fix this?
This value provides a way for the OEM to fine tune the power budget of
the device. This is (usually) used to prevent parts of the platform
from getting too hot. We have a certain default that is good enough
for most cases. If Dell didn't want to set proper limits for this
device, it probably means that it is not a concern. Dell does set
these values correctly for other platforms.
So, I guess it's up to you if you want to clarify this with them. This
could be some default "blank" values when they don't want to change
them.
^ permalink raw reply
* [PATCH 1/2] IPv6: Drop the temporary address regen_timer
From: Jiri Bohac @ 2016-10-13 16:50 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy
Cc: netdev
The randomized interface identifier (rndid) was periodically updated from
the regen_timer timer. Simplify the code by updating the rndid only when
needed by ipv6_try_regen_rndid().
This makes the follow-up DESYNC_FACTOR fix much simpler. Also it fixes a
reference counting error in this error path, where an in6_dev_put was
missing:
err = addrconf_sysctl_register(ndev);
if (err) {
ipv6_mc_destroy_dev(ndev);
- del_timer(&ndev->regen_timer);
snmp6_unregister_dev(ndev);
goto err_release;
Signed-off-by: Jiri Bohac <jbohac@suse.cz>
---
include/net/if_inet6.h | 1 -
net/ipv6/addrconf.c | 61 ++++++++------------------------------------------
2 files changed, 9 insertions(+), 53 deletions(-)
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 515352c..ae70735 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -191,7 +191,6 @@ struct inet6_dev {
int dead;
u8 rndid[8];
- struct timer_list regen_timer;
struct list_head tempaddr_list;
struct in6_addr token;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index d8983e1..63fc857 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -147,9 +147,8 @@ static inline void addrconf_sysctl_unregister(struct inet6_dev *idev)
}
#endif
-static void __ipv6_regen_rndid(struct inet6_dev *idev);
-static void __ipv6_try_regen_rndid(struct inet6_dev *idev, struct in6_addr *tmpaddr);
-static void ipv6_regen_rndid(unsigned long data);
+static void ipv6_regen_rndid(struct inet6_dev *idev);
+static void ipv6_try_regen_rndid(struct inet6_dev *idev, struct in6_addr *tmpaddr);
static int ipv6_generate_eui64(u8 *eui, struct net_device *dev);
static int ipv6_count_addresses(struct inet6_dev *idev);
@@ -409,9 +408,7 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
goto err_release;
}
- /* One reference from device. We must do this before
- * we invoke __ipv6_regen_rndid().
- */
+ /* One reference from device. */
in6_dev_hold(ndev);
if (dev->flags & (IFF_NOARP | IFF_LOOPBACK))
@@ -425,17 +422,14 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
#endif
INIT_LIST_HEAD(&ndev->tempaddr_list);
- setup_timer(&ndev->regen_timer, ipv6_regen_rndid, (unsigned long)ndev);
if ((dev->flags&IFF_LOOPBACK) ||
dev->type == ARPHRD_TUNNEL ||
dev->type == ARPHRD_TUNNEL6 ||
dev->type == ARPHRD_SIT ||
dev->type == ARPHRD_NONE) {
ndev->cnf.use_tempaddr = -1;
- } else {
- in6_dev_hold(ndev);
- ipv6_regen_rndid((unsigned long) ndev);
- }
+ } else
+ ipv6_regen_rndid(ndev);
ndev->token = in6addr_any;
@@ -447,7 +441,6 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
err = addrconf_sysctl_register(ndev);
if (err) {
ipv6_mc_destroy_dev(ndev);
- del_timer(&ndev->regen_timer);
snmp6_unregister_dev(ndev);
goto err_release;
}
@@ -1222,7 +1215,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
}
in6_ifa_hold(ifp);
memcpy(addr.s6_addr, ifp->addr.s6_addr, 8);
- __ipv6_try_regen_rndid(idev, tmpaddr);
+ ipv6_try_regen_rndid(idev, tmpaddr);
memcpy(&addr.s6_addr[8], idev->rndid, 8);
age = (now - ifp->tstamp) / HZ;
tmp_valid_lft = min_t(__u32,
@@ -2150,7 +2143,7 @@ static int ipv6_inherit_eui64(u8 *eui, struct inet6_dev *idev)
}
/* (re)generation of randomized interface identifier (RFC 3041 3.2, 3.5) */
-static void __ipv6_regen_rndid(struct inet6_dev *idev)
+static void ipv6_regen_rndid(struct inet6_dev *idev)
{
regen:
get_random_bytes(idev->rndid, sizeof(idev->rndid));
@@ -2179,43 +2172,10 @@ static void __ipv6_regen_rndid(struct inet6_dev *idev)
}
}
-static void ipv6_regen_rndid(unsigned long data)
-{
- struct inet6_dev *idev = (struct inet6_dev *) data;
- unsigned long expires;
-
- rcu_read_lock_bh();
- write_lock_bh(&idev->lock);
-
- if (idev->dead)
- goto out;
-
- __ipv6_regen_rndid(idev);
-
- expires = jiffies +
- idev->cnf.temp_prefered_lft * HZ -
- idev->cnf.regen_max_retry * idev->cnf.dad_transmits *
- NEIGH_VAR(idev->nd_parms, RETRANS_TIME) -
- idev->cnf.max_desync_factor * HZ;
- if (time_before(expires, jiffies)) {
- pr_warn("%s: too short regeneration interval; timer disabled for %s\n",
- __func__, idev->dev->name);
- goto out;
- }
-
- if (!mod_timer(&idev->regen_timer, expires))
- in6_dev_hold(idev);
-
-out:
- write_unlock_bh(&idev->lock);
- rcu_read_unlock_bh();
- in6_dev_put(idev);
-}
-
-static void __ipv6_try_regen_rndid(struct inet6_dev *idev, struct in6_addr *tmpaddr)
+static void ipv6_try_regen_rndid(struct inet6_dev *idev, struct in6_addr *tmpaddr)
{
if (tmpaddr && memcmp(idev->rndid, &tmpaddr->s6_addr[8], 8) == 0)
- __ipv6_regen_rndid(idev);
+ ipv6_regen_rndid(idev);
}
/*
@@ -3594,9 +3554,6 @@ static int addrconf_ifdown(struct net_device *dev, int how)
if (!how)
idev->if_flags &= ~(IF_RS_SENT|IF_RA_RCVD|IF_READY);
- if (how && del_timer(&idev->regen_timer))
- in6_dev_put(idev);
-
/* Step 3: clear tempaddr list */
while (!list_empty(&idev->tempaddr_list)) {
ifa = list_first_entry(&idev->tempaddr_list,
--
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ
^ permalink raw reply related
* [PATCH 2/2] IPv6: fix DESYNC_FACTOR
From: Jiri Bohac @ 2016-10-13 16:52 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy
Cc: netdev
The IPv6 temporary address generation uses a variable called DESYNC_FACTOR
to prevent hosts updating the addresses at the same time. Quoting RFC 4941:
... The value DESYNC_FACTOR is a random value (different for each
client) that ensures that clients don't synchronize with each other and
generate new addresses at exactly the same time ...
DESYNC_FACTOR is defined as:
DESYNC_FACTOR -- A random value within the range 0 - MAX_DESYNC_FACTOR.
It is computed once at system start (rather than each time it is used)
and must never be greater than (TEMP_VALID_LIFETIME - REGEN_ADVANCE).
First, I believe the RFC has a typo in it and meant to say: "and must
never be greater than (TEMP_PREFERRED_LIFETIME - REGEN_ADVANCE)"
The reason is that at various places in the RFC, DESYNC_FACTOR is used in
a calculation like (TEMP_PREFERRED_LIFETIME - DESYNC_FACTOR) or
(TEMP_PREFERRED_LIFETIME - REGEN_ADVANCE - DESYNC_FACTOR). It needs to be
smaller than (TEMP_PREFERRED_LIFETIME - REGEN_ADVANCE) for the result of
these calculations to be larger than zero. It's never used in a
calculation together with TEMP_VALID_LIFETIME.
I already submitted an errata to the rfc-editor:
https://www.rfc-editor.org/errata_search.php?rfc=4941
The Linux implementation of DESYNC_FACTOR is very wrong:
max_desync_factor is used in places DESYNC_FACTOR should be used.
max_desync_factor is initialized to the RFC-recommended value for
MAX_DESYNC_FACTOR (600) but the whole point is to get a _random_ value.
And nothing ensures that the value used is not greater than
(TEMP_PREFERRED_LIFETIME - REGEN_ADVANCE), which leads to underflows. The
effect can easily be observed when setting the temp_prefered_lft sysctl
e.g. to 60. The preferred lifetime of the temporary addresses will be
bogus.
TEMP_PREFERRED_LIFETIME and REGEN_ADVANCE are not constants and can be
influenced by these three sysctls: regen_max_retry, dad_transmits and
temp_prefered_lft. Thus, the upper bound for desync_factor needs to be
re-calculated each time a new address is generated and if desync_factor is
larger than the new upper bound, a new random value needs to be
re-generated.
And since we already have max_desync_factor configurable per interface, we
also need to calculate and store desync_factor per interface.
Signed-off-by: Jiri Bohac <jbohac@suse.cz>
---
include/net/if_inet6.h | 1 +
net/ipv6/addrconf.c | 39 +++++++++++++++++++++++++++++++--------
2 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index ae70735..b0576cb 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -190,6 +190,7 @@ struct inet6_dev {
__u32 if_flags;
int dead;
+ u32 desync_factor;
u8 rndid[8];
struct list_head tempaddr_list;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 63fc857..eca15f3 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -422,6 +422,7 @@ static struct inet6_dev *ipv6_add_dev(struct net_device *dev)
#endif
INIT_LIST_HEAD(&ndev->tempaddr_list);
+ ndev->desync_factor = U32_MAX;
if ((dev->flags&IFF_LOOPBACK) ||
dev->type == ARPHRD_TUNNEL ||
dev->type == ARPHRD_TUNNEL6 ||
@@ -1183,6 +1184,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
int ret = 0;
u32 addr_flags;
unsigned long now = jiffies;
+ long max_desync_factor;
write_lock_bh(&idev->lock);
if (ift) {
@@ -1218,20 +1220,41 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
ipv6_try_regen_rndid(idev, tmpaddr);
memcpy(&addr.s6_addr[8], idev->rndid, 8);
age = (now - ifp->tstamp) / HZ;
+
+ regen_advance = idev->cnf.regen_max_retry *
+ idev->cnf.dad_transmits *
+ NEIGH_VAR(idev->nd_parms, RETRANS_TIME) / HZ;
+
+ /* recalculate max_desync_factor each time and update
+ * idev->desync_factor if it's larger
+ */
+ max_desync_factor = min_t(__u32,
+ idev->cnf.max_desync_factor,
+ idev->cnf.temp_prefered_lft - regen_advance);
+
+ if (unlikely(idev->desync_factor > max_desync_factor)) {
+ if (max_desync_factor > 0) {
+ get_random_bytes(&idev->desync_factor,
+ sizeof(idev->desync_factor));
+ idev->desync_factor %= max_desync_factor;
+ } else {
+ idev->desync_factor = 0;
+ }
+ }
+
tmp_valid_lft = min_t(__u32,
ifp->valid_lft,
idev->cnf.temp_valid_lft + age);
- tmp_prefered_lft = min_t(__u32,
- ifp->prefered_lft,
- idev->cnf.temp_prefered_lft + age -
- idev->cnf.max_desync_factor);
+ tmp_prefered_lft = idev->cnf.temp_prefered_lft + age -
+ idev->desync_factor;
+ /* guard against underflow in case of concurrent updates to cnf */
+ if (unlikely(tmp_prefered_lft < 0))
+ tmp_prefered_lft = 0;
+ tmp_prefered_lft = min_t(__u32, ifp->prefered_lft, tmp_prefered_lft);
tmp_plen = ifp->prefix_len;
tmp_tstamp = ifp->tstamp;
spin_unlock_bh(&ifp->lock);
- regen_advance = idev->cnf.regen_max_retry *
- idev->cnf.dad_transmits *
- NEIGH_VAR(idev->nd_parms, RETRANS_TIME) / HZ;
write_unlock_bh(&idev->lock);
/* A temporary address is created only if this calculated Preferred
@@ -2316,7 +2339,7 @@ static void manage_tempaddrs(struct inet6_dev *idev,
max_valid = 0;
max_prefered = idev->cnf.temp_prefered_lft -
- idev->cnf.max_desync_factor - age;
+ idev->desync_factor - age;
if (max_prefered < 0)
max_prefered = 0;
--
2.10.0
--
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox