* Re: [Patch net] ipv4: restore rt->fi for reference counting
From: Eric Dumazet @ 2017-05-09 0:01 UTC (permalink / raw)
To: David Miller; +Cc: xiyou.wangcong, netdev, andreyknvl, edumazet
In-Reply-To: <20170508.143557.105629611489969352.davem@davemloft.net>
On Mon, 2017-05-08 at 14:35 -0400, David Miller wrote:
> From: Cong Wang <xiyou.wangcong@gmail.com>
> Date: Thu, 4 May 2017 14:54:17 -0700
>
> > IPv4 dst could use fi->fib_metrics to store metrics but fib_info
> > itself is refcnt'ed, so without taking a refcnt fi and
> > fi->fib_metrics could be freed while dst metrics still points to
> > it. This triggers use-after-free as reported by Andrey twice.
> >
> > This patch reverts commit 2860583fe840 ("ipv4: Kill rt->fi") to
> > restore this reference counting. It is a quick fix for -net and
> > -stable, for -net-next, as Eric suggested, we can consider doing
> > reference counting for metrics itself instead of relying on fib_info.
> >
> > IPv6 is very different, it copies or steals the metrics from mx6_config
> > in fib6_commit_metrics() so probably doesn't need a refcnt.
> >
> > Decnet has already done the refcnt'ing, see dn_fib_semantic_match().
> >
> > Fixes: 2860583fe840 ("ipv4: Kill rt->fi")
> > Reported-by: Andrey Konovalov <andreyknvl@google.com>
> > Tested-by: Andrey Konovalov <andreyknvl@google.com>
> > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>
> Applied and queued up for -stable, thanks.
Although I now have on latest net tree these messages when I reboot my
test machine.
[ 224.085873] unregister_netdevice: waiting for eth0 to become free. Usage count = 43
[ 234.106018] unregister_netdevice: waiting for eth0 to become free. Usage count = 43
[ 244.366187] unregister_netdevice: waiting for eth0 to become free. Usage count = 43
[ 254.626325] unregister_netdevice: waiting for eth0 to become free. Usage count = 43
[ 264.706472] unregister_netdevice: waiting for eth0 to become free. Usage count = 43
[ 274.736619] unregister_netdevice: waiting for eth0 to become free. Usage count = 43
[ 284.766766] unregister_netdevice: waiting for eth0 to become free. Usage count = 43
^ permalink raw reply
* RE: [PATCH] drivers: net: wimax: i2400m: i2400m-usb: Use time_after for time comparison
From: Karim Eshapa @ 2017-05-09 0:06 UTC (permalink / raw)
To: inaky.perez-gonzalez; +Cc: linux-wimax, netdev, linux-kernel, Karim Eshapa
In-Reply-To: <1494262682-5401-1-git-send-email-karim.eshapa@gmail.com>
Use time_after() for time comparison with the new fix.
Signed-off-by: Karim Eshapa <karim.eshapa@gmail.com>
---
drivers/net/wimax/i2400m/i2400m-usb.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wimax/i2400m/i2400m-usb.h b/drivers/net/wimax/i2400m/i2400m-usb.h
index 649ecad..eff4f464 100644
--- a/drivers/net/wimax/i2400m/i2400m-usb.h
+++ b/drivers/net/wimax/i2400m/i2400m-usb.h
@@ -131,7 +131,7 @@ static inline int edc_inc(struct edc *edc, u16 max_err, u16 timeframe)
unsigned long now;
now = jiffies;
- if (now - edc->timestart > timeframe) {
+ if (time_after(now, edc->timestart + timeframe)) {
edc->errorcount = 1;
edc->timestart = now;
} else if (++edc->errorcount > max_err) {
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] net: dsa: loop: Check for memory allocation failure
From: Florian Fainelli @ 2017-05-09 0:35 UTC (permalink / raw)
To: Julia Lawall, Joe Perches
Cc: David Laight, 'Christophe JAILLET', andrew@lunn.ch,
vivien.didelot@savoirfairelinux.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
In-Reply-To: <alpine.DEB.2.20.1705090745190.3309@hadrien>
On 05/08/2017 04:46 PM, Julia Lawall wrote:
>
>
> On Mon, 8 May 2017, Joe Perches wrote:
>
>> On Mon, 2017-05-08 at 20:32 +0800, Julia Lawall wrote:
>>>
>>> On Mon, 8 May 2017, David Laight wrote:
>>>
>>>> From: Christophe JAILLET
>>>>> Sent: 06 May 2017 06:30
>>>>> If 'devm_kzalloc' fails, a NULL pointer will be dereferenced.
>>>>> Return -ENOMEM instead, as done for some other memory allocation just a
>>>>> few lines above.
>>>>
>>>> ...
>>>>> --- a/drivers/net/dsa/dsa_loop.c
>>>>> +++ b/drivers/net/dsa/dsa_loop.c
>>>>> @@ -256,6 +256,9 @@ static int dsa_loop_drv_probe(struct mdio_device *mdiodev)
>>>>> return -ENOMEM;
>>>>>
>>>>> ps = devm_kzalloc(&mdiodev->dev, sizeof(*ps), GFP_KERNEL);
>>>>> + if (!ps)
>>>>> + return -ENOMEM;
>>>>> +
>>>>> ps->netdev = dev_get_by_name(&init_net, pdata->netdev);
>>>>> if (!ps->netdev)
>>>>> return -EPROBE_DEFER;
>>>>
>>>> On the face if it this code leaks like a sieve.
>>>
>>> I don't think so. The allocations (dsa_switch_alloc and devm_kzalloc) use
>>> devm functions.
>>
>> It's at least wasteful.
>>
>> Each time -EPROBE_DEFER occurs, another set of calls to
>> dsa_switch_alloc and dev_kzalloc also occurs.
>>
>> Perhaps it'd be better to do:
>>
>> if (ps->netdev) {
>> devm_kfree(&devmdev->dev, ps);
>> devm_kfree(&mdiodev->dev, ds);
>> return -EPROBE_DEFER;
>> }
>
> Is EPROBE_DEFER handled differently than other kinds of errors?
In the core device driver model, yes, EPROBE_DEFER is treated
differently than other errors because it puts the driver on a retry queue.
EPROBE_DEFER is already a slow and exceptional path, and this is a
mock-up driver, so I am not sure what value there is in trying to
balance devm_kzalloc() with corresponding devm_kfree()...
--
Florian
^ permalink raw reply
* Re: [PATCH] net: dsa: loop: Check for memory allocation failure
From: Julia Lawall @ 2017-05-09 0:39 UTC (permalink / raw)
To: Florian Fainelli
Cc: Joe Perches, David Laight, 'Christophe JAILLET',
andrew@lunn.ch, vivien.didelot@savoirfairelinux.com,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel-janitors@vger.kernel.org
In-Reply-To: <f9058de7-ef9e-f6eb-751d-72ffdce512bb@gmail.com>
On Mon, 8 May 2017, Florian Fainelli wrote:
> On 05/08/2017 04:46 PM, Julia Lawall wrote:
> >
> >
> > On Mon, 8 May 2017, Joe Perches wrote:
> >
> >> On Mon, 2017-05-08 at 20:32 +0800, Julia Lawall wrote:
> >>>
> >>> On Mon, 8 May 2017, David Laight wrote:
> >>>
> >>>> From: Christophe JAILLET
> >>>>> Sent: 06 May 2017 06:30
> >>>>> If 'devm_kzalloc' fails, a NULL pointer will be dereferenced.
> >>>>> Return -ENOMEM instead, as done for some other memory allocation just a
> >>>>> few lines above.
> >>>>
> >>>> ...
> >>>>> --- a/drivers/net/dsa/dsa_loop.c
> >>>>> +++ b/drivers/net/dsa/dsa_loop.c
> >>>>> @@ -256,6 +256,9 @@ static int dsa_loop_drv_probe(struct mdio_device *mdiodev)
> >>>>> return -ENOMEM;
> >>>>>
> >>>>> ps = devm_kzalloc(&mdiodev->dev, sizeof(*ps), GFP_KERNEL);
> >>>>> + if (!ps)
> >>>>> + return -ENOMEM;
> >>>>> +
> >>>>> ps->netdev = dev_get_by_name(&init_net, pdata->netdev);
> >>>>> if (!ps->netdev)
> >>>>> return -EPROBE_DEFER;
> >>>>
> >>>> On the face if it this code leaks like a sieve.
> >>>
> >>> I don't think so. The allocations (dsa_switch_alloc and devm_kzalloc) use
> >>> devm functions.
> >>
> >> It's at least wasteful.
> >>
> >> Each time -EPROBE_DEFER occurs, another set of calls to
> >> dsa_switch_alloc and dev_kzalloc also occurs.
> >>
> >> Perhaps it'd be better to do:
> >>
> >> if (ps->netdev) {
> >> devm_kfree(&devmdev->dev, ps);
> >> devm_kfree(&mdiodev->dev, ds);
> >> return -EPROBE_DEFER;
> >> }
> >
> > Is EPROBE_DEFER handled differently than other kinds of errors?
>
> In the core device driver model, yes, EPROBE_DEFER is treated
> differently than other errors because it puts the driver on a retry queue.
>
> EPROBE_DEFER is already a slow and exceptional path, and this is a
> mock-up driver, so I am not sure what value there is in trying to
> balance devm_kzalloc() with corresponding devm_kfree()...
OK, thanks for the explanation.
julia
^ permalink raw reply
* Re: [PATCH 1/2] PCI: Add new PCIe Fabric End Node flag, PCI_DEV_FLAGS_NO_RELAXED_ORDERING
From: Casey Leedom @ 2017-05-09 0:48 UTC (permalink / raw)
To: Alexander Duyck, Ding Tianhong
Cc: Raj, Ashok, Bjorn Helgaas, Michael Werner, Ganesh GR, Arjun V.,
Asit K Mallick, Patrick J Cramer, Suravee Suthikulpanit, Bob Shaw,
h, Mark Rutland, Amir Ancel, Gabriele Paoloni, Catalin Marinas,
Will Deacon, LinuxArm, David Laight, Jeff Kirsher, Netdev,
"Robin Murphy" <robin.mu
In-Reply-To: <CAKgT0Ufg6uebqj79e1FOHoPpF-GxPYMkp353r3Ok_=nM8wF6qA@mail.gmail.com>
| From: Alexander Duyck <alexander.duyck@gmail.com>
| Date: Saturday, May 6, 2017 11:07 AM
|
| | From: Ding Tianhong <dingtianhong@huawei.com>
| | Date: Fri, May 5, 2017 at 8:08 PM
| |
| | According the suggestion, I could only think of this code:
| | ..
|
| This is a bit simplistic but it is a start.
Yes, something tells me that this is going to be more complicated than any
of us like ...
| The other bit I was getting at is that we need to update the core PCIe
| code so that when we configure devices and the root complex reports no
| support for relaxed ordering it should be clearing the relaxed
| ordering bits in the PCIe configuration registers on the upstream
| facing devices.
Of course, this can be written to by the Driver at any time ... and is in
the case of the cxgb4 Driver ...
After a lot of rummaging around, it does look like KVM prohibits writes to
the PCIe Capability Device Control register in drivers/xen/xen-pciback/
conf_space_capability.c and conf_space.c simply because writes aren't
allowed unless "permissive" is set. So it ~looks~ like a driver running in
a Virtual Machine can't turn Enable Relaxed Ordering back on ...
| The last bit we need in all this is a way to allow for setups where
| peer-to-peer wants to perform relaxed ordering but for writes to the
| host we have to not use relaxed ordering. For that we need to enable a
| special case and that isn't handled right now in any of the solutions
| we have coded up so far.
Yes, we do need this.
| From: Alexander Duyck <alexander.duyck@gmail.com>
| Date: Saturday, May 8, 2017 08:22 AM
|
| The problem is we need to have something that can be communicated
| through a VM. Your change doesn't work in that regard. That was why I
| suggested just updating the code so that we when we initialized PCIe
| devices what we do is either set or clear the relaxed ordering bit in
| the PCIe device control register. That way when we direct assign an
| interface it could know just based on the bits int the PCIe
| configuration if it could use relaxed ordering or not.
|
| At that point the driver code itself becomes very simple since you
| could just enable the relaxed ordering by default in the igb/ixgbe
| driver and if the bit is set or cleared in the PCIe configuration then
| we are either sending with relaxed ordering requests or not and don't
| have to try and locate the root complex.
|
| So from the sound of it Casey has a special use case where he doesn't
| want to send relaxed ordering frames to the root complex, but instead
| would like to send them to another PCIe device. To do that he needs to
| have a way to enable the relaxed ordering bit in the PCIe
| configuration but then not send any to the root complex. Odds are that
| is something he might be able to just implement in the driver, but is
| something that may become a more general case in the future. I don't
| see our change here impacting it as long as we keep the solution
| generic and mostly confined to when we instantiate the devices as the
| driver could likely make the decision to change the behavior later.
It's not just me. Intel has said that while RO directed at the Root
Complex Host Coherent Memory has a performance bug (not Data Corruption),
it's a performance win for Peer-to-Peer writes to MMIO Space. (I'll be very
interested in hearing what the bug is if we get that much detail. The very
same TLPs directed to the Root Complex Port without Relaxed Ordering set get
good performance. So this is essentially a bug in the hardware that was
~trying~ to implement a performance win.)
Meanwhile, I currently only know of a single PCIe End Point which causes
catastrophic results: the AMD A1100 ARM SoC ("SEATTLE"). And it's not even
clear that product is even alive anymore since I haven't been able to get
any responses from them for several months.
What I'm saying is: let's try to architect a solution which doesn't throw
the baby out with the bath water ...
I think that if a Device's Root Complex Port has problems with Relaxed
Ordering, it ~probably~ makes sense to turn off the PCIe Capability Device
Control[Enable Relaxed Ordering] when we assign a device to a Virtual
Machine since the Device Driver can no longer query the Relaxed Ordering
Support of the Root Complex Port. The only down side of this would be if we
assigned two Peers to a VM in an application which wanted to do Peer-to-Peer
transfers. But that seems like a hard application to support in any case
since the PCI Bus:Slot.Function IDs for assigned Devices within a VM don't
match the actual values.
For Devices running in the base OS/Hypervisor, their Drivers can query the
Relaxed Ordering Support for the Root Complex Port or a Peer Device. So a
simple flag within the (struct pci_dev *)->dev_flags would serve for that
along with a per-Architecture/Platform mechanism for setting it ...
Casey
^ permalink raw reply
* Re: [PATCH RFC net-next 3/6] net: Introduce IFF_LWT_NETDEV flag
From: David Ahern @ 2017-05-09 0:57 UTC (permalink / raw)
To: David Miller, johannes; +Cc: netdev, roopa, f.fainelli, nicolas.dichtel
In-Reply-To: <20170508.161130.1538627061428691500.davem@davemloft.net>
On 5/8/17 1:11 PM, David Miller wrote:
> From: Johannes Berg <johannes@sipsolutions.net>
> Date: Mon, 08 May 2017 10:55:12 +0200
>
>>
>>> +static inline bool netif_is_lwd(struct net_device *dev)
>>> +{
>>> + return !!(dev->priv_flags & IFF_LWT_NETDEV);
>>> +}
>>
>> Am I the only one who thinks that this "LWT_NETDEV" vs "LWD" is a bit
>> confusing?
>
> Agreed, my old eyes can't discern them at a distance :-)
>
perhaps it is the tiny font your old eyes are having trouble with :-)
I am fine with Johannes' suggestion -- just spell it out:
netif_is_lwt_netdev
where lwt = LightWeighT
^ permalink raw reply
* Re: [Patch net] ipv4: restore rt->fi for reference counting
From: David Miller @ 2017-05-09 1:22 UTC (permalink / raw)
To: eric.dumazet; +Cc: xiyou.wangcong, netdev, andreyknvl, edumazet
In-Reply-To: <1494288080.7796.59.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 08 May 2017 17:01:20 -0700
> On Mon, 2017-05-08 at 14:35 -0400, David Miller wrote:
>> From: Cong Wang <xiyou.wangcong@gmail.com>
>> Date: Thu, 4 May 2017 14:54:17 -0700
>>
>> > IPv4 dst could use fi->fib_metrics to store metrics but fib_info
>> > itself is refcnt'ed, so without taking a refcnt fi and
>> > fi->fib_metrics could be freed while dst metrics still points to
>> > it. This triggers use-after-free as reported by Andrey twice.
>> >
>> > This patch reverts commit 2860583fe840 ("ipv4: Kill rt->fi") to
>> > restore this reference counting. It is a quick fix for -net and
>> > -stable, for -net-next, as Eric suggested, we can consider doing
>> > reference counting for metrics itself instead of relying on fib_info.
>> >
>> > IPv6 is very different, it copies or steals the metrics from mx6_config
>> > in fib6_commit_metrics() so probably doesn't need a refcnt.
>> >
>> > Decnet has already done the refcnt'ing, see dn_fib_semantic_match().
>> >
>> > Fixes: 2860583fe840 ("ipv4: Kill rt->fi")
>> > Reported-by: Andrey Konovalov <andreyknvl@google.com>
>> > Tested-by: Andrey Konovalov <andreyknvl@google.com>
>> > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>>
>> Applied and queued up for -stable, thanks.
>
> Although I now have on latest net tree these messages when I reboot my
> test machine.
>
> [ 224.085873] unregister_netdevice: waiting for eth0 to become free. Usage count = 43
Strange, the refcounting looks quite OK in the patch you're quoting.
I looked over it a few times and cannot figure out a possible cause
there.
I am assuming you are quite confident it is this change?
^ permalink raw reply
* iproute2 ss outputs duplicate tcp sockets info on kernel 3.10.105
From: Li Er @ 2017-05-09 1:56 UTC (permalink / raw)
To: netdev
i'm using v4.11.0 release of iproute2 and kernel 3.10.105, simply
running
$ ss
Netid State Recv-Q Send-Q Local Address:Port Peer Address:Port
tcp CLOSE-WAIT 434 0 10.0.0.1:47931 65.49.18.136:https
tcp CLOSE-WAIT 432 0 10.0.0.1:47932 65.49.18.136:https
tcp CLOSE-WAIT 434 0 10.0.0.1:47931 65.49.18.136:https
tcp CLOSE-WAIT 432 0 10.0.0.1:47932 65.49.18.136:https
as you can see, there's one duplicate entry of each tcp socket,
however, if i explicitly specify tcp socket by adding the -t
switch,
$ ss -t
State Recv-Q Send-Q Local Address:Port Peer Address:Port
CLOSE-WAIT 434 0 10.0.0.1:47931 65.49.18.136:https
CLOSE-WAIT 432 0 10.0.0.1:47932 65.49.18.136:https
the duplication is gone.
this problem also occurs on git master, but not on iproute2
v4.3.0, so i did a git bisect and found out the commit which
caused this is 9f66764e308e9c645b3fb2d1cfbb7fb207eb5de1, and by
revert this commit on git master, i.e. removing
err = rtnl_dump_done(rth, h);
if (err < 0)
return -1;
these 3 lines of code of lib/libnetlink.c, the problem is gone.
since i'm not familiar with the source code, i doubt this is the
right way to solve the problem, what's your suggestions? thanks.
^ permalink raw reply
* Re: [Patch net] ipv4: restore rt->fi for reference counting
From: Eric Dumazet @ 2017-05-09 2:18 UTC (permalink / raw)
To: David Miller; +Cc: xiyou.wangcong, netdev, andreyknvl, edumazet
In-Reply-To: <20170508.212211.1291611254198273979.davem@davemloft.net>
On Mon, 2017-05-08 at 21:22 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon, 08 May 2017 17:01:20 -0700
>
> > On Mon, 2017-05-08 at 14:35 -0400, David Miller wrote:
> >> From: Cong Wang <xiyou.wangcong@gmail.com>
> >> Date: Thu, 4 May 2017 14:54:17 -0700
> >>
> >> > IPv4 dst could use fi->fib_metrics to store metrics but fib_info
> >> > itself is refcnt'ed, so without taking a refcnt fi and
> >> > fi->fib_metrics could be freed while dst metrics still points to
> >> > it. This triggers use-after-free as reported by Andrey twice.
> >> >
> >> > This patch reverts commit 2860583fe840 ("ipv4: Kill rt->fi") to
> >> > restore this reference counting. It is a quick fix for -net and
> >> > -stable, for -net-next, as Eric suggested, we can consider doing
> >> > reference counting for metrics itself instead of relying on fib_info.
> >> >
> >> > IPv6 is very different, it copies or steals the metrics from mx6_config
> >> > in fib6_commit_metrics() so probably doesn't need a refcnt.
> >> >
> >> > Decnet has already done the refcnt'ing, see dn_fib_semantic_match().
> >> >
> >> > Fixes: 2860583fe840 ("ipv4: Kill rt->fi")
> >> > Reported-by: Andrey Konovalov <andreyknvl@google.com>
> >> > Tested-by: Andrey Konovalov <andreyknvl@google.com>
> >> > Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> >>
> >> Applied and queued up for -stable, thanks.
> >
> > Although I now have on latest net tree these messages when I reboot my
> > test machine.
> >
> > [ 224.085873] unregister_netdevice: waiting for eth0 to become free. Usage count = 43
>
> Strange, the refcounting looks quite OK in the patch you're quoting.
> I looked over it a few times and cannot figure out a possible cause
> there.
>
> I am assuming you are quite confident it is this change?
At least, reverting the patch resolves the issue for me.
Keeping fib (and their reference to netdev) is apparently too much,
we probably need to implement a refcount on the metrics themselves,
being stand alone objects.
^ permalink raw reply
* Re: [Patch net] ipv4: restore rt->fi for reference counting
From: David Miller @ 2017-05-09 2:35 UTC (permalink / raw)
To: eric.dumazet; +Cc: xiyou.wangcong, netdev, andreyknvl, edumazet
In-Reply-To: <1494296302.7796.61.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 08 May 2017 19:18:22 -0700
> Keeping fib (and their reference to netdev) is apparently too much,
> we probably need to implement a refcount on the metrics themselves,
> being stand alone objects.
I think I see the problem, when we flush the dst cache on device
unregister, we point the dst at loopback but do not flush metrics.
I'm going to revert.
^ permalink raw reply
* Re: [PATCH net] ip6_tunnel: remove unreachable ICMP_REDIRECT code
From: Hangbin Liu @ 2017-05-09 3:25 UTC (permalink / raw)
To: David Miller; +Cc: network dev
In-Reply-To: <20170508.155903.61649135414142419.davem@davemloft.net>
2017-05-09 3:59 GMT+08:00 David Miller <davem@davemloft.net>:
> From: Hangbin Liu <liuhangbin@gmail.com>
> Date: Mon, 8 May 2017 19:11:03 +0800
>
>> After call ip6_tnl_err(), the rel_type will be ether ICMPV6_DEST_UNREACH
>> or ICMPV6_PKT_TOOBIG. We will never reach ICMP_REDIRECT. So remove it.
>>
>> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
>
> Your patches here, all seemingly due to "visual inspection", are starting
> to really, truly, irritate me.
Yes, this is based on the coverity scan result. I'm really sorry to make
you feel uncomfortable and I apologize for this. I trust the result too much
and did not do enough research. I will try my best to make sure the patch
really improve or fix something next time.
Sorry again.
Regards
Hangbin
>
> Half of them are unnecessary, some are completely buggy.
>
> I have to review them and audit them very satrictly as a result, and
> this takes up an unnecessarily large amount of my time.
>
> Therefore, I'm basically going to stop looking at your changes _unless_
> you can show that you specifically tested and exercised all of the code
> paths you are changing.
>
> I am sorry to have to do this, but the value to effort ratio of
> reviewing and integrating your changes is quite poor.
^ permalink raw reply
* [PATCH] net: wireless: ath: ath9k: remove unnecessary code
From: Gustavo A. R. Silva @ 2017-05-09 3:48 UTC (permalink / raw)
To: QCA ath9k Development, Kalle Valo
Cc: linux-wireless, netdev, linux-kernel, Gustavo A. R. Silva
The name of an array used by itself will always return the array's address.
So this test will always evaluate as true.
Addresses-Coverity-ID: 1364903
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
drivers/net/wireless/ath/ath9k/eeprom.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath9k/eeprom.c b/drivers/net/wireless/ath/ath9k/eeprom.c
index fb80ec8..5c3bc28 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom.c
@@ -143,7 +143,7 @@ bool ath9k_hw_nvram_read(struct ath_hw *ah, u32 off, u16 *data)
if (ah->eeprom_blob)
ret = ath9k_hw_nvram_read_firmware(ah->eeprom_blob, off, data);
- else if (pdata && !pdata->use_eeprom && pdata->eeprom_data)
+ else if (pdata && !pdata->use_eeprom)
ret = ath9k_hw_nvram_read_pdata(pdata, off, data);
else
ret = common->bus_ops->eeprom_read(common, off, data);
--
2.5.0
^ permalink raw reply related
* [PATCH] net: wireless: ath: ath10k: remove unnecessary code
From: Gustavo A. R. Silva @ 2017-05-09 4:21 UTC (permalink / raw)
To: Kalle Valo
Cc: ath10k, linux-wireless, netdev, linux-kernel, Gustavo A. R. Silva
The name of an array used by itself will always return the array's address.
So these tests will always evaluate as false and therefore the _return_
will never be executed.
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
drivers/net/wireless/ath/ath10k/wmi.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 2f1743e..135cf83 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -5933,15 +5933,6 @@ static struct sk_buff *ath10k_wmi_10_4_op_gen_init(struct ath10k *ar)
int ath10k_wmi_start_scan_verify(const struct wmi_start_scan_arg *arg)
{
- if (arg->ie_len && !arg->ie)
- return -EINVAL;
- if (arg->n_channels && !arg->channels)
- return -EINVAL;
- if (arg->n_ssids && !arg->ssids)
- return -EINVAL;
- if (arg->n_bssids && !arg->bssids)
- return -EINVAL;
-
if (arg->ie_len > WLAN_SCAN_PARAMS_MAX_IE_LEN)
return -EINVAL;
if (arg->n_channels > ARRAY_SIZE(arg->channels))
--
2.5.0
^ permalink raw reply related
* [PATCH] wcn36xx: Close SMD channel on device removal
From: Bjorn Andersson @ 2017-05-09 4:36 UTC (permalink / raw)
To: Eugene Krasnikov, Kalle Valo, Eyal Ilsar
Cc: wcn36xx, linux-wireless, netdev, linux-kernel, linux-arm-msm
The SMD channel is not the primary WCNSS channel and must explicitly be
closed as the device is removed, or the channel will already by open on
a subsequent probe call in e.g. the case of reloading the kernel module.
This issue was introduced because I simplified the underlying SMD
implementation while the SMD adaptions of the driver sat on the mailing
list, but missed to update these patches. The patch does however only
apply back to the transition to rpmsg, hence the limited Fixes.
Fixes: 5052de8deff5 ("soc: qcom: smd: Transition client drivers from smd to rpmsg")
Reported-by: Eyal Ilsar <c_eilsar@qti.qualcomm.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
drivers/net/wireless/ath/wcn36xx/main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index d5e993dc9b23..517a315e259b 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -1271,6 +1271,8 @@ static int wcn36xx_remove(struct platform_device *pdev)
qcom_smem_state_put(wcn->tx_enable_state);
qcom_smem_state_put(wcn->tx_rings_empty_state);
+ rpmsg_destroy_ept(wcn->smd_channel);
+
iounmap(wcn->dxe_base);
iounmap(wcn->ccu_base);
--
2.12.0
^ permalink raw reply related
* Re: [PATCH 0/4] TI Bluetooth serdev support
From: Baruch Siach @ 2017-05-09 4:48 UTC (permalink / raw)
To: Rob Herring
Cc: Adam Ford, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Johan Hedberg,
Gustavo Padovan, Marcel Holtmann, Sebastian Reichel, Wei Xu,
open list:BLUETOOTH DRIVERS, Eyal Reizer, netdev, Satish Patel,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <CAL_JsqKBAPUPRtn+pEtz8B8pCrA=45RkEq6X_0i_HYoWsVmtyw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Rob,
On Mon, May 08, 2017 at 04:12:16PM -0500, Rob Herring wrote:
> On Fri, May 5, 2017 at 9:51 AM, Adam Ford <aford173-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Sun, Apr 30, 2017 at 11:04 AM, Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >> On Sun, Apr 30, 2017 at 10:14:20AM -0500, Adam Ford wrote:
> >>> On Wed, Apr 5, 2017 at 1:30 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> >>> > This series adds serdev support to the HCI LL protocol used on TI BT
> >>> > modules and enables support on HiKey board with with the WL1835 module.
> >>> > With this the custom TI UIM daemon and btattach are no longer needed.
> >>>
> >>> Without UIM daemon, what instruction do you use to load the BT firmware?
> >>>
> >>> I was thinking 'hciattach' but I was having trouble. I was hoping you
> >>> might have some insight.
> >>>
> >>> hciattach -t 30 -s 115200 /dev/ttymxc1 texas 3000000 flow Just
> >>> returns a timeout.
> >>>
> >>> I modified my i.MX6 device tree per the binding documentation and
> >>> setup the regulators and enable GPIO pins.
> >>
> >> If you configured everything correctly no userspace interaction is
> >> required. The driver should request the firmware automatically once
> >> you power up the bluetooth device.
> >>
> >> Apart from DT changes make sure, that the following options are
> >> enabled and check dmesg for any hints.
> >>
> >> CONFIG_SERIAL_DEV_BUS
> >> CONFIG_SERIAL_DEV_CTRL_TTYPORT
> >> CONFIG_BT_HCIUART
> >> CONFIG_BT_HCIUART_LL
> >
> > I have enabled those flags, and I have updated my device tree.
> > I am testing this on an OMAP3630 (DM3730) board with a WL1283. I am
> > getting a lot of timeout errors. I tested this against the original
> > implemention I had in pdata-quirks.c using the ti-st driver, uim & and
> > the btwilink driver.
> >
> > I pulled in some of the newer patches to enable the wl1283-st, but I
> > am obviously missing something.
> >
> > I 58.717651] Bluetooth: hci0: Reading TI version information failed
> > (-110)
> > [ 58.724853] Bluetooth: hci0: download firmware failed, retrying...
> > [ 60.957641] Bluetooth: hci0 command 0x1001 tx timeout
> > [ 68.957641] Bluetooth: hci0: Reading TI version information failed
> > (-110)
> > [ 68.964843] Bluetooth: hci0: download firmware failed, retrying...
> > [ 69.132171] Bluetooth: Unknown HCI packet type 06
> > [ 69.138244] Bluetooth: Unknown HCI packet type 0c
> > [ 69.143249] Bluetooth: Unknown HCI packet type 40
> > [ 69.148498] Bluetooth: Unknown HCI packet type 20
> > [ 69.153533] Bluetooth: Data length is too large
> > [ 69.158569] Bluetooth: Unknown HCI packet type a0
> > [ 69.163574] Bluetooth: Unknown HCI packet type 00
> > [ 69.168731] Bluetooth: Unknown HCI packet type 00
> > [ 69.173736] Bluetooth: Unknown HCI packet type 34
> > [ 69.178924] Bluetooth: Unknown HCI packet type 91
> > [ 71.197631] Bluetooth: hci0 command 0x1001 tx timeout
> > [ 79.197662] Bluetooth: hci0: Reading TI version information failed (-110)
>
> There's a bug in serdev_device_write(), so if you have that function
> you need either the fix I sent or the patch to make
> serdev_device_writebuf atomic again. Both are on the linux-serial
> list, but not in any tree yet.
You refer to the patches below, right?
[PATCH] tty: serdev: fix serdev_device_write return value,
http://www.spinics.net/lists/linux-serial/msg26117.html
[PATCH] serdev: Restore serdev_device_write_buf for atomic context,
http://www.spinics.net/lists/linux-serial/msg26113.html
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org - tel: +972.2.679.5364, http://www.tkos.co.il -
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH RFC net-next 3/6] net: Introduce IFF_LWT_NETDEV flag
From: Roopa Prabhu @ 2017-05-09 5:04 UTC (permalink / raw)
To: David Ahern
Cc: David Miller, Johannes Berg, netdev@vger.kernel.org,
Florian Fainelli, Nicolas Dichtel
In-Reply-To: <52621c1c-57c4-efb7-80dc-648a0da7ae7f@gmail.com>
On Mon, May 8, 2017 at 5:57 PM, David Ahern <dsahern@gmail.com> wrote:
> On 5/8/17 1:11 PM, David Miller wrote:
>> From: Johannes Berg <johannes@sipsolutions.net>
>> Date: Mon, 08 May 2017 10:55:12 +0200
>>
>>>
>>>> +static inline bool netif_is_lwd(struct net_device *dev)
>>>> +{
>>>> + return !!(dev->priv_flags & IFF_LWT_NETDEV);
>>>> +}
>>>
>>> Am I the only one who thinks that this "LWT_NETDEV" vs "LWD" is a bit
>>> confusing?
>>
>> Agreed, my old eyes can't discern them at a distance :-)
>>
>
> perhaps it is the tiny font your old eyes are having trouble with :-)
>
> I am fine with Johannes' suggestion -- just spell it out:
> netif_is_lwt_netdev
>
> where lwt = LightWeighT
makes sense...but this does sound like a 'light weight tunnel
netdevice' though.....just cause 'LWT' already expands to 'light
weight tunnel'
^ permalink raw reply
* Re: [PATCH] net: wireless: ath: ath10k: remove unnecessary code
From: Kalle Valo @ 2017-05-09 5:33 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: netdev@vger.kernel.org, linux-wireless@vger.kernel.org,
linux-kernel@vger.kernel.org, ath10k@lists.infradead.org
In-Reply-To: <20170509042159.GA19727@embeddedgus>
"Gustavo A. R. Silva" <garsilva@embeddedor.com> writes:
> The name of an array used by itself will always return the array's address.
> So these tests will always evaluate as false and therefore the _return_
> will never be executed.
>
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
I don't understand the commit log, especially what does "The name of an
array used by itself" mean?
--
Kalle Valo
^ permalink raw reply
* [PATCH] net: fec: select queue depending on VLAN priority
From: Stefan Agner @ 2017-05-09 5:37 UTC (permalink / raw)
To: fugang.duan; +Cc: andrew, festevam, netdev, linux-kernel, Stefan Agner
Since the addition of the multi queue code with commit 59d0f7465644
("net: fec: init multi queue date structure") the queue selection
has been handelt by the default transmit queue selection
implementation which tries to evenly distribute the traffic across
all available queues. This selection presumes that the queues are
using an equal priority, however, the queues 1 and 2 are actually
of higher priority (the classification of the queues is enabled in
fec_enet_enable_ring).
This can lead to net scheduler warnings and continuous TX ring
dumps when exercising the system with iperf.
Use only queue 0 for all common traffic (no VLAN and P802.1p
priority 0 and 1) and route level 2-7 through queue 1 and 2.
Signed-off-by: Fugang Duan <fugang.duan@nxp.com>
Fixes: 59d0f7465644 ("net: fec: init multi queue date structure")
---
This patch solves the issue documented in this thread:
https://www.spinics.net/lists/netdev/msg430679.html
drivers/net/ethernet/freescale/fec_main.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 91a16641e851..72343cbfddc9 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -72,6 +72,7 @@ static void fec_enet_itr_coal_init(struct net_device *ndev);
#define DRIVER_NAME "fec"
#define FEC_ENET_GET_QUQUE(_x) ((_x == 0) ? 1 : ((_x == 1) ? 2 : 0))
+static const u16 fec_enet_vlan_pri_to_queue[8] = {0, 0, 1, 1, 1, 2, 2, 2};
/* Pause frame feild and FIFO threshold */
#define FEC_ENET_FCE (1 << 5)
@@ -3052,10 +3053,28 @@ static int fec_set_features(struct net_device *netdev,
return 0;
}
+u16 fec_enet_select_queue(struct net_device *ndev, struct sk_buff *skb,
+ void *accel_priv, select_queue_fallback_t fallback)
+{
+ struct fec_enet_private *fep = netdev_priv(ndev);
+ u16 vlan_tci, vlan_pcp;
+
+ if (!(fep->quirks & FEC_QUIRK_HAS_AVB))
+ return fallback(ndev, skb);
+
+ /* Assign queue 0 if no VLAN tag present */
+ if (vlan_get_tag(skb, &vlan_tci) < 0)
+ return 0;
+
+ vlan_pcp = (vlan_tci & VLAN_PRIO_MASK) >> VLAN_PRIO_SHIFT;
+ return fec_enet_vlan_pri_to_queue[vlan_pcp];
+}
+
static const struct net_device_ops fec_netdev_ops = {
.ndo_open = fec_enet_open,
.ndo_stop = fec_enet_close,
.ndo_start_xmit = fec_enet_start_xmit,
+ .ndo_select_queue = fec_enet_select_queue,
.ndo_set_rx_mode = set_multicast_list,
.ndo_validate_addr = eth_validate_addr,
.ndo_tx_timeout = fec_timeout,
--
2.12.2
^ permalink raw reply related
* Re: sky2: Use seq_putc() in sky2_debug_show()
From: Stephen Hemminger @ 2017-05-09 5:50 UTC (permalink / raw)
To: SF Markus Elfring
Cc: Lino Sanfilippo, netdev, Mirko Lindner, LKML, kernel-janitors
In-Reply-To: <a85959a8-a357-d8e0-d7e9-617e2052aed6@users.sourceforge.net>
On Mon, 8 May 2017 19:42:46 +0200
SF Markus Elfring <elfring@users.sourceforge.net> wrote:
> > Which issue do you mean? I dont see any issue you fix here.
>
> Are the run time characteristics a bit nicer for the function “seq_putc”
> in comparison to the function “seq_puts” for printing a single line break here?
>
> Regards,
> Markus
I would put this in why bother category. seq_puts is correct and this is only
in diagnostic output useful to developer and disabled on most distro kernels
^ permalink raw reply
* [PATCH] ip: mpls: fix printing of mpls labels
From: David Ahern @ 2017-05-09 6:04 UTC (permalink / raw)
To: stephen, netdev; +Cc: roopa, David Ahern
If the kernel returns more labels than iproute2 expects, none of
the labels are printed and (null) is shown instead:
$ ip -f mpls ro ls
101 as to (null) via inet 172.16.2.2 dev virt12
201 as to 202/203 via inet6 2001:db8:2::2 dev virt12
Remove the use of MPLS_MAX_LABELS and rely on buffer length that is
passed to mpls_ntop. With this change ip can print the label stack
returned by the kernel up to 255 characters (limit is due to size of
buf passed in) which amounts to 31 labels with a separator.
With this change the above is:
$ ip/ip -f mpls ro ls
101 as to 102/103/104/105/106/107/108/109/110 via inet 172.16.2.2 dev virt12
Signed-off-by: David Ahern <dsahern@gmail.com>
---
lib/mpls_ntop.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/lib/mpls_ntop.c b/lib/mpls_ntop.c
index 945d6d5e..5902f503 100644
--- a/lib/mpls_ntop.c
+++ b/lib/mpls_ntop.c
@@ -10,18 +10,20 @@ static const char *mpls_ntop1(const struct mpls_label *addr, char *buf, size_t b
{
size_t destlen = buflen;
char *dest = buf;
- int count;
+ int count = 0;
- for (count = 0; count < MPLS_MAX_LABELS; count++) {
- uint32_t entry = ntohl(addr[count].entry);
+ while (1) {
+ uint32_t entry = ntohl(addr[count++].entry);
uint32_t label = (entry & MPLS_LS_LABEL_MASK) >> MPLS_LS_LABEL_SHIFT;
int len = snprintf(dest, destlen, "%u", label);
+ if (len >= destlen)
+ break;
+
/* Is this the end? */
if (entry & MPLS_LS_S_MASK)
return buf;
-
dest += len;
destlen -= len;
if (destlen) {
--
2.11.0 (Apple Git-81)
^ permalink raw reply related
* Re: [PATCH iproute2] tc: bpf: add ppc64 and sparc64 to list of archs with eBPF support
From: Stephen Hemminger @ 2017-05-09 6:06 UTC (permalink / raw)
To: Alexander Alemayhu; +Cc: netdev, daniel
In-Reply-To: <1494102610-19853-1-git-send-email-alexander@alemayhu.com>
On Sat, 6 May 2017 22:30:10 +0200
Alexander Alemayhu <alexander@alemayhu.com> wrote:
> sparc64 support was added in 7a12b5031c6b (sparc64: Add eBPF JIT., 2017-04-17)[0]
> and ppc64 in 156d0e290e96 (powerpc/ebpf/jit: Implement JIT compiler for extended BPF, 2016-06-22)[1].
>
> [0]: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=7a12b5031c6b
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=156d0e290e96
> Signed-off-by: Alexander Alemayhu <alexander@alemayhu.com>
> ---
> man/man8/tc-bpf.8 | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/man/man8/tc-bpf.8 b/man/man8/tc-bpf.8
> index e371964d06ab..2e9812ede028 100644
> --- a/man/man8/tc-bpf.8
> +++ b/man/man8/tc-bpf.8
> @@ -75,9 +75,9 @@ In Linux, it's generally considered that eBPF is the successor of cBPF.
> The kernel internally transforms cBPF expressions into eBPF expressions and
> executes the latter. Execution of them can be performed in an interpreter
> or at setup time, they can be just-in-time compiled (JIT'ed) to run as
> -native machine code. Currently, x86_64, ARM64 and s390 architectures have
> -eBPF JIT support, whereas PPC, SPARC, ARM and MIPS have cBPF, but did not
> -(yet) switch to eBPF JIT support.
> +native machine code. Currently, x86_64, ARM64, s390, ppc64 and sparc64
> +architectures have eBPF JIT support, whereas PPC, SPARC, ARM and MIPS have
> +cBPF, but did not (yet) switch to eBPF JIT support.
>
> eBPF's instruction set has similar underlying principles as the cBPF
> instruction set, it however is modelled closer to the underlying
Applied thanks.
^ permalink raw reply
* Re: [PATCH] wcn36xx: Close SMD channel on device removal
From: Kalle Valo @ 2017-05-09 6:17 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Eugene Krasnikov, Eyal Ilsar, wcn36xx, linux-wireless, netdev,
linux-kernel, linux-arm-msm
In-Reply-To: <20170509043637.28179-1-bjorn.andersson@linaro.org>
Bjorn Andersson <bjorn.andersson@linaro.org> writes:
> The SMD channel is not the primary WCNSS channel and must explicitly be
> closed as the device is removed, or the channel will already by open on
> a subsequent probe call in e.g. the case of reloading the kernel module.
>
> This issue was introduced because I simplified the underlying SMD
> implementation while the SMD adaptions of the driver sat on the mailing
> list, but missed to update these patches. The patch does however only
> apply back to the transition to rpmsg, hence the limited Fixes.
>
> Fixes: 5052de8deff5 ("soc: qcom: smd: Transition client drivers from smd to rpmsg")
> Reported-by: Eyal Ilsar <c_eilsar@qti.qualcomm.com>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
As this is a regression I'll queue this to 4.12.
But if this is an older bug (didn't quite understand your description
though) should there be a separate patch for stable releases?
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] net/fsl: remove func xgmac_wait_until_free() as duplicate
From: Alexandru Ardelean @ 2017-05-09 6:56 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20170508.153414.1871607886646871422.davem@davemloft.net>
On Mon, May 8, 2017 at 10:34 PM, David Miller <davem@davemloft.net> wrote:
> From: Alexandru Ardelean <ardeleanalex@gmail.com>
> Date: Mon, 8 May 2017 14:31:18 +0300
>
>> Looking at xgmac_wait_until_done() and xgmac_wait_until_free()
>> functions, they seem to have turned out completely identical.
>>
>> Though, judging from the git history it seems they
>> initially weren't.
>>
>> Remove xgmac_wait_until_free() in favor of xgmac_wait_until_done().
>>
>> Signed-off-by: Alexandru Ardelean <ardeleanalex@gmail.com>
>
> This situation got created by the commit below, I wonder if it
> is even correct.
>
> Certainly the original author of this code intended the MDIO
> _data_ rather than the _status_ register to be polled.
>
> Otherwise, why in the world have two functions which are in
> every other regard identical?
[ Removed Shaohui.Xie@nxp.com email from thread ; seems email addr no
longer valid ].
I should have added an RFC tag.
Thing is, we just use this driver, so, I can't offer much input here.
I was debugging [a while ago] some other issue related to this and
stumbled over this.
I haven't seem any obvious issues yet, to suggest one thing or or the
other [regarding polling status or data registers].
I would have liked some input from some NXP/Freescale people on this email list.
Or maybe I could forward this to more people that contribute the NXP eco-system.
If you have any suggestions, please advise and I'll adapt.
Thanks
Alex
>
> commit 26eee0210ad72a29eb4a70b34320bda266f91a0d
> Author: Shaohui Xie <Shaohui.Xie@freescale.com>
> Date: Mon Mar 16 18:55:50 2015 +0800
>
> net/fsl: fix a bug in xgmac_mdio
>
> There is a bug in xgmac_wait_until_done() which mdio_stat should be used
> instead of mdio_data when checking if busy bit is cleared.
>
> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> diff --git a/drivers/net/ethernet/freescale/xgmac_mdio.c b/drivers/net/ethernet/freescale/xgmac_mdio.c
> index 3a83bc2..5f691f2 100644
> --- a/drivers/net/ethernet/freescale/xgmac_mdio.c
> +++ b/drivers/net/ethernet/freescale/xgmac_mdio.c
> @@ -79,7 +79,7 @@ static int xgmac_wait_until_done(struct device *dev,
>
> /* Wait till the MDIO write is complete */
> timeout = TIMEOUT;
> - while ((ioread32be(®s->mdio_data) & MDIO_DATA_BSY) && timeout) {
> + while ((ioread32be(®s->mdio_stat) & MDIO_STAT_BSY) && timeout) {
> cpu_relax();
> timeout--;
> }
^ permalink raw reply
* Re: [RFC iproute2 0/8] RDMA tool
From: Leon Romanovsky @ 2017-05-09 7:03 UTC (permalink / raw)
To: Dennis Dalessandro
Cc: Bart Van Assche, jiri-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
ram.amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org,
sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org,
ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
hch-jcswGhMUV9g@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org,
stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org,
dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
ariela-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
In-Reply-To: <b82f5d3d-f198-3410-af85-85befc14a2ec-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 5043 bytes --]
On Mon, May 08, 2017 at 11:55:25AM -0400, Dennis Dalessandro wrote:
> On 05/04/2017 03:42 PM, Leon Romanovsky wrote:
> > On Thu, May 04, 2017 at 03:26:13PM -0400, Dennis Dalessandro wrote:
> > > On 05/04/2017 02:45 PM, Leon Romanovsky wrote:
> > > > On Thu, May 04, 2017 at 06:30:27PM +0000, Bart Van Assche wrote:
> > > > > On Thu, 2017-05-04 at 21:25 +0300, Leon Romanovsky wrote:
> > > > > > On Thu, May 04, 2017 at 06:10:54PM +0000, Bart Van Assche wrote:
> > > > > > > On Thu, 2017-05-04 at 21:02 +0300, Leon Romanovsky wrote:
> > > > > > > > Following our discussion both in mailing list [1] and at the LPC 2016 [2],
> > > > > > > > we would like to propose this RDMA tool to be part of iproute2 package
> > > > > > > > and finally improve this situation.
> > > > > > >
> > > > > > > Hello Leon,
> > > > > > >
> > > > > > > Although I really appreciate your work: can you clarify why you would like to
> > > > > > > add *RDMA* functionality to an *IP routing* tool? I haven't found any motivation
> > > > > > > for adding RDMA functionality to iproute2 in [1].
> > > > > >
> > > > > > We are planning to reuse the same infrastructure provided by iproute2,
> > > > > > like netlink parsing, access to distributions, same CLI and same standards.
> > > > > >
> > > > > > Right now, RDMA is already tightened to netdev: iWARP, RoCE, IPoIB, HFI-VNIC.
> > > > > > Many drivers (mlx, qed, i40, cxgb) are sharing code between net and
> > > > > > RDMA.
> > > > > >
> > > > > > I do expect that iproute2 will be installed on every machine with any
> > > > > > type of connection, including IB and OPA.
> > > > > >
> > > > > > So I think that it is enough to be part of that suite and don't invent
> > > > > > our own for one specific tool.
> > > > >
> > > > > Hello Leon,
> > > > >
> > > > > Sorry but to me that sounds like a weak argument for including RDMA functionality
> > > > > in iproute2. There is already a library for communication over netlink sockets,
> > > > > namely libnl. Is there functionality that is in iproute2 but not in libnl and
> > > > > that is needed for the new tool? If so, have you considered to create a new
> > > > > library for that functionality?
> > > >
> > > > It is not hard to create new tool, the hardest part is to ensure that it is
> > > > part of the distributions. Did you count how many months we are trying to
> > > > add rdma-core to debian?
> > >
> > > I do agree that it is a strange pairing and am not really a fan. However at
> > > the end of the day it's just a name for a repo/package. If the iproute folks
> > > are fine to include rdma in their repo/package, great we can leverage their
> > > code for CLI and other common stuff.
> > >
> > > Now if the interface was something like "ip -FlagForRdma ..." I would object
> > > to that, but the interface is "rdma ... " so from users perspective it's
> > > different tools. They don't need to care that it was sourced from a common
> > > git repo.
> > >
> > > Just as an aside this already works a bit with OPA:
> > >
> > > $ ./rdma link
> > > 1/1: hfi1_0/1: ifname NONE cap_mask 0x00410022 lid 0x1 lid_mask_count 0
> > > link_layer InfiniBand
> > > phys_state 5: LinkUp rate 100 Gb/sec (4X EDR) sm_lid 0x1 sm_sl 0
> > > state 4: ACTIVE
> > >
> > > Leon I'll get you more feedback and testing, I've just been really bogged
> > > down this week, sorry.
> >
> > Thanks Denny,
> >
> > Before you are starting to test it, can you please provide your feedback
> > on my initial questions? Usability and need of sysfs.
> > ----
> > This is initial phase to understand if user experience for this tool fits
> > RDMA and netdev communities exepectations. Also I would like to get feedback
> > if it is really worth to provide legacy sysfs for old kernels, or maybe I should
> > implement netlink from the beginning and abandon sysfs completely.
> > -----
>
> For the initial phase I think you did the right thing by reading sysfs. I
> like the ability for the tool to be compatible with legacy kernels, but at
> the same time I don't know if it's worth the hassle. I won't fight too hard
> either way.
>
> Perhaps we should take a stab and seeing what a dual sysfs/netlink interface
> would look like, and see just how hard and complicated it really is. Then we
> can make that call. You already have the sysfs version, and have to do
> netink anyway, so let's not rip out what's there just yet, this is an RFC
> after all, not like you are asking for this exact version to be merged yet.
Not at all, it is too early to merge it, at the end it is POC.
>
> As far as usability, I think what's here is a great start and we can
> continue to refine. I'm particularly interested in the stats capabilities.
> In particular being able to filter what stats are shown and watch as they
> change over time. RDMA devices have lots of counters and stats. A common
> tool for users to be able to get that data across HW types would be a really
> good thing.
Yeah, the statistics will be tough nut to crack.
>
> -Denny
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] qca_debug: Reduce function calls for sequence output in qcaspi_info_show()
From: Stefan Wahren @ 2017-05-09 7:07 UTC (permalink / raw)
To: SF Markus Elfring, netdev, David S. Miller, Philippe Reynes
Cc: LKML, kernel-janitors
In-Reply-To: <9932ee63-f766-49ce-d760-9a0bd06f0657@users.sourceforge.net>
Am 08.05.2017 um 19:29 schrieb SF Markus Elfring:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 8 May 2017 19:21:27 +0200
>
> A bit of data was put into a sequence by separate function calls.
> Print the same data together with adjusted seq_printf() calls instead.
Sorry, i'm not happy with this patch. It doesn't increase readabilityand
mixes the output of multiple lines.
The only benefit is a little bit higher performance. But for debugfs
this won't be necessary.
Stefan
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> drivers/net/ethernet/qualcomm/qca_debug.c | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox