* Re: [PATCH] mv88e6xxx: Fix mv88e6xxx_g1_irq_free() interrupt count
From: Andrew Lunn @ 2016-11-27 21:39 UTC (permalink / raw)
To: Andreas Färber
Cc: netdev, linux-arm-kernel, Uwe Kleine-König, Michal Hrusecki,
Tomas Hlavacek, Bed??icha Ko??atu, Vivien Didelot,
Florian Fainelli, linux-kernel
In-Reply-To: <ff0d121a-0ecb-8af7-af11-cf94df31639c@suse.de>
On Sun, Nov 27, 2016 at 10:32:41PM +0100, Andreas Färber wrote:
> Hi Andrew,
>
> Am 27.11.2016 um 22:22 schrieb Andrew Lunn:
> > On Sun, Nov 27, 2016 at 09:43:44PM +0100, Andreas Färber wrote:
> >> mv88e6xxx_g1_irq_setup() sets up chip->g1_irq.nirqs interrupt mappings,
> >> so free the same amount. This will be 8 or 9 in practice, less than 16.
> >
> > Hi Andreas
> >
> > The patch is correct, but please read
> > Documentation/networking/netdev-FAQ.txt
> > and then resubmit the patch.
>
> Do you mean --subject-prefix="PATCH net-next"
Yep.
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH 2/2] net: dsa: mv88e6xxx: Add 88E6176 device tree support
From: Andreas Färber @ 2016-11-27 21:50 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, Michal Hrusecki, Uwe Kleine-König,
Vivien Didelot, netdev, Tomas Hlavacek, linux-kernel,
linux-arm-kernel, Bed??icha Ko??atu
In-Reply-To: <20161127212709.GD13318@lunn.ch>
Am 27.11.2016 um 22:27 schrieb Andrew Lunn:
> On Sun, Nov 27, 2016 at 09:57:59PM +0100, Andreas Färber wrote:
>> This model is found on the Turris Omnia.
>
> This driver already supports nearly 30 different Marvell switch
> models. Please document why the marvell,mv88e6176 is special and why
> it needs its own compatible string when the others don't.
I don't understand.
The commit message above already points out for which device this is
(and you also know from the LAKML thread).
You as driver author should know that the .data pointer is vital to your
driver - you even recently accepted another model that conflicted with
my patch. So are you arguing for a ", which uses a Device Tree for
booting" half-sentence here?
The others not having an entry simply means no one needed them yet.
And any Turris Omnia side changes need to go through the mvebu tree.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply
* Re: [PATCH 2/2] net: dsa: mv88e6xxx: Add 88E6176 device tree support
From: Andrew Lunn @ 2016-11-27 22:08 UTC (permalink / raw)
To: Andreas Färber
Cc: netdev, linux-arm-kernel, Uwe Kleine-König, Michal Hrusecki,
Tomas Hlavacek, Bed??icha Ko??atu, Vivien Didelot,
Florian Fainelli, linux-kernel
In-Reply-To: <9500470d-09c3-3ecb-994b-3d108bffc99e@suse.de>
> > This driver already supports nearly 30 different Marvell switch
> > models. Please document why the marvell,mv88e6176 is special and why
> > it needs its own compatible string when the others don't.
>
> I don't understand.
Think about what i said. Why does the 6176 need its own compatible
string, when the two 6352s and the 6165 on the zii-devel-b don't have
one? And the DIR 665 has a 6171, which does not have a compatible
string of its own. The clearfog actually has a 6176, and it seems to
work fine without a compatible string.
> You as driver author should know that the .data pointer is vital to your
> driver
Exactly, so if i ask why is it needed, maybe you should stop and think
for a while.
> you even recently accepted another model that conflicted with
> my patch.
And think about that also, and you will find the 6390 family, who's
first device is 6190, is not compatible with the 6085, and so needs a
different compatible string.
Andrew
^ permalink raw reply
* Re: [PATCH net-next v2 3/4] Documentation: net: phy: Add blurb about RGMII
From: Timur Tabi @ 2016-11-27 22:24 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: davem, andrew, sf84, martin.blumenstingl, mans, alexandre.torgue,
peppe.cavallaro, jbrunet
In-Reply-To: <20161127184449.12351-4-f.fainelli@gmail.com>
Just some grammatical corrections. You might want to run a spellchecker
on all the patches.
Florian Fainelli wrote:
> + The Reduced Gigabit Medium Independent Interface (RGMII) is a 12 pins
"is a 12-pin"
> + electrical signal interface using a synchronous 125Mhz clock signal and several
> + data lines. Due to this design decision, a 1.5ns to 2ns delay must be added
> + between the clock line (RXC or TXC) and the data lines to let the PHY (clock
> + sink) have enough setup and hold times to sample the data lines correctly. The
> + PHY library offers different types of PHY_INTERFACE_MODE_RGMII* values to let
> + the PHY driver and optionaly the MAC driver implement the required delay. The
"driver, and optionally the MAC driver, implement"
> + values of phy_interface_t must be understood from the perspective of the PHY
> + device itself, leading to the following:
> +
> + * PHY_INTERFACE_MODE_RGMII: the PHY is not responsible for inserting any
> + internal delay by itself, it assumes that either the Ethernet MAC (if capable
> + or the PCB traces) insert the correct 1.5-2ns delay
> +
> + * PHY_INTERFACE_MODE_RGMII_TXID: the PHY should be inserting an internal delay
"should insert"
> + for the transmit data lines (TXD[3:0]) processed by the PHY device
> +
> + * PHY_INTERFACE_MODE_RGMII_RXID: the PHY should be inserting an internal delay
"should insert"
> + for the receive data lines (RXD[3:0]) processed by the PHY device
> +
> + * PHY_INTERFACE_MODE_RGMII_ID: the PHY should be inserting internal delays for
"should insert"
> + both transmit AND receive data lines from/to the PHY device
> +
> + Whenever it is possible, it is preferrable to utilize the PHY side RGMII delay
> + for several reasons:
"Whenever possible, use the PHY side RGMII delay for these reasons:"
> + * PHY devices may offer sub-nanosecond granularity in how they allow a
> + receiver/transmitter side delay (e.g: 0.5, 1.0, 1.5ns) to be specified. Such
> + precision may be required to account for differences in PCB trace lengths
> +
> + * PHY devices are typically qualified for a large range of applications
> + (industrial, medical, automotive...), and they provide a constant and
> + reliable delay across temperature/pressure/voltage ranges
> +
> + * PHY device drivers in PHYLIB being reusable by nature, being able to
> + configure correctly a specified delay enables more designs with similar delay
> + requirements to be operate correctly
Ok, this one I don't know how to fix. I'm not really sure what you're
trying to say.
> +
> + For cases where the PHY is not capable of providing this delay, but the
> + Ethernet MAC driver is capable of doing it, the correct phy_interface_t value
"doing so,"
> + should be PHY_INTERFACE_MODE_RGMII, and the Ethernet MAC driver should be
> + configured correctly in order to provide the required transmit and/or receive
> + side delay from the perspective of the PHY device. Conversely, if the Ethernet
> + MAC driver looks at the phy_interface_t value, for any other mode but
> + PHY_INTERFACE_MODE_RGMII, it should make sure that the MAC-level delays are
> + disabled.
> +
> + In case neither the Ethernet MAC, nor the PHY are capable of providing the
> + required delays, as defined per the RGMII standard, several options may be
> + available:
> +
> + * Some SoCs may offer a pin pad/mux/controller capable of configuring a given
> + set of pins' drive strength, delays and voltage, and it may be a suitable
"strength, delays, and voltage; and"
> + option to insert the expected 2ns RGMII delay
> +
> + * Modifying the PCB design to include a fixed delay (e.g: using a specifically
> + designed serpentine), which may not require software configuration at all
period after "all".
> +
> +Common problems with RGMII delay mismatch
> +
> + When there is a RGMII delay mismatch between the Ethernet MAC and the PHY, this
> + will most likely result in the clock and data line sampling to capture unstable
I'm not sure what "sampling to capture unstable" is supposed to mean.
> + signals, typical symptoms include:
> +
> + * Transmission/reception partially works, and there is frequent or occasional
> + packet loss observed
> +
> + * Ethernet MAC may report some, or all packets ingressing with a FCS/CRC error,
No comma after "some".
> + or just discard them all
> +
> + * Switching to lower speeds such as 10/100Mbits/sec makes the problem go away
> + (since there is enough setup/hold time in that case)
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.
^ permalink raw reply
* [PATCH net-next] net: dsa: mv88e6xxx: Fix mv88e6xxx_g1_irq_free() interrupt count
From: Andreas Färber @ 2016-11-27 22:26 UTC (permalink / raw)
To: David S . Miller
Cc: Uwe Kleine-König, Michal Hrusecki, Tomas Hlavacek,
Bedřicha Košatu, Andreas Färber, Andrew Lunn,
Vivien Didelot, Florian Fainelli, netdev, linux-kernel
mv88e6xxx_g1_irq_setup() sets up chip->g1_irq.nirqs interrupt mappings,
so free the same amount. This will be 8 or 9 in practice, less than 16.
Fixes: dc30c35be720 ("net: dsa: mv88e6xxx: Implement interrupt support.")
Cc: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index b14b3d5099c8..77f13ada2612 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -421,7 +421,7 @@ static void mv88e6xxx_g1_irq_free(struct mv88e6xxx_chip *chip)
free_irq(chip->irq, chip);
- for (irq = 0; irq < 16; irq++) {
+ for (irq = 0; irq < chip->g1_irq.nirqs; irq++) {
virq = irq_find_mapping(chip->g1_irq.domain, irq);
irq_dispose_mapping(virq);
}
--
2.6.6
^ permalink raw reply related
* Re: [PATCH net-next] net: dsa: mv88e6xxx: Fix mv88e6xxx_g1_irq_free() interrupt count
From: Andrew Lunn @ 2016-11-27 22:30 UTC (permalink / raw)
To: Andreas Färber
Cc: David S . Miller, Uwe Kleine-König, Michal Hrusecki,
Tomas Hlavacek, Bed??icha Ko??atu, Vivien Didelot,
Florian Fainelli, netdev, linux-kernel
In-Reply-To: <1480285588-13501-1-git-send-email-afaerber@suse.de>
On Sun, Nov 27, 2016 at 11:26:28PM +0100, Andreas Färber wrote:
> mv88e6xxx_g1_irq_setup() sets up chip->g1_irq.nirqs interrupt mappings,
> so free the same amount. This will be 8 or 9 in practice, less than 16.
>
> Fixes: dc30c35be720 ("net: dsa: mv88e6xxx: Implement interrupt support.")
> Cc: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH 2/2] net: dsa: mv88e6xxx: Add 88E6176 device tree support
From: Andreas Färber @ 2016-11-27 22:42 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev, linux-arm-kernel, Uwe Kleine-König, Michal Hrusecki,
Tomas Hlavacek, Bed??icha Ko??atu, Vivien Didelot,
Florian Fainelli, linux-kernel
In-Reply-To: <20161127220846.GH13318@lunn.ch>
Andrew,
Am 27.11.2016 um 23:08 schrieb Andrew Lunn:
>>> This driver already supports nearly 30 different Marvell switch
>>> models. Please document why the marvell,mv88e6176 is special and why
>>> it needs its own compatible string when the others don't.
>>
>> I don't understand.
>
> Think about what i said. Why does the 6176 need its own compatible
> string, when the two 6352s and the 6165 on the zii-devel-b don't have
> one? And the DIR 665 has a 6171, which does not have a compatible
> string of its own. The clearfog actually has a 6176, and it seems to
> work fine without a compatible string.
>
>> You as driver author should know that the .data pointer is vital to your
>> driver
>
> Exactly, so if i ask why is it needed, maybe you should stop and think
> for a while.
>
>> you even recently accepted another model that conflicted with
>> my patch.
>
> And think about that also, and you will find the 6390 family, who's
> first device is 6190, is not compatible with the 6085, and so needs a
> different compatible string.
Try to see it from my perspective: I see that some vf610 device I don't
have (found via `git grep marvell,mv88e6` or so) uses
"marvell,mv88e6085". I then assume it has that device on board. How
would I know it doesn't? Same for the other boards you mention.
Unfortunately some of your replies are slightly cryptic. Had you simply
replied 'please just use "marvell,mv88e6085" instead', it would've been
much more clear what you want. (Same for extending the subject instead
of just pointing to some FAQ.)
So are you okay with patch 1/2 documenting the compatible? Then we could
drop 2/2 and use "marvell,mv88e6176", "marvell,mv88e6085" instead of
just the latter. Or would you rather drop both and keep the actual chip
a comment?
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
^ permalink raw reply
* Re: [PATCH net-next v2 3/4] Documentation: net: phy: Add blurb about RGMII
From: Florian Fainelli @ 2016-11-27 23:02 UTC (permalink / raw)
To: Timur Tabi, netdev
Cc: davem, andrew, sf84, martin.blumenstingl, mans, alexandre.torgue,
peppe.cavallaro, jbrunet
In-Reply-To: <583B5D3B.4040108@codeaurora.org>
Le 27/11/2016 à 14:24, Timur Tabi a écrit :
>> + * PHY device drivers in PHYLIB being reusable by nature, being able to
>> + configure correctly a specified delay enables more designs with
>> similar delay
>> + requirements to be operate correctly
>
> Ok, this one I don't know how to fix. I'm not really sure what you're
> trying to say.
What I am trying to say is that once a PHY driver properly configures a
delay that you have specified, there is no reason why this is not
applicable to other platforms using this same PHY driver.
>> +
>> +Common problems with RGMII delay mismatch
>> +
>> + When there is a RGMII delay mismatch between the Ethernet MAC and
>> the PHY, this
>> + will most likely result in the clock and data line sampling to
>> capture unstable
>
> I'm not sure what "sampling to capture unstable" is supposed to mean.
When the PHY devices takes a "snapshot" of the state of the data lines,
after a clock edge, if the delay is improperly configured, these data
lines are going to still be floating, or show some kind of
capacitance/inductance effect, so the logical level which is going to be
read may be incorrect.
--
Florian
^ permalink raw reply
* Re: [PATCH 2/2] net: dsa: mv88e6xxx: Add 88E6176 device tree support
From: Andrew Lunn @ 2016-11-27 23:10 UTC (permalink / raw)
To: Andreas Färber
Cc: netdev, linux-arm-kernel, Uwe Kleine-König, Michal Hrusecki,
Tomas Hlavacek, Bed??icha Ko??atu, Vivien Didelot,
Florian Fainelli, linux-kernel
In-Reply-To: <e5d45b55-a4c1-3b5e-0bdc-a5e340b072d0@suse.de>
> Try to see it from my perspective: I see that some vf610 device I don't
> have (found via `git grep marvell,mv88e6` or so) uses
> "marvell,mv88e6085". I then assume it has that device on board. How
> would I know it doesn't? Same for the other boards you mention.
>
> Unfortunately some of your replies are slightly cryptic. Had you simply
> replied 'please just use "marvell,mv88e6085" instead', it would've been
> much more clear what you want. (Same for extending the subject instead
> of just pointing to some FAQ.)
By reading the FAQ you have learnt more than me saying put the correct
tree in the subject line. By asking you to explain why you need a
compatible string, i'm trying to make you think, look at the code and
understand it. In the future, you might think and understand the code
before posting a patch, and then we all save time.
> So are you okay with patch 1/2 documenting the compatible? Then we could
> drop 2/2 and use "marvell,mv88e6176", "marvell,mv88e6085" instead of
> just the latter. Or would you rather drop both and keep the actual chip
> a comment?
A comment only please.
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH net 1/1] driver: ipvlan: Fix one possible memleak in ipvlan_link_new
From: David Miller @ 2016-11-28 0:58 UTC (permalink / raw)
To: fgao; +Cc: maheshb, edumazet, netdev, gfree.wind
In-Reply-To: <1480001999-10500-1-git-send-email-fgao@ikuai8.com>
From: fgao@ikuai8.com
Date: Thu, 24 Nov 2016 23:39:59 +0800
> From: Gao Feng <fgao@ikuai8.com>
>
> When ipvlan_link_new fails and creates one ipvlan port, it does not
> destroy the ipvlan port created. It causes mem leak and the physical
> device contains invalid ipvlan data.
>
> Signed-off-by: Gao Feng <fgao@ikuai8.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] irda: fix overly long udelay()
From: David Miller @ 2016-11-28 1:00 UTC (permalink / raw)
To: arnd; +Cc: samuel, netdev, linux-kernel
In-Reply-To: <20161124162630.3802535-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Thu, 24 Nov 2016 17:26:22 +0100
> irda_get_mtt() returns a hardcoded '10000' in some cases,
> and with gcc-7, we get a build error because this triggers a
> compile-time check in udelay():
>
> drivers/net/irda/w83977af_ir.o: In function `w83977af_hard_xmit':
> w83977af_ir.c:(.text.w83977af_hard_xmit+0x14c): undefined reference to `__bad_udelay'
>
> Older compilers did not run into this because they either did not
> completely inline the irda_get_mtt() or did not consider the
> 10000 value a constant expression.
>
> The code has been wrong since the start of git history.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied, thanks Arnd.
^ permalink raw reply
* Re: [PATCH net v2 0/5] net: fix phydev reference leaks
From: David Miller @ 2016-11-28 1:02 UTC (permalink / raw)
To: johan
Cc: f.fainelli, madalin.bucur, timur, andrew, vivien.didelot, netdev,
linux-kernel
In-Reply-To: <1480011691-13278-1-git-send-email-johan@kernel.org>
From: Johan Hovold <johan@kernel.org>
Date: Thu, 24 Nov 2016 19:21:26 +0100
> This series fixes a number of phydev reference leaks (and one of_node
> leak) due to failure to put the reference taken by of_phy_find_device().
>
> Note that I did not try to fix drivers/net/phy/xilinx_gmii2rgmii.c which
> still leaks a reference.
>
> Against net but should apply just as fine to net-next.
...
> v2:
> - use put_device() instead of phy_dev_free() to put the references
> taken in net/dsa (patch 1/4).
> - add four new patches fixing similar leaks
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH net 1/1] driver: macvtap: Unregister netdev rx_handler if macvtap_newlink fails
From: David Miller @ 2016-11-28 1:04 UTC (permalink / raw)
To: fgao; +Cc: jasowang, edumazet, netdev, gfree.wind
In-Reply-To: <1480039506-29740-1-git-send-email-fgao@ikuai8.com>
From: fgao@ikuai8.com
Date: Fri, 25 Nov 2016 10:05:06 +0800
> From: Gao Feng <fgao@ikuai8.com>
>
> The macvtap_newlink registers the netdev rx_handler firstly, but it
> does not unregister the handler if macvlan_common_newlink failed.
>
> Signed-off-by: Gao Feng <fgao@ikuai8.com>
Applied.
^ permalink raw reply
* Re: pull request (net): ipsec 2016-11-25
From: David Miller @ 2016-11-28 1:22 UTC (permalink / raw)
To: steffen.klassert; +Cc: herbert, netdev
In-Reply-To: <1480057080-20177-1-git-send-email-steffen.klassert@secunet.com>
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Fri, 25 Nov 2016 07:57:57 +0100
> 1) Fix a refcount leak in vti6.
> From Nicolas Dichtel.
>
> 2) Fix a wrong if statement in xfrm_sk_policy_lookup.
> From Florian Westphal.
>
> 3) The flowcache watermarks are per cpu. Take this into
> account when comparing to the threshold where we
> refusing new allocations. From Miroslav Urbanek.
>
> Please pull or let me know if there are problems.
Pulled, thanks Steffen!
^ permalink raw reply
* Re: [PATCH 1/1] net: macb: fix the RX queue reset in macb_rx()
From: David Miller @ 2016-11-28 1:25 UTC (permalink / raw)
To: cyrille.pitchen
Cc: nicolas.ferre, netdev, soren.brinkmann, Andrei.Pistirica,
linux-arm-kernel, linux-kernel
In-Reply-To: <fd8131cbe6c0546b2b8ee35bcaac5e7eb1a1647f.1480063339.git.cyrille.pitchen@atmel.com>
From: Cyrille Pitchen <cyrille.pitchen@atmel.com>
Date: Fri, 25 Nov 2016 09:49:32 +0100
> On macb only (not gem), when a RX queue corruption was detected from
> macb_rx(), the RX queue was reset: during this process the RX ring
> buffer descriptor was initialized by macb_init_rx_ring() but we forgot
> to also set bp->rx_tail to 0.
>
> Indeed, when processing the received frames, bp->rx_tail provides the
> macb driver with the index in the RX ring buffer of the next buffer to
> process. So when the whole ring buffer is reset we must also reset
> bp->rx_tail so the driver is synchronized again with the hardware.
>
> Since macb_init_rx_ring() is called from many locations, currently from
> macb_rx() and macb_init_rings(), we'd rather add the "bp->rx_tail = 0;"
> line inside macb_init_rx_ring() than add the very same line after each
> call of this function.
>
> Without this fix, the rx queue is not reset properly to recover from
> queue corruption and connection drop may occur.
>
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> Fixes: 9ba723b081a2 ("net: macb: remove BUG_ON() and reset the queue to handle RX errors")
This doesn't apply cleanly to the 'net' tree, where
RX_RING_SIZE is used instead of bp->rx_ring_size. It seems
you generated this against net-next, however you didn't say
that either in your Subject line nor the commit message.
As a bug fix this should be targetted at 'net'.
^ permalink raw reply
* Re: pull-request: wireless-drivers-next 2016-11-25
From: David Miller @ 2016-11-28 1:27 UTC (permalink / raw)
To: kvalo-sgV2jX0FEOL9JmXXK+q4OQ
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <8760nbaoey.fsf-HodKDYzPHsUD5k0oWYwrnHL1okKdlPRT@public.gmane.org>
From: Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Date: Fri, 25 Nov 2016 11:39:49 +0200
> here's a pull request for 4.10. ath9k has now been converted to use
> mac80211 intermediate software queues to fix bufferbloat problems. rsi
> has become active again and latevy mwifiex has been getting a _lot_ of
> love.
>
> I'm not expecting to see any problems with this pull request. When you
> pull git will do lots of automerging but at least I didn't see any
> conflicts. Please let me know if you have any problems.
Pulled, thanks Kalle.
^ permalink raw reply
* Re: [PATCH] net: fec: turn on device when extracting statistics
From: David Miller @ 2016-11-28 1:29 UTC (permalink / raw)
To: nikita.yoush
Cc: fugang.duan, troy.kisky, andrew, eric, tremyfr, johannes, netdev,
linux-kernel, cphealy
In-Reply-To: <1480068120-22137-1-git-send-email-nikita.yoush@cogentembedded.com>
From: Nikita Yushchenko <nikita.yoush@cogentembedded.com>
Date: Fri, 25 Nov 2016 13:02:00 +0300
> + int i, ret;
> +
> + ret = pm_runtime_get_sync(&fep->pdev->dev);
> + if (IS_ERR_VALUE(ret)) {
> + memset(data, 0, sizeof(*data) * ARRAY_SIZE(fec_stats));
> + return;
> + }
This really isn't the way to do this.
When the device is suspended and the clocks are going to be stopped,
you must fetch the statistic values into a software copy and provide
those if the device is suspended when statistics are requested.
^ permalink raw reply
* Re: [patch v2 net-next] sfc: remove unneeded variable
From: David Miller @ 2016-11-28 1:30 UTC (permalink / raw)
To: dan.carpenter; +Cc: linux-net-drivers, ecree, bkenward, netdev, kernel-janitors
In-Reply-To: <20161125104304.GA5938@mwanda>
From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Fri, 25 Nov 2016 13:43:04 +0300
> We don't use ->heap_buf after commit 46d1efd852cc ("sfc: remove Software
> TSO") so let's remove the last traces.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Applied, thanks Dan.
^ permalink raw reply
* [net,v2] neigh: fix the loop index error in neigh dump
From: Zhang Shengju @ 2016-11-28 1:32 UTC (permalink / raw)
To: netdev, dsa
Loop index in neigh dump function is not updated correctly under some
circumstances, this patch will fix it.
Fixes: 16660f0bd9 ("net: Add support for filtering neigh dump by device index")
Fixes: 21fdd092ac ("net: Add support for filtering neigh dump by master device")
Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
---
net/core/neighbour.c | 39 ++++++++++++++++++---------------------
1 file changed, 18 insertions(+), 21 deletions(-)
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 2ae929f..ce32e9c 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -2256,6 +2256,16 @@ static bool neigh_ifindex_filtered(struct net_device *dev, int filter_idx)
return false;
}
+static bool neigh_dump_filtered(struct net_device *dev, int filter_idx,
+ int filter_master_idx)
+{
+ if (neigh_ifindex_filtered(dev, filter_idx) ||
+ neigh_master_filtered(dev, filter_master_idx))
+ return true;
+
+ return false;
+}
+
static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
struct netlink_callback *cb)
{
@@ -2285,20 +2295,15 @@ static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
rcu_read_lock_bh();
nht = rcu_dereference_bh(tbl->nht);
- for (h = s_h; h < (1 << nht->hash_shift); h++) {
- if (h > s_h)
- s_idx = 0;
+ for (h = s_h; h < (1 << nht->hash_shift); h++, s_idx = 0) {
for (n = rcu_dereference_bh(nht->hash_buckets[h]), idx = 0;
n != NULL;
- n = rcu_dereference_bh(n->next)) {
- if (!net_eq(dev_net(n->dev), net))
- continue;
- if (neigh_ifindex_filtered(n->dev, filter_idx))
+ n = rcu_dereference_bh(n->next), idx++) {
+ if (idx < s_idx || !net_eq(dev_net(n->dev), net))
continue;
- if (neigh_master_filtered(n->dev, filter_master_idx))
+ if (neigh_dump_filtered(n->dev, filter_idx,
+ filter_master_idx))
continue;
- if (idx < s_idx)
- goto next;
if (neigh_fill_info(skb, n, NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq,
RTM_NEWNEIGH,
@@ -2306,8 +2311,6 @@ static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
rc = -1;
goto out;
}
-next:
- idx++;
}
}
rc = skb->len;
@@ -2328,14 +2331,10 @@ static int pneigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
read_lock_bh(&tbl->lock);
- for (h = s_h; h <= PNEIGH_HASHMASK; h++) {
- if (h > s_h)
- s_idx = 0;
- for (n = tbl->phash_buckets[h], idx = 0; n; n = n->next) {
- if (pneigh_net(n) != net)
+ for (h = s_h; h <= PNEIGH_HASHMASK; h++, s_idx = 0) {
+ for (n = tbl->phash_buckets[h], idx = 0; n; n = n->next, idx++) {
+ if (idx < s_idx || pneigh_net(n) != net)
continue;
- if (idx < s_idx)
- goto next;
if (pneigh_fill_info(skb, n, NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq,
RTM_NEWNEIGH,
@@ -2344,8 +2343,6 @@ static int pneigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
rc = -1;
goto out;
}
- next:
- idx++;
}
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v2 0/7] stmmac: dwmac-meson8b: configurable RGMII TX delay
From: David Miller @ 2016-11-28 1:33 UTC (permalink / raw)
To: martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg
Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
khilman-rdvid1DuHRBWk0Htik3J/w, mark.rutland-5wv7dgnIgG8,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
alexandre.torgue-qxv4g6HH51o, peppe.cavallaro-qxv4g6HH51o,
will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
carlo-KA+7E9HrN00dnm+yROfE0A, f.fainelli-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20161125130156.17879-1-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
From: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
Date: Fri, 25 Nov 2016 14:01:49 +0100
> Currently the dwmac-meson8b stmmac glue driver uses a hardcoded 1/4
> cycle TX clock delay. This seems to work fine for many boards (for
> example Odroid-C2 or Amlogic's reference boards) but there are some
> others where TX traffic is simply broken.
> There are probably multiple reasons why it's working on some boards
> while it's broken on others:
> - some of Amlogic's reference boards are using a Micrel PHY
> - hardware circuit design
> - maybe more...
The ARM arch file changes do not apply cleanly to net-next, you probably
want to merge them via the ARM tree instead of mine, and respin this series
to be without the .dts file changes.
--
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 net 1/1] tipc: fix link statistics counter errors
From: David Miller @ 2016-11-28 1:37 UTC (permalink / raw)
To: jon.maloy
Cc: netdev, parthasarathy.bhuvaragan, ying.xue, maloy,
tipc-discussion
In-Reply-To: <1480088102-31516-1-git-send-email-jon.maloy@ericsson.com>
From: Jon Maloy <jon.maloy@ericsson.com>
Date: Fri, 25 Nov 2016 10:35:02 -0500
> In commit e4bf4f76962b ("tipc: simplify packet sequence number
> handling") we changed the internal representation of the packet
> sequence number counters from u32 to u16, reflecting what is really
> sent over the wire.
>
> Since then some link statistics counters have been displaying incorrect
> values, partially because the counters meant to be used as sequence
> number snapshots are now used as direct counters, stored as u32, and
> partially because some counter updates are just missing in the code.
>
> In this commit we correct this in two ways. First, we base the
> displayed packet sent/received values on direct counters instead
> of as previously a calculated difference between current sequence
> number and a snapshot. Second, we add the missing updates of the
> counters.
>
> This change is compatible with the current netlink API, and requires
> no changes to the user space tools.
>
> Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 0/6] BPF cleanups and misc updates
From: David Miller @ 2016-11-28 1:39 UTC (permalink / raw)
To: daniel; +Cc: alexei.starovoitov, netdev
In-Reply-To: <cover.1480119395.git.daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Sat, 26 Nov 2016 01:28:03 +0100
> This patch set adds couple of cleanups in first few patches,
> exposes owner_prog_type for array maps as well as mlocked mem
> for maps in fdinfo, allows for mount permissions in fs and
> fixes various outstanding issues in selftests and samples.
Series applied, thanks Daniel.
^ permalink raw reply
* Re: [net,v2] neigh: fix the loop index error in neigh dump
From: David Ahern @ 2016-11-28 2:09 UTC (permalink / raw)
To: Zhang Shengju, netdev
In-Reply-To: <1480296725-5563-1-git-send-email-zhangshengju@cmss.chinamobile.com>
On 11/27/16 6:32 PM, Zhang Shengju wrote:
> Loop index in neigh dump function is not updated correctly under some
> circumstances, this patch will fix it.
What's an example?
>
> Fixes: 16660f0bd9 ("net: Add support for filtering neigh dump by device index")
> Fixes: 21fdd092ac ("net: Add support for filtering neigh dump by master device")
>
> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
> ---
> net/core/neighbour.c | 39 ++++++++++++++++++---------------------
> 1 file changed, 18 insertions(+), 21 deletions(-)
>
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 2ae929f..ce32e9c 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -2256,6 +2256,16 @@ static bool neigh_ifindex_filtered(struct net_device *dev, int filter_idx)
> return false;
> }
>
> +static bool neigh_dump_filtered(struct net_device *dev, int filter_idx,
> + int filter_master_idx)
> +{
> + if (neigh_ifindex_filtered(dev, filter_idx) ||
> + neigh_master_filtered(dev, filter_master_idx))
> + return true;
> +
> + return false;
> +}
> +
> static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
> struct netlink_callback *cb)
> {
> @@ -2285,20 +2295,15 @@ static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
> rcu_read_lock_bh();
> nht = rcu_dereference_bh(tbl->nht);
>
> - for (h = s_h; h < (1 << nht->hash_shift); h++) {
> - if (h > s_h)
> - s_idx = 0;
> + for (h = s_h; h < (1 << nht->hash_shift); h++, s_idx = 0) {
> for (n = rcu_dereference_bh(nht->hash_buckets[h]), idx = 0;
> n != NULL;
> - n = rcu_dereference_bh(n->next)) {
> - if (!net_eq(dev_net(n->dev), net))
> - continue;
> - if (neigh_ifindex_filtered(n->dev, filter_idx))
> + n = rcu_dereference_bh(n->next), idx++) {
> + if (idx < s_idx || !net_eq(dev_net(n->dev), net))
> continue;
> - if (neigh_master_filtered(n->dev, filter_master_idx))
> + if (neigh_dump_filtered(n->dev, filter_idx,
> + filter_master_idx))
> continue;
> - if (idx < s_idx)
> - goto next;
> if (neigh_fill_info(skb, n, NETLINK_CB(cb->skb).portid,
> cb->nlh->nlmsg_seq,
> RTM_NEWNEIGH,
> @@ -2306,8 +2311,6 @@ static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
> rc = -1;
> goto out;
> }
> -next:
> - idx++;
> }
> }
> rc = skb->len;
> @@ -2328,14 +2331,10 @@ static int pneigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
>
> read_lock_bh(&tbl->lock);
>
> - for (h = s_h; h <= PNEIGH_HASHMASK; h++) {
> - if (h > s_h)
> - s_idx = 0;
> - for (n = tbl->phash_buckets[h], idx = 0; n; n = n->next) {
> - if (pneigh_net(n) != net)
> + for (h = s_h; h <= PNEIGH_HASHMASK; h++, s_idx = 0) {
> + for (n = tbl->phash_buckets[h], idx = 0; n; n = n->next, idx++) {
> + if (idx < s_idx || pneigh_net(n) != net)
> continue;
> - if (idx < s_idx)
> - goto next;
> if (pneigh_fill_info(skb, n, NETLINK_CB(cb->skb).portid,
> cb->nlh->nlmsg_seq,
> RTM_NEWNEIGH,
> @@ -2344,8 +2343,6 @@ static int pneigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
> rc = -1;
> goto out;
> }
> - next:
> - idx++;
> }
> }
>
This fix is way to be complicated to be fixing anything related to 16660f0bd9 or 21fdd092ac. Both of those commits added a continue:
if (neigh_ifindex_filtered(n->dev, filter_idx))
continue;
if (neigh_master_filtered(n->dev, filter_master_idx))
continue;
At best the continue is replaced by 'goto next;' and I am not convinced that is right.
You are completely rewriting the dump loops.
^ permalink raw reply
* Re: [PATCH net v2 0/5] net: fix phydev reference leaks
From: Timur Tabi @ 2016-11-28 2:11 UTC (permalink / raw)
To: David Miller, johan
Cc: f.fainelli, madalin.bucur, andrew, vivien.didelot, netdev,
linux-kernel
In-Reply-To: <20161127.200238.2048371945400616113.davem@davemloft.net>
David Miller wrote:
> Series applied, thanks.
I was really hoping you'd give me the chance to test the patches before
applying them.
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.
^ permalink raw reply
* Re: [Patch net-next] net_sched: move the empty tp check from ->destroy() to ->delete()
From: John Fastabend @ 2016-11-28 2:26 UTC (permalink / raw)
To: Roi Dayan, Daniel Borkmann, Cong Wang
Cc: Linux Kernel Network Developers, Jiri Pirko
In-Reply-To: <583A7D67.50003@mellanox.com>
On 16-11-26 10:29 PM, Roi Dayan wrote:
>
>
> On 27/11/2016 06:47, Roi Dayan wrote:
>>
>>
>> On 27/11/2016 02:33, Daniel Borkmann wrote:
>>> On 11/26/2016 12:09 PM, Daniel Borkmann wrote:
>>>> On 11/26/2016 07:46 AM, Cong Wang wrote:
>>>>> On Thu, Nov 24, 2016 at 7:20 AM, Daniel Borkmann
>>>>> <daniel@iogearbox.net> wrote:
>>> [...]
>>>>>> Ok, strange, qdisc_destroy() calls into ops->destroy(), where ingress
>>>>>> drops its entire chain via tcf_destroy_chain(), so that will be NULL
>>>>>> eventually. The tps are freed by call_rcu() as well as qdisc itself
>>>>>> later on via qdisc_rcu_free(), where it frees per-cpu bstats as well.
>>>>>> Outstanding readers should either bail out due to if (!cl) or can
>>>>>> still
>>>>>> process the chain until read section ends, but during that time,
>>>>>> cl->q
>>>>>> resp. bstats should be good. Do you happen to know what's at address
>>>>>> ffff880a68b04028? I was wondering wrt call_rcu() vs call_rcu_bh(),
>>>>>> but
>>>>>> at least on ingress (netif_receive_skb_internal()) we hold
>>>>>> rcu_read_lock()
>>>>>> here. The KASAN report is reliably happening at this location, right?
>>>>>
>>>>> I am confused as well, I don't see how it could be related to my
>>>>> patch yet.
>>>>> I will take a deep look in the weekend.
>>
>>
>>
>> Hi Cong,
>>
>> When reported the new trace I didn't mean it's related to your patch,
>> I just wanted to point it out it exposed something. I should have been
>> clear about it.
>>
>>
>>>>
>>>> Ok, I'm currently on the run. Got too late yesterday night, but I'll
>>>> write what I found in the evening today, not related to ingress though.
>>>
>>> Just pushed out my analysis to netdev under "[PATCH net] net, sched:
>>> respect
>>> rcu grace period on cls destruction". My conclusion is that both
>>> issues are
>>> actually separate, and that one is small enough where we could route
>>> it via
>>> net actually. Perhaps this at the same time shrinks your "[PATCH
>>> net-next]
>>> net_sched: move the empty tp check from ->destroy() to ->delete()" to a
>>> reasonable size that it's suitable to net as well. Your
>>> ->delete()/->destroy()
>>> one is definitely needed, too. The tp->root one is independant of
>>> ->delete()/
>>> ->destroy() as they are different races and tp->root could also
>>> happen when
>>> you just destroy the whole tp directly. I think that seems like a
>>> good path
>>> forward to me.
>>>
>>> Thanks,
>>> Daniel
>>
>>
>>
>> Hi Daniel,
>>
>> As for the tainted kernel. I was in old (week or two) net-next tree
>> and only cherry-picked from latest net-next related patches to
>> Mellanox HCA, cls_api, cls_flower, devlink. so those are the tainted
>> modules.
>> I have the issue reproducing in that tree so wanted it to check it
>> with Cong's patch instead of latest net-next.
>> I'll try running reproducing the issue with your new patch and later
>> try latest net-next as well.
>>
>> Thanks,
>> Roi
>>
>
> Hi,
>
> I tested "[PATCH net] net, sched: respect rcu grace period on cls
> destruction" and could not reproduce my original issue.
Hi Roi,
Just so I'm 100% clear. No issue with just the above "respect rcu grace
period on cls destruction" per above statement.
> I rebased "[Patch net-next] net_sched: move the empty tp check from
> ->destroy() to ->delete()" over to test it in the same tree and got into
> a new trace in fl_delete.
In this case did you test with "net_sched: move the empty tp check from
->destroy() to ->delete()" _only_ or did this include both patches when
you see the error below.
>From my inspection we really need both patches to get correct behavior.
Thanks!
John
>
> [35659.012123] BUG: KASAN: wild-memory-access on address 1ffffffff803ca31
> [35659.020042] Write of size 1 by task ovs-vswitchd/20135
> [35659.025878] CPU: 19 PID: 20135 Comm: ovs-vswitchd Tainted:
> G O 4.9.0-rc3+ #18
> [35659.035948] Hardware name: HP ProLiant DL380p Gen8, BIOS P70 07/01/2015
> [35659.043730] Call Trace:
> [35659.046619] [<ffffffff95b6dc42>] dump_stack+0x63/0x81
> [35659.052456] [<ffffffff955fbbf8>] kasan_report_error+0x408/0x4e0
> [35659.059402] [<ffffffff955fc2e8>] kasan_report+0x58/0x60
> [35659.065428] [<ffffffff952d5e8d>] ? call_rcu_sched+0x1d/0x20
> [35659.072119] [<ffffffffc01e0701>] ? fl_destroy_filter+0x21/0x30
> [cls_flower]
> [35659.080217] [<ffffffffc01e1ccf>] ? fl_delete+0x1df/0x2e0 [cls_flower]
> [35659.087580] [<ffffffff955fa4ca>] __asan_store1+0x4a/0x50
> [35659.093697] [<ffffffffc01e1ccf>] fl_delete+0x1df/0x2e0 [cls_flower]
> [35659.100870] [<ffffffff9653ecba>] tc_ctl_tfilter+0x10da/0x1b90
>
>
> 0x1d02 is in fl_delete (net/sched/cls_flower.c:805).
> 800 struct cls_fl_filter *f = (struct cls_fl_filter *) arg;
> 801
> 802 rhashtable_remove_fast(&head->ht, &f->ht_node,
> 803 head->ht_params);
> 804 __fl_delete(tp, f);
> 805 *last = list_empty(&head->filters);
> 806 return 0;
> 807 }
>
>
> Thanks,
> Roi
^ 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