* Re: [PATCH] [RFC] net: dsa: mv88e6xxx: Pre-initialize err in mv88e6xxx_port_bridge_join()
From: Geert Uytterhoeven @ 2016-04-25 17:53 UTC (permalink / raw)
To: Vivien Didelot
Cc: David S. Miller, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <87vb35a942.fsf@ketchup.mtl.sfl>
Hi Vivien,
On Mon, Apr 25, 2016 at 7:31 PM, Vivien Didelot
<vivien.didelot@savoirfairelinux.com> wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>> On Mon, Apr 25, 2016 at 5:03 PM, Vivien Didelot
>> <vivien.didelot@savoirfairelinux.com> wrote:
>>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>>>> drivers/net/dsa/mv88e6xxx.c: In function ‘mv88e6xxx_port_bridge_join’:
>>>> drivers/net/dsa/mv88e6xxx.c:2184: warning: ‘err’ may be used uninitialized in this function
>>>
>>> Interesting, I don't have those warnings on 207afda1b5036009...
>>
>> It depends on the compiler version (still using 4.1.2) and options.
>>
>>>> If netdev_notifier_changeupper_info.upper_dev is ever NULL, the bridge
>>>> parameter will be NULL too, and the function will return an
>>>> uninitialized value.
>>>>
>>>> Pre-initialize err to zero to fix this.
>>>>
>>>> Fixes: 207afda1b5036009 ("net: dsa: mv88e6xxx: share the same default FDB")
>>>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>>> ---
>>>> Can this actually happen?
>>>
>>> bridge cannot be NULL here. Also ps->ports[port].bridge_dev is assigned
>>> to it before entering the for loop, so _mv88e6xxx_port_based_vlan_map
>>> will be called at least for this port.
>>
>> But there's no way the compiler can know that...
>
> Or maybe it can in new configurations. Anyway, this fix doesn't hurt,
> with a relevant commit message, I'd ack it.
What would you consider a relevant commit message?
Thanks!
Gr{oetje,eeting}s,
Geert
^ permalink raw reply
* Re: [PATCH v3 00/18] wcn36xx fixes
From: Kalle Valo @ 2016-04-25 17:56 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Eugene Krasnikov, Pontus Fuchs, linux-wireless, netdev,
linux-kernel
In-Reply-To: <1461042056-10607-1-git-send-email-bjorn.andersson@linaro.org>
Bjorn Andersson <bjorn.andersson@linaro.org> writes:
> The bulk of the following patches have been sitting in Eugene's Github tree for
> quite some time. They fix various issues existing in the mainline drivers, so
> they should be merged there too.
>
> Also included are two new fixes, of my own; the important one being the
> reordering of deletion of the bss, as this crashes the firmware on the
> Dragonbaord 410c (apq8016 with pronto & wcn3620).
>
> Lastly is a patch that adds a bunch of new capabilities found in the downstream
> driver.
>
> Changes since v2:
> - Restore BEACON_TEMPLATE_SIZE to not break UPDATE_PROBE_RSP_TEMPLATE_REQ
> - Added patch to correct WCN36XX_HAL_RMV_BSSKEY_RSP decoder
> - Added patch with missing capabilities from downstream
>
> Changes since v1:
> - Reorder patch 6 and 7 to not break the build temporarily
> - Inline fix from Jason Mobarak in the TIM PVM padding
>
> Bjorn Andersson (3):
> wcn36xx: Delete BSS before idling link
> wcn36xx: Correct remove bss key response encoding
> wcn36xx: Fill in capability list
>
> Pontus Fuchs (15):
> wcn36xx: Clean up wcn36xx_smd_send_beacon
> wcn36xx: Pad TIM PVM if needed
> wcn36xx: Add helper macros to cast vif to private vif and vice versa
> wcn36xx: Use consistent name for private vif
> wcn36xx: Use define for invalid index and fix typo
> wcn36xx: Add helper macros to cast sta to priv
> wcn36xx: Fetch private sta data from sta entry instead of from vif
> wcn36xx: Remove sta pointer in private vif struct
> wcn36xx: Parse trigger_ba response properly
> wcn36xx: Copy all members in config_sta v1 conversion
> wcn36xx: Use allocated self sta index instead of hard coded
> wcn36xx: Clear encrypt_type when deleting bss key
> wcn36xx: Track association state
> wcn36xx: Implement multicast filtering
> wcn36xx: Use correct command struct for EXIT_BMPS_REQ
All applied, thanks.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH v2 net-next] net: ethernet: enc28j60: add device tree support
From: Andrew F. Davis @ 2016-04-25 18:04 UTC (permalink / raw)
To: Michael Heimpold
Cc: Jonathan Cameron, Mark Brown, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala
In-Reply-To: <7030050.tME0DkDS0E@kerker>
On 04/25/2016 12:46 PM, Michael Heimpold wrote:
> Hi,
>
> Am Monday 25 April 2016, 10:39:41 schrieben Sie:
>> On 04/24/2016 04:28 PM, Michael Heimpold wrote:
>>> - eth_hw_addr_random(dev);
>>> +
>>> + macaddr = of_get_mac_address(spi->dev.of_node);
>>> + if (macaddr)
>>
>> You should also check if it is a valid MAC for Ethernet, recommend:
>>
>> if (macaddr && is_valid_ether_addr(macaddr))
>>
>
> But of_get_mac_address already takes care of this, see
> http://lxr.free-electrons.com/source/drivers/of/of_net.c#L45
> Also it already checks whether spi->dev.of_node is populated at all.
> It returns NULL in both error cases.
> So I prefered to omit both test here.
>
Ah, missed that, no problem here then.
Hmm, I wonder how many other drivers then do this check needlessly..
Time to fire-up Coccinelle :)
Andrew
--
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 v2 3/3] gre: allow creation of gretap interfaces in metadata mode
From: pravin shelar @ 2016-04-25 18:00 UTC (permalink / raw)
To: Jiri Benc
Cc: Linux Kernel Network Developers, Pravin B Shelar, Thomas Graf,
Simon Horman
In-Reply-To: <4917335385a75586c1f13b6a6d570d4cc5b0b132.1461495411.git.jbenc@redhat.com>
On Sun, Apr 24, 2016 at 4:00 AM, Jiri Benc <jbenc@redhat.com> wrote:
> The IFLA_GRE_REMOTE attribute does not make sense together with collect
> metadata and is ignored in such case. However, iproute2 always sets it; it
> will be zero if there's no remote address specified on the command line.
>
> Remove the check for non-zero IFLA_GRE_REMOTE when collect medata flag is
> set.
>
Rather than cover up in ip_gre kernel module, why not just fix
iproute2 to set the attribute correctly?
> Before the patch, this command returns failure, after the patch, it works as
> expected:
>
> ip link add gre1 type gretap external
>
> Fixes: 2e15ea390e6f4 ("ip_gre: Add support to collect tunnel metadata.")
> Signed-off-by: Jiri Benc <jbenc@redhat.com>
> ---
> New in v2.
> ---
> net/ipv4/ip_gre.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
^ permalink raw reply
* Re: [PATCH V3] net: stmmac: socfpga: Remove re-registration of reset controller
From: Joachim Eastwood @ 2016-04-25 18:11 UTC (permalink / raw)
To: Marek Vasut
Cc: netdev, peppe.cavallaro, alexandre.torgue, Matthew Gerlach,
Dinh Nguyen, David S . Miller
In-Reply-To: <1461240710-5381-1-git-send-email-marex@denx.de>
Hi Marek,
On 21 April 2016 at 14:11, Marek Vasut <marex@denx.de> wrote:
> Both socfpga_dwmac_parse_data() in dwmac-socfpga.c and stmmac_dvr_probe()
> in stmmac_main.c functions call devm_reset_control_get() to register an
> reset controller for the stmmac. This results in an attempt to register
> two reset controllers for the same non-shared reset line.
>
> The first attempt to register the reset controller works fine. The second
> attempt fails with warning from the reset controller core, see below.
> The warning is produced because the reset line is non-shared and thus
> it is allowed to have only up-to one reset controller associated with
> that reset line, not two or more.
>
> The solution has multiple parts. First, the original socfpga_dwmac_init()
> is tweaked to use reset controller pointer from the stmmac_priv (private
> data of the stmmac core) instead of the local instance, which was used
> before. The local re-registration of the reset controller is removed.
>
> Next, the socfpga_dwmac_init() is moved after stmmac_dvr_probe() in the
> probe function. This order is legal according to Altera and it makes the
> code much easier, since there is no need to temporarily register and
> unregister the reset controller ; the reset controller is already registered
> by the stmmac_dvr_probe().
>
> Finally, plat_dat->exit and socfpga_dwmac_exit() is no longer necessary,
> since the functionality is already performed by the stmmac core.
I am trying to rebase my changes on top of your two patches and
noticed a couple of things.
> static int socfpga_dwmac_init(struct platform_device *pdev, void *priv)
> {
> - struct socfpga_dwmac *dwmac = priv;
> + struct socfpga_dwmac *dwmac = priv;
> struct net_device *ndev = platform_get_drvdata(pdev);
> struct stmmac_priv *stpriv = NULL;
> int ret = 0;
>
> - if (ndev)
> - stpriv = netdev_priv(ndev);
> + if (!ndev)
> + return -EINVAL;
ndev can never be NULL here. socfpga_dwmac_init() is only called if
stmmac_dvr_probe() succeeds or we are running the resume callback. So
I don't see how this could ever be NULL.
> +
> + stpriv = netdev_priv(ndev);
It's not really nice to access 'stmmac_priv' as it should be private
to the core driver, but I don't see any other good solution right now.
> + if (!stpriv)
> + return -EINVAL;
>
> /* Assert reset to the enet controller before changing the phy mode */
> - if (dwmac->stmmac_rst)
> - reset_control_assert(dwmac->stmmac_rst);
> + if (stpriv->stmmac_rst)
> + reset_control_assert(stpriv->stmmac_rst);
>
> /* Setup the phy mode in the system manager registers according to
> * devicetree configuration
> @@ -227,8 +210,8 @@ static int socfpga_dwmac_init(struct platform_device *pdev, void *priv)
> /* Deassert reset for the phy configuration to be sampled by
> * the enet controller, and operation to start in requested mode
> */
> - if (dwmac->stmmac_rst)
> - reset_control_deassert(dwmac->stmmac_rst);
> + if (stpriv->stmmac_rst)
> + reset_control_deassert(stpriv->stmmac_rst);
>
> /* Before the enet controller is suspended, the phy is suspended.
> * This causes the phy clock to be gated. The enet controller is
> @@ -245,7 +228,7 @@ static int socfpga_dwmac_init(struct platform_device *pdev, void *priv)
> * control register 0, and can be modified by the phy driver
> * framework.
> */
> - if (stpriv && stpriv->phydev)
> + if (stpriv->phydev)
> phy_resume(stpriv->phydev);
Before this change phy_resume() was only called during driver resume
when , but your patches cause phy_resume() to called at probe time as
well. Is this okey?
regards,
Joachim Eastwood
^ permalink raw reply
* [PATCH] net: fix net_gso_ok for new GSO types.
From: Marcelo Ricardo Leitner @ 2016-04-25 18:13 UTC (permalink / raw)
To: netdev
Fix casting in net_gso_ok. Otherwise the shift on
gso_type << NETIF_F_GSO_SHIFT may hit the 32th bit and make it look like
a INT_MIN, which is then promoted from signed to uint64 which is
0xffffffff80000000, resulting in wrong behavior when it is and'ed with
the feature itself, as in:
This test app:
#include <stdio.h>
#include <stdint.h>
int main(int argc, char **argv)
{
uint64_t feature1;
uint64_t feature2;
int gso_type = 1 << 15;
feature1 = gso_type << 16;
feature2 = (uint64_t)gso_type << 16;
printf("%lx %lx\n", feature1, feature2);
return 0;
}
Gives:
ffffffff80000000 80000000
So that this:
return (features & feature) == feature;
Actually works on more bits than expected and invalid ones.
Fix is to promote it earlier.
Issue noted while rebasing SCTP GSO patch but posting separetely as
someone else may experience this meanwhile.
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
Dave I couldn't find a good way to indent this that wouldn't be uglier
than a bit long line, but let me know if you prefer otherwise.
include/linux/netdevice.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 1f6d5db471a24f7f23bbe425c359d64adb5dfd67..13ee05f71e3d74c9d2feef6c371547a4b2f82879 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3991,7 +3991,7 @@ netdev_features_t netif_skb_features(struct sk_buff *skb);
static inline bool net_gso_ok(netdev_features_t features, int gso_type)
{
- netdev_features_t feature = gso_type << NETIF_F_GSO_SHIFT;
+ netdev_features_t feature = (netdev_features_t)gso_type << NETIF_F_GSO_SHIFT;
/* check flags correspondence */
BUILD_BUG_ON(SKB_GSO_TCPV4 != (NETIF_F_TSO >> NETIF_F_GSO_SHIFT));
--
2.5.0
^ permalink raw reply related
* Re: [PATCH RFC] b43: stop hardcoding LED behavior
From: Lucas Stach @ 2016-04-25 18:21 UTC (permalink / raw)
To: Michael Büsch; +Cc: Kalle Valo, netdev, linux-wireless, b43-dev
In-Reply-To: <20160425175326.407eba27@wiggum>
Am Montag, den 25.04.2016, 17:53 +0200 schrieb Michael Büsch:
> On Mon, 25 Apr 2016 09:40:51 +0200
> Lucas Stach <dev@lynxeye.de> wrote:
>
> >
> > On my system the SPROM correctly defines the only wired LED (radio)
> > but
> > skips all others, leading to the hardcode to register LEDs with RX
> > and TX
> > triggers.
> Hm ok. It probably is a good idea to change the condition from
>
> if (sprom[led_index] == 0xFF)
>
> to
>
> if ((sprom[0] & sprom[1] & sprom[2] & sprom[3]) == 0xFF)
>
> So the hardcoding only happens if there is no LED configured in the
> SPROM. (I think my card does this (see below), but I can check that
> later)
>
>
> >
> > These triggers cause many uneccesary CPU wakeups to drive LEDs
> > that aren't even present in the system, reducing battery runtime.
>
> Numbers please. Did you measure that is actually causes more
> _wakeups_?
> How many?
> The led work is placed in the mac80211 workqueue and LED updates only
> happen on behalf of mac80211 activities (by default). It only causes
> additional wakeups, if there's nothing else scheduled on the
> workqueue
> anyways (which might well be the case. So we need numbers. :)
>
The blinking LEDs use a timer to enforce a constant blink rate at a
50ms on/off interval. While they are only triggered if there is some
RX/TX activity in the system, they cause up to 20 wakeups/second/led.
As the timers used for LED activity aren't deferrable, this hardcode is
causing 40 unnecessary CPU wakeups/s in my system.
>
> >
> > Remove the hardcode to stop it from doing any harm. If this code is
> > useful
> > for others it should probably be reworked as a quirk table
> > triggering only
> > for individual systems that need it.
>
> There are cards that need it. I don't know how many that are, but I
> own
> an older 4306 PC-Card card that needs this.
>
> So this effectively is a regression for this card.
>
> So I don't think this is acceptable.
> You should at least make this configurable via module parameter or
> such.
> Or maybe the change from above already is enough. It should work for
> your case.
>
There are some people that find those kinds of blinking LEDs
distracting, so a module parameter to disable them altogether might be
a good thing to have. Causing CPU wakeups in a system where those LEDs
aren't even physically populated is clearly undesired behavior.
If checking that the SPROM doesn't define any LED behavior is enough to
not regress your use case, I would be glad to rework the patch
accordingly.
Regards,
Lucas
>
> >
> > Signed-off-by: Lucas Stach <dev@lynxeye.de>
> > ---
> > drivers/net/wireless/broadcom/b43/leds.c | 26 ++----------------
> > --------
> > 1 file changed, 2 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/net/wireless/broadcom/b43/leds.c
> > b/drivers/net/wireless/broadcom/b43/leds.c
> > index d79ab2a..77d2dad 100644
> > --- a/drivers/net/wireless/broadcom/b43/leds.c
> > +++ b/drivers/net/wireless/broadcom/b43/leds.c
> > @@ -224,31 +224,9 @@ static void b43_led_get_sprominfo(struct
> > b43_wldev *dev,
> >
> > if (sprom[led_index] == 0xFF) {
> > /* There is no LED information in the SPROM
> > - * for this LED. Hardcode it here. */
> > + * for this LED. Keep it disabled. */
> > *activelow = false;
> > - switch (led_index) {
> > - case 0:
> > - *behaviour = B43_LED_ACTIVITY;
> > - *activelow = true;
> > - if (dev->dev->board_vendor ==
> > PCI_VENDOR_ID_COMPAQ)
> > - *behaviour = B43_LED_RADIO_ALL;
> > - break;
> > - case 1:
> > - *behaviour = B43_LED_RADIO_B;
> > - if (dev->dev->board_vendor ==
> > PCI_VENDOR_ID_ASUSTEK)
> > - *behaviour = B43_LED_ASSOC;
> > - break;
> > - case 2:
> > - *behaviour = B43_LED_RADIO_A;
> > - break;
> > - case 3:
> > - *behaviour = B43_LED_OFF;
> > - break;
> > - default:
> > - *behaviour = B43_LED_OFF;
> > - B43_WARN_ON(1);
> > - return;
> > - }
> > + *behaviour = B43_LED_OFF;
> > } else {
> > *behaviour = sprom[led_index] & B43_LED_BEHAVIOUR;
> > *activelow = !!(sprom[led_index] &
> > B43_LED_ACTIVELOW);
>
>
>
^ permalink raw reply
* [net-next PATCH 0/8] Fix Tunnel features and enable GSO partial for Mellanox adapters
From: Alexander Duyck @ 2016-04-25 18:30 UTC (permalink / raw)
To: talal, netdev, davem, galp, ogerlitz, eranbe
This patch series is meant to allow us to get the best performance possible
for Mellanox ConnectX-3/4 adapters in terms of VXLAN tunnels.
The first few patches address issues I found when just trying to collect
performance numbers. Specifically I was unable to get rates of any more
than 1 or 2 Mb/s if I was using a tunnel that ran over IPv6. In addition I
found a few other items related to GSO_PARTIAL and the TSO_MANGLEID that
needed to be addressed.
The last 4 patches go through and enable GSO_PARTIAL for tunnels that have
an outer checksum enabled, and then enable IPv6 support where we can. In
my tests I found that the mlx4 doesn't support outer IPv6 but the mlx5 did
so the code is updated to reflect that in the patches that enable IPv6
support.
---
Alexander Duyck (8):
net: Disable segmentation if checksumming is not supported
gso: Only allow GSO_PARTIAL if we can checksum the inner protocol
net: Fix netdev_fix_features so that TSO_MANGLEID is only available with TSO
vxlan: Add checksum check to the features check function
mlx4: Add support for UDP tunnel segmentation with outer checksum offload
mlx4: Add support for inner IPv6 checksum offloads and TSO
mlx5e: Add support for UDP tunnel segmentation with outer checksum offload
mlx5e: Fix IPv6 tunnel checksum offload
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 38 +++++++++++++++++----
drivers/net/ethernet/mellanox/mlx4/en_tx.c | 15 +++++++-
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 10 ++++--
include/linux/if_ether.h | 5 +++
include/net/vxlan.h | 4 ++
net/core/dev.c | 6 +++
net/core/skbuff.c | 6 ++-
7 files changed, 67 insertions(+), 17 deletions(-)
^ permalink raw reply
* [net-next PATCH 1/8] net: Disable segmentation if checksumming is not supported
From: Alexander Duyck @ 2016-04-25 18:31 UTC (permalink / raw)
To: talal, netdev, davem, galp, ogerlitz, eranbe
In-Reply-To: <20160425182442.11331.88349.stgit@ahduyck-xeon-server>
In the case of the mlx4 and mlx5 driver they do not support IPv6 checksum
offload for tunnels. With this being the case we should disable GSO in
addition to the checksum offload features when we find that a device cannot
perform a checksum on a given packet type.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
net/core/dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 6324bc9267f7..d6d9f286c4e1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2815,7 +2815,7 @@ static netdev_features_t harmonize_features(struct sk_buff *skb,
if (skb->ip_summed != CHECKSUM_NONE &&
!can_checksum_protocol(features, type)) {
- features &= ~NETIF_F_CSUM_MASK;
+ features &= ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
} else if (illegal_highdma(skb->dev, skb)) {
features &= ~NETIF_F_SG;
}
^ permalink raw reply related
* [net-next PATCH 2/8] gso: Only allow GSO_PARTIAL if we can checksum the inner protocol
From: Alexander Duyck @ 2016-04-25 18:31 UTC (permalink / raw)
To: talal, netdev, davem, galp, ogerlitz, eranbe
In-Reply-To: <20160425182442.11331.88349.stgit@ahduyck-xeon-server>
This patch addresses a possible issue that can occur if we get into any odd
corner cases where we support TSO for a given protocol but not the checksum
or scatter-gather offload. There are few drivers floating around that
setup their tunnels this way and by enforcing the checksum piece we can
avoid mangling any frames.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
net/core/skbuff.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 7ff7788b0151..d2871d081750 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3080,8 +3080,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
unsigned int headroom;
unsigned int len = head_skb->len;
__be16 proto;
- bool csum;
- int sg = !!(features & NETIF_F_SG);
+ bool csum, sg;
int nfrags = skb_shinfo(head_skb)->nr_frags;
int err = -ENOMEM;
int i = 0;
@@ -3093,13 +3092,14 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
if (unlikely(!proto))
return ERR_PTR(-EINVAL);
+ sg = !!(features & NETIF_F_SG);
csum = !!can_checksum_protocol(features, proto);
/* GSO partial only requires that we trim off any excess that
* doesn't fit into an MSS sized block, so take care of that
* now.
*/
- if (features & NETIF_F_GSO_PARTIAL) {
+ if (sg && csum && (features & NETIF_F_GSO_PARTIAL)) {
partial_segs = len / mss;
mss *= partial_segs;
}
^ permalink raw reply related
* [net-next PATCH 3/8] net: Fix netdev_fix_features so that TSO_MANGLEID is only available with TSO
From: Alexander Duyck @ 2016-04-25 18:31 UTC (permalink / raw)
To: talal, netdev, davem, galp, ogerlitz, eranbe
In-Reply-To: <20160425182442.11331.88349.stgit@ahduyck-xeon-server>
This change makes it so that we will strip the TSO_MANGLEID bit if TSO is
not present. This way we will also handle ECN correctly of TSO is not
present.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
net/core/dev.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index d6d9f286c4e1..6a5ef49ed1ab 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6720,6 +6720,10 @@ static netdev_features_t netdev_fix_features(struct net_device *dev,
features &= ~NETIF_F_TSO6;
}
+ /* TSO with IPv4 ID mangling requires IPv4 TSO be enabled */
+ if ((features & NETIF_F_TSO_MANGLEID) && !(features & NETIF_F_TSO))
+ features &= ~NETIF_F_TSO_MANGLEID;
+
/* TSO ECN requires that TSO is present as well. */
if ((features & NETIF_F_ALL_TSO) == NETIF_F_TSO_ECN)
features &= ~NETIF_F_TSO_ECN;
^ permalink raw reply related
* [net-next PATCH 4/8] vxlan: Add checksum check to the features check function
From: Alexander Duyck @ 2016-04-25 18:31 UTC (permalink / raw)
To: talal, netdev, davem, galp, ogerlitz, eranbe
In-Reply-To: <20160425182442.11331.88349.stgit@ahduyck-xeon-server>
We need to perform an additional check on the inner headers to determine if
we can offload the checksum for them. Previously this check didn't occur
so we would generate an invalid frame in the case of an IPv6 header
encapsulated inside of an IPv4 tunnel. To fix this I added a secondary
check to vxlan_features_check so that we can verify that we can offload the
inner checksum.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
include/linux/if_ether.h | 5 +++++
include/net/vxlan.h | 4 +++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/include/linux/if_ether.h b/include/linux/if_ether.h
index d5569734f672..548fd535fd02 100644
--- a/include/linux/if_ether.h
+++ b/include/linux/if_ether.h
@@ -28,6 +28,11 @@ static inline struct ethhdr *eth_hdr(const struct sk_buff *skb)
return (struct ethhdr *)skb_mac_header(skb);
}
+static inline struct ethhdr *inner_eth_hdr(const struct sk_buff *skb)
+{
+ return (struct ethhdr *)skb_inner_mac_header(skb);
+}
+
int eth_header_parse(const struct sk_buff *skb, unsigned char *haddr);
extern ssize_t sysfs_format_mac(char *buf, const unsigned char *addr, int len);
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 673e9f9e6da7..b8803165df91 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -317,7 +317,9 @@ static inline netdev_features_t vxlan_features_check(struct sk_buff *skb,
(skb->inner_protocol_type != ENCAP_TYPE_ETHER ||
skb->inner_protocol != htons(ETH_P_TEB) ||
(skb_inner_mac_header(skb) - skb_transport_header(skb) !=
- sizeof(struct udphdr) + sizeof(struct vxlanhdr))))
+ sizeof(struct udphdr) + sizeof(struct vxlanhdr)) ||
+ (skb->ip_summed != CHECKSUM_NONE &&
+ !can_checksum_protocol(features, inner_eth_hdr(skb)->h_proto))))
return features & ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
return features;
^ permalink raw reply related
* [net-next PATCH 5/8] mlx4: Add support for UDP tunnel segmentation with outer checksum offload
From: Alexander Duyck @ 2016-04-25 18:31 UTC (permalink / raw)
To: talal, netdev, davem, galp, ogerlitz, eranbe
In-Reply-To: <20160425182442.11331.88349.stgit@ahduyck-xeon-server>
This patch assumes that the mlx4 hardware will ignore existing IPv4/v6
header fields for length and checksum as well as the length and checksum
fields for outer UDP headers.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 8bd143dda95d..bce37cbfde24 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2358,7 +2358,9 @@ out:
/* set offloads */
priv->dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
- NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL;
+ NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL |
+ NETIF_F_GSO_UDP_TUNNEL_CSUM |
+ NETIF_F_GSO_PARTIAL;
}
static void mlx4_en_del_vxlan_offloads(struct work_struct *work)
@@ -2368,7 +2370,9 @@ static void mlx4_en_del_vxlan_offloads(struct work_struct *work)
vxlan_del_task);
/* unset offloads */
priv->dev->hw_enc_features &= ~(NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
- NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL);
+ NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL |
+ NETIF_F_GSO_UDP_TUNNEL_CSUM |
+ NETIF_F_GSO_PARTIAL);
ret = mlx4_SET_PORT_VXLAN(priv->mdev->dev, priv->port,
VXLAN_STEER_BY_OUTER_MAC, 0);
@@ -2992,8 +2996,13 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
}
if (mdev->dev->caps.tunnel_offload_mode == MLX4_TUNNEL_OFFLOAD_MODE_VXLAN) {
- dev->hw_features |= NETIF_F_GSO_UDP_TUNNEL;
- dev->features |= NETIF_F_GSO_UDP_TUNNEL;
+ dev->hw_features |= NETIF_F_GSO_UDP_TUNNEL |
+ NETIF_F_GSO_UDP_TUNNEL_CSUM |
+ NETIF_F_GSO_PARTIAL;
+ dev->features |= NETIF_F_GSO_UDP_TUNNEL |
+ NETIF_F_GSO_UDP_TUNNEL_CSUM |
+ NETIF_F_GSO_PARTIAL;
+ dev->gso_partial_features = NETIF_F_GSO_UDP_TUNNEL_CSUM;
}
mdev->pndev[port] = dev;
^ permalink raw reply related
* [net-next PATCH 6/8] mlx4: Add support for inner IPv6 checksum offloads and TSO
From: Alexander Duyck @ 2016-04-25 18:31 UTC (permalink / raw)
To: talal, netdev, davem, galp, ogerlitz, eranbe
In-Reply-To: <20160425182442.11331.88349.stgit@ahduyck-xeon-server>
>From what I can tell the ConnectX-3 will support an inner IPv6 checksum and
segmentation offload, however it cannot support outer IPv6 headers. For
this reason I am adding the feature to the hw_enc_features and adding an
extra check to the features_check call that will disable GSO and checksum
offload in the case that the encapsulated frame has an outer IP version of
that is not 4.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 25 +++++++++++++++++++-----
drivers/net/ethernet/mellanox/mlx4/en_tx.c | 15 ++++++++++++--
2 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index bce37cbfde24..6f28ac58251c 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2357,8 +2357,10 @@ out:
}
/* set offloads */
- priv->dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
- NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL |
+ priv->dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
+ NETIF_F_RXCSUM |
+ NETIF_F_TSO | NETIF_F_TSO6 |
+ NETIF_F_GSO_UDP_TUNNEL |
NETIF_F_GSO_UDP_TUNNEL_CSUM |
NETIF_F_GSO_PARTIAL;
}
@@ -2369,8 +2371,10 @@ static void mlx4_en_del_vxlan_offloads(struct work_struct *work)
struct mlx4_en_priv *priv = container_of(work, struct mlx4_en_priv,
vxlan_del_task);
/* unset offloads */
- priv->dev->hw_enc_features &= ~(NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
- NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL |
+ priv->dev->hw_enc_features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
+ NETIF_F_RXCSUM |
+ NETIF_F_TSO | NETIF_F_TSO6 |
+ NETIF_F_GSO_UDP_TUNNEL |
NETIF_F_GSO_UDP_TUNNEL_CSUM |
NETIF_F_GSO_PARTIAL);
@@ -2431,7 +2435,18 @@ static netdev_features_t mlx4_en_features_check(struct sk_buff *skb,
netdev_features_t features)
{
features = vlan_features_check(skb, features);
- return vxlan_features_check(skb, features);
+ features = vxlan_features_check(skb, features);
+
+ /* The ConnectX-3 doesn't support outer IPv6 checksums but it does
+ * support inner IPv6 checksums and segmentation so we need to
+ * strip that feature if this is an IPv6 encapsulated frame.
+ */
+ if (skb->encapsulation &&
+ (skb->ip_summed == CHECKSUM_PARTIAL) &&
+ (ip_hdr(skb)->version != 4))
+ features &= ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
+
+ return features;
}
#endif
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index c0d7b7296236..c9f5388ea22a 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -41,6 +41,7 @@
#include <linux/vmalloc.h>
#include <linux/tcp.h>
#include <linux/ip.h>
+#include <linux/ipv6.h>
#include <linux/moduleparam.h>
#include "mlx4_en.h"
@@ -918,8 +919,18 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
tx_ind, fragptr);
if (skb->encapsulation) {
- struct iphdr *ipv4 = (struct iphdr *)skb_inner_network_header(skb);
- if (ipv4->protocol == IPPROTO_TCP || ipv4->protocol == IPPROTO_UDP)
+ union {
+ struct iphdr *v4;
+ struct ipv6hdr *v6;
+ unsigned char *hdr;
+ } ip;
+ u8 proto;
+
+ ip.hdr = skb_inner_network_header(skb);
+ proto = (ip.v4->version == 4) ? ip.v4->protocol :
+ ip.v6->nexthdr;
+
+ if (proto == IPPROTO_TCP || proto == IPPROTO_UDP)
op_own |= cpu_to_be32(MLX4_WQE_CTRL_IIP | MLX4_WQE_CTRL_ILP);
else
op_own |= cpu_to_be32(MLX4_WQE_CTRL_IIP);
^ permalink raw reply related
* [net-next PATCH 7/8] mlx5e: Add support for UDP tunnel segmentation with outer checksum offload
From: Alexander Duyck @ 2016-04-25 18:31 UTC (permalink / raw)
To: talal, netdev, davem, galp, ogerlitz, eranbe
In-Reply-To: <20160425182442.11331.88349.stgit@ahduyck-xeon-server>
This patch assumes that the mlx5 hardware will ignore existing IPv4/v6
header fields for length and checksum as well as the length and checksum
fields for outer UDP headers.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index d485d1e4e100..a687d6624c8e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -2580,13 +2580,18 @@ static void mlx5e_build_netdev(struct net_device *netdev)
netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
if (mlx5e_vxlan_allowed(mdev)) {
- netdev->hw_features |= NETIF_F_GSO_UDP_TUNNEL;
+ netdev->hw_features |= NETIF_F_GSO_UDP_TUNNEL |
+ NETIF_F_GSO_UDP_TUNNEL_CSUM |
+ NETIF_F_GSO_PARTIAL;
netdev->hw_enc_features |= NETIF_F_IP_CSUM;
netdev->hw_enc_features |= NETIF_F_RXCSUM;
netdev->hw_enc_features |= NETIF_F_TSO;
netdev->hw_enc_features |= NETIF_F_TSO6;
netdev->hw_enc_features |= NETIF_F_RXHASH;
netdev->hw_enc_features |= NETIF_F_GSO_UDP_TUNNEL;
+ netdev->hw_enc_features |= NETIF_F_GSO_UDP_TUNNEL_CSUM |
+ NETIF_F_GSO_PARTIAL;
+ netdev->gso_partial_features = NETIF_F_GSO_UDP_TUNNEL_CSUM;
}
netdev->features = netdev->hw_features;
^ permalink raw reply related
* [net-next PATCH 8/8] mlx5e: Fix IPv6 tunnel checksum offload
From: Alexander Duyck @ 2016-04-25 18:31 UTC (permalink / raw)
To: talal, netdev, davem, galp, ogerlitz, eranbe
In-Reply-To: <20160425182442.11331.88349.stgit@ahduyck-xeon-server>
The mlx5 driver exposes support for TSO6 but not IPv6 csum for hardware
encapsulated tunnels. This leads to issues as it triggers warnings in
skb_checksum_help as it ends up being called as we report supporting the
segmentation but not the checksumming for IPv6 frames.
This patch corrects that and drops 2 features that don't actually need to
be supported in hw_enc_features since they are Rx features and don't
actually impact anything by being present in hw_enc_features.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index a687d6624c8e..4728abd46cb2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -2584,10 +2584,9 @@ static void mlx5e_build_netdev(struct net_device *netdev)
NETIF_F_GSO_UDP_TUNNEL_CSUM |
NETIF_F_GSO_PARTIAL;
netdev->hw_enc_features |= NETIF_F_IP_CSUM;
- netdev->hw_enc_features |= NETIF_F_RXCSUM;
+ netdev->hw_enc_features |= NETIF_F_IPV6_CSUM;
netdev->hw_enc_features |= NETIF_F_TSO;
netdev->hw_enc_features |= NETIF_F_TSO6;
- netdev->hw_enc_features |= NETIF_F_RXHASH;
netdev->hw_enc_features |= NETIF_F_GSO_UDP_TUNNEL;
netdev->hw_enc_features |= NETIF_F_GSO_UDP_TUNNEL_CSUM |
NETIF_F_GSO_PARTIAL;
^ permalink raw reply related
* Re: [PATCH RFC] b43: stop hardcoding LED behavior
From: Michael Büsch @ 2016-04-25 18:32 UTC (permalink / raw)
To: Lucas Stach; +Cc: Kalle Valo, netdev, linux-wireless, b43-dev
In-Reply-To: <1461608496.2364.36.camel@lynxeye.de>
[-- Attachment #1: Type: text/plain, Size: 1972 bytes --]
On Mon, 25 Apr 2016 20:21:36 +0200
Lucas Stach <dev@lynxeye.de> wrote:
> > Numbers please. Did you measure that is actually causes more
> > _wakeups_?
> > How many?
> > The led work is placed in the mac80211 workqueue and LED updates only
> > happen on behalf of mac80211 activities (by default). It only causes
> > additional wakeups, if there's nothing else scheduled on the
> > workqueue
> > anyways (which might well be the case. So we need numbers. :)
> >
> The blinking LEDs use a timer to enforce a constant blink rate at a
> 50ms on/off interval. While they are only triggered if there is some
> RX/TX activity in the system, they cause up to 20 wakeups/second/led.
> As the timers used for LED activity aren't deferrable, this hardcode is
> causing 40 unnecessary CPU wakeups/s in my system.
Ok this is 40 to 40k for the interrupt requests?
We need real measured numbers and a percentage on how much the b43 LEDs
increase the system wakeups in relation to all other wakeups.
> There are some people that find those kinds of blinking LEDs
> distracting,
Those can already disable them via the standard LED framework.
> so a module parameter to disable them altogether might be
> a good thing to have.
No. We have a standard API for this.
> Causing CPU wakeups in a system where those LEDs
> aren't even physically populated is clearly undesired behavior.
Yes, but this is not going to be fixed via regressions.
> If checking that the SPROM doesn't define any LED behavior is enough to
> not regress your use case, I would be glad to rework the patch
> accordingly.
As it turns out I don't have that card here and I don't have a dump of
its SPROM as I expected. So I cannot really verify this. But I'm pretty
sure that this card did not define any LEDs in its SPROM at all.
I'm not aware of any card that only partially defines LEDs in the
SPROM. So that fix would be OK.
--
Michael
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] [RFC] net: dsa: mv88e6xxx: Pre-initialize err in mv88e6xxx_port_bridge_join()
From: Vivien Didelot @ 2016-04-25 18:35 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: David S. Miller, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <CAMuHMdVfYTx3CCa0GnqmcWHBQPYZZUYZQz1hOOaUjgXFmhVP3w@mail.gmail.com>
Hi Geert,
Geert Uytterhoeven <geert@linux-m68k.org> writes:
> Hi Vivien,
>
> On Mon, Apr 25, 2016 at 7:31 PM, Vivien Didelot
> <vivien.didelot@savoirfairelinux.com> wrote:
>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>>> On Mon, Apr 25, 2016 at 5:03 PM, Vivien Didelot
>>> <vivien.didelot@savoirfairelinux.com> wrote:
>>>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>>>>> drivers/net/dsa/mv88e6xxx.c: In function ‘mv88e6xxx_port_bridge_join’:
>>>>> drivers/net/dsa/mv88e6xxx.c:2184: warning: ‘err’ may be used uninitialized in this function
>>>>
>>>> Interesting, I don't have those warnings on 207afda1b5036009...
>>>
>>> It depends on the compiler version (still using 4.1.2) and options.
>>>
>>>>> If netdev_notifier_changeupper_info.upper_dev is ever NULL, the bridge
>>>>> parameter will be NULL too, and the function will return an
>>>>> uninitialized value.
>>>>>
>>>>> Pre-initialize err to zero to fix this.
>>>>>
>>>>> Fixes: 207afda1b5036009 ("net: dsa: mv88e6xxx: share the same default FDB")
>>>>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>>>> ---
>>>>> Can this actually happen?
>>>>
>>>> bridge cannot be NULL here. Also ps->ports[port].bridge_dev is assigned
>>>> to it before entering the for loop, so _mv88e6xxx_port_based_vlan_map
>>>> will be called at least for this port.
>>>
>>> But there's no way the compiler can know that...
>>
>> Or maybe it can in new configurations. Anyway, this fix doesn't hurt,
>> with a relevant commit message, I'd ack it.
>
> What would you consider a relevant commit message?
bridge being NULL is not the reason why err can eventually get returned
uninitialized. The GCC version would be great too, I have no such
warning here with 5.3.0.
Thanks,
-v
^ permalink raw reply
* Re: [PATCH v2 net-next] net: ethernet: enc28j60: add device tree support
From: Michael Heimpold @ 2016-04-25 18:41 UTC (permalink / raw)
To: Rob Herring
Cc: Jonathan Cameron, Andrew F . Davis, Mark Brown, netdev,
devicetree, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala
In-Reply-To: <20160425132326.GA17445@rob-hp-laptop>
Hi,
Am Monday 25 April 2016, 08:23:26 schrieb Rob Herring:
> On Sun, Apr 24, 2016 at 11:28:03PM +0200, Michael Heimpold wrote:
> > The following patch adds the required match table for device tree support
> > (and while at, fix the indent). It's also possible to specify the
> > MAC address in the DT blob.
> >
> > Also add the corresponding binding documentation file.
> >
> > Signed-off-by: Michael Heimpold <mhei@heimpold.de>
> > ---
> >
> > v2: * took care of Arnd Bergmann's review comments
> >
> > - allow to specify MAC address via DT
> > - unconditionally define DT id table
> >
> > * increased the driver version minor number
> > * driver author's email address bounces, removed from address list
> >
> > .../devicetree/bindings/net/microchip-enc28j60.txt | 50
> > ++++++++++++++++++++++
> Matching the compatible string is preferred here: microchip,enc28j60.txt
>
Ok, will change in next round.
> > drivers/net/ethernet/microchip/enc28j60.c | 20 +++++++--
> > 2 files changed, 67 insertions(+), 3 deletions(-)
> > create mode 100644
> > Documentation/devicetree/bindings/net/microchip-enc28j60.txt>
> > diff --git a/Documentation/devicetree/bindings/net/microchip-enc28j60.txt
> > b/Documentation/devicetree/bindings/net/microchip-enc28j60.txt new file
> > mode 100644
> > index 0000000..847a97b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/microchip-enc28j60.txt
> > @@ -0,0 +1,50 @@
> > +* Microchip ENC28J60
> > +
> > +This is a standalone 10 MBit ethernet controller with SPI interface.
> > +
> > +For each device connected to a SPI bus, define a child node within
> > +the SPI master node.
> > +
> > +Required properties:
> > +- compatible: Should be "microchip,enc28j60"
> > +- reg: Specify the SPI chip select the ENC28J60 is wired to
> > +- interrupts: Specify the interrupt and interrupt type (usually falling
> > edge) +
> > +Optional properties:
> > +- interrupt-parent: Specify the pHandle of the source interrupt
>
> This is required in the sense that either the node or a parent node must
> define it. In this case, since the SPI controller likely has a different
> parent, you will pretty much always need it defined in this node.
Understand. I think I should then move it to the required properties?
BTW: I orientated on the qca-qca7000-spi.txt as this is a SPI ethernet
device, too. Maybe we should also clarify this topic for this binding...
> > +- spi-max-frequency: Maximum frequency of the SPI bus when accessing the
> > ENC28J60. + According to the ENC28J80 datasheet, the chip allows a
> > maximum of 20 MHz, however, + board designs may need to limit this
> > value.
> > +- local-mac-address: See ethernet.txt in the same directory.
> > +
> > +
> > +Example (for NXP i.MX28 with pin control stuff for GPIO irq):
> > +
> > + ssp2: ssp@80014000 {
> > + compatible = "fsl,imx28-spi";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&spi2_pins_b &spi2_sck_cfg>;
> > + status = "okay";
> > +
> > + enc28j60: ethernet@0 {
> > + compatible = "microchip,enc28j60";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&enc28j60_pins>;
>
> Need to document using the pinctrl binding.
Would it be enough, when I list both properties above and refer to
the pinctrl binding?
I wondered whether there are other possibilities to wire the irq,
but I guess that using GPIO line is the usual way to do, so I'd add
this to required properties, too.
Thanks for your feedback, regards
Michael
>
> > + reg = <0>;
> > + interrupt-parent = <&gpio3>;
> > + interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
> > + spi-max-frequency = <12000000>;
> > + };
> > + };
> > +
^ permalink raw reply
* Re: [PATCH net-next 0/9] netlink: align attributes when needed (patchset #2)
From: David Miller @ 2016-04-25 19:09 UTC (permalink / raw)
To: nicolas.dichtel
Cc: netdev, jhs, stephen, pshelar, aar, linux-wpan, wensong, horms,
ja, pablo, kaber, kadlec, lvs-devel, netfilter-devel, johannes,
linux-wireless
In-Reply-To: <1461572722-6029-1-git-send-email-nicolas.dichtel@6wind.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Mon, 25 Apr 2016 10:25:13 +0200
> This is the continuation (series #2) of the work done to align netlink
> attributes when these attributes contain some 64-bit fields.
>
> In patch #3, I didn't modify the function ila_encap_nlsize(). I was waiting
> feedback for this patch: http://patchwork.ozlabs.org/patch/613766/
> If it's approved, there will be an update to switch nla_total_size() to
> nla_total_size_64bit() after the merge of net in net-next.
Looks good, series applied, thanks!
^ permalink raw reply
* Re: [PATCH net-next] macvlan: fix failure during registration v3
From: Eric W. Biederman @ 2016-04-25 19:00 UTC (permalink / raw)
To: Francesco Ruggeri; +Cc: netdev, David S. Miller, Mahesh Bandewar
In-Reply-To: <1461449012-7401-1-git-send-email-fruggeri@arista.com>
Francesco Ruggeri <fruggeri@arista.com> writes:
> If macvlan_common_newlink fails in register_netdevice after macvlan_init
> then it decrements port->count twice, first in macvlan_uninit (from
> register_netdevice or rollback_registered) and then again in
> macvlan_common_newlink.
> A similar problem may exist in the ipvlan driver.
> This patch consolidates modifications to port->count into macvlan_init
> and macvlan_uninit (thanks to Eric Biederman for suggesting this approach).
>
> v3: remove macvtap specific bits.
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
> Signed-off-by: Francesco Ruggeri <fruggeri@arista.com>
> ---
> drivers/net/macvlan.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index 2bcf1f3..cb01023 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -795,6 +795,7 @@ static int macvlan_init(struct net_device *dev)
> {
> struct macvlan_dev *vlan = netdev_priv(dev);
> const struct net_device *lowerdev = vlan->lowerdev;
> + struct macvlan_port *port = vlan->port;
>
> dev->state = (dev->state & ~MACVLAN_STATE_MASK) |
> (lowerdev->state & MACVLAN_STATE_MASK);
> @@ -812,6 +813,8 @@ static int macvlan_init(struct net_device *dev)
> if (!vlan->pcpu_stats)
> return -ENOMEM;
>
> + port->count += 1;
> +
> return 0;
> }
>
> @@ -1312,10 +1315,9 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
> return err;
> }
>
> - port->count += 1;
> err = register_netdevice(dev);
> if (err < 0)
> - goto destroy_port;
> + return err;
>
> dev->priv_flags |= IFF_MACVLAN;
> err = netdev_upper_dev_link(lowerdev, dev);
> @@ -1330,10 +1332,6 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
>
> unregister_netdev:
> unregister_netdevice(dev);
> -destroy_port:
> - port->count -= 1;
> - if (!port->count)
> - macvlan_port_destroy(lowerdev);
>
> return err;
> }
^ permalink raw reply
* Re: [PATCH net-next] macvtap: check minor when unregistering
From: Eric W. Biederman @ 2016-04-25 19:01 UTC (permalink / raw)
To: Francesco Ruggeri; +Cc: netdev, David S. Miller
In-Reply-To: <1461449071-7499-1-git-send-email-fruggeri@arista.com>
Francesco Ruggeri <fruggeri@arista.com> writes:
> macvtap_device_event(NETDEV_UNREGISTER) should check vlan->minor to
> determine if it is being invoked in the context of a macvtap_newlink
> that failed, for example in this code sequence:
>
> macvtap_newlink
> macvlan_common_newlink
> register_netdevice
> call_netdevice_notifiers(NETDEV_REGISTER, dev)
> macvtap_device_event(NETDEV_REGISTER)
> <fail here, vlan->minor = 0>
> rollback_registered(dev);
> rollback_registered_many
> call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
> macvtap_device_event(NETDEV_UNREGISTER)
> <nothing to clean up here>
>
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
> Signed-off-by: Francesco Ruggeri <fruggeri@arista.com>
> ---
> drivers/net/macvtap.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index 95394ed..74cb15a 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -1303,6 +1303,9 @@ static int macvtap_device_event(struct notifier_block *unused,
> }
> break;
> case NETDEV_UNREGISTER:
> + /* vlan->minor == 0 if NETDEV_REGISTER above failed */
> + if (vlan->minor == 0)
> + break;
> devt = MKDEV(MAJOR(macvtap_major), vlan->minor);
> device_destroy(macvtap_class, devt);
> macvtap_free_minor(vlan);
^ permalink raw reply
* Re: [PATCH net v2 2/3] drivers: net: cpsw: fix error messages when using phy-handle DT property
From: Grygorii Strashko @ 2016-04-25 19:12 UTC (permalink / raw)
To: David Rivshin (Allworx), Rob Herring
Cc: netdev, linux-omap, linux-arm-kernel, devicetree, linux-kernel,
David Miller, Mugunthan V N, Andrew Goodbody, Markus Brunner,
Nicolas Chauvet
In-Reply-To: <20160422114545.57b477ad.drivshin.allworx@gmail.com>
On 04/22/2016 06:45 PM, David Rivshin (Allworx) wrote:
> On Fri, 22 Apr 2016 16:03:34 +0300
> Grygorii Strashko <grygorii.strashko@ti.com> wrote:
>
>> On 04/21/2016 09:26 PM, David Rivshin (Allworx) wrote:
>>> From: David Rivshin <drivshin@allworx.com>
>>>
>>> The phy-handle, phy_id, and fixed-link properties are mutually exclusive,
>>> and only one need be specified. However if phy-handle was specified, an
>>> error message would complain about the lack of phy_id or fixed-link.
I think, commit message need to be updated.
You not only fix log messages - you also fix the issue with
of_get_phy_mode(slave_node); which will not be called if phy-handle is used.
slave_data->phy_if = of_get_phy_mode(slave_node);
^ see below
>>>
>>> Also, if phy-handle was specified and the subsequent of_phy_connect()
>>> failed, the error message still referenced slaved->data->phy_id, which
>>> would be empty. Instead, use the name of the device_node as a useful
>>> identifier.
>>>
>>> Fixes: 9e42f715264f ("drivers: net: cpsw: add phy-handle parsing")
>>> Signed-off-by: David Rivshin <drivshin@allworx.com>
>>> Acked-by: Rob Herring <robh@kernel.org>
>>> Tested-by: Nicolas Chauvet <kwizart@gmail.com>
>>> ---
>>> If would like this for -stable it should apply cleanly as far back
>>> as 4.5. It failes on 4.4 due to some context differences, but can be
>>> applied with 'git am -C2'. Or, I can produce a separate patch against
>>> linux-4.4.y if preferred.
>>>
>>> Changes since v1 [1]:
>>> - Rebased (no conflicts)
>>> - Added Tested-by from Nicolas Chauvet
>>> - Added Acked-by from Rob Herring for the binding change
>>>
>>> [1] https://patchwork.ozlabs.org/patch/560324/
>>>
>>> Documentation/devicetree/bindings/net/cpsw.txt | 4 ++--
>>> drivers/net/ethernet/ti/cpsw.c | 17 +++++++++++++----
>>> 2 files changed, 15 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
>>> index 28a4781..3033c0f 100644
>>> --- a/Documentation/devicetree/bindings/net/cpsw.txt
>>> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
>>> @@ -46,16 +46,16 @@ Optional properties:
>>> - dual_emac_res_vlan : Specifies VID to be used to segregate the ports
>>> - mac-address : See ethernet.txt file in the same directory
>>> - phy_id : Specifies slave phy id
>>
>> May be the "phy_id" can be marked as deprecated? (while here)
>> The recommended property now is "phy-handle".
>
> I can certainly do that. Perhaps something like this?
> - phy_id : Specifies slave phy id (deprecated, use phy-handle)
>
> Rob, would you have any issues with bundling that?
>
>>
>>> - phy-handle : See ethernet.txt file in the same directory
>>>
>>> Slave sub-nodes:
>>> - fixed-link : See fixed-link.txt file in the same directory
>>> - Either the property phy_id, or the sub-node
>>> - fixed-link can be specified
>>> +
>>> +Note: Exactly one of phy_id, phy-handle, or fixed-link must be specified.
>>>
>>> Note: "ti,hwmods" field is used to fetch the base address and irq
>>> resources from TI, omap hwmod data base during device registration.
>>> Future plan is to migrate hwmod data base contents into device tree
>>> blob so that, all the required data will be used from device tree dts
>>> file.
>>>
>>> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
>>> index d69cb3f..3c81413 100644
>>> --- a/drivers/net/ethernet/ti/cpsw.c
>>> +++ b/drivers/net/ethernet/ti/cpsw.c
>>> @@ -1150,16 +1150,19 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
>>> if (slave->data->phy_node)
>>> slave->phy = of_phy_connect(priv->ndev, slave->data->phy_node,
>>> &cpsw_adjust_link, 0, slave->data->phy_if);
>>> else
>>> slave->phy = phy_connect(priv->ndev, slave->data->phy_id,
>>> &cpsw_adjust_link, slave->data->phy_if);
>>> if (IS_ERR(slave->phy)) {
>>> - dev_err(priv->dev, "phy %s not found on slave %d\n",
>>> - slave->data->phy_id, slave->slave_num);
>>> + dev_err(priv->dev, "phy \"%s\" not found on slave %d\n",
>>> + slave->data->phy_node ?
>>> + slave->data->phy_node->full_name :
>>> + slave->data->phy_id,
>>> + slave->slave_num);
>>
>> Unfortunately, there are some inconsistency between legacy and FDT API :(
>> of_phy_connect() will return valid phy_device or NULL, but phy_connect()
>> can return valid phy_device or ERR_PTR().
>
> Good catch, I hadn't noticed that. It looks like that's actually a more
> serious (pre-existing) bug: if of_phy_connect() returns NULL, we'd end
> up dereferencing it and pagefaulting.
>
> How about moving the IS_ERR() check into the phy_connect() case like this:
> if (slave->data->phy_node) {
> slave->phy = of_phy_connect(priv->ndev, slave->data->phy_node,
> &cpsw_adjust_link, 0, slave->data->phy_if);
[1]
> } else {
> slave->phy = phy_connect(priv->ndev, slave->data->phy_id,
> &cpsw_adjust_link, slave->data->phy_if);
> if (IS_ERR(slave->phy))
> slave->phy = NULL;
[2]
> }
> if (!slave->phy) {
> dev_err(priv->dev, "phy \"%s\" not found on slave %d\n",
> slave->data->phy_node ?
> slave->data->phy_node->full_name :
> slave->data->phy_id,
> slave->slave_num);
> } else {
>
> Since you say the phy_id case is deprecated anyways, I'm not too concerned
> about not printing the error code returned by phy_connect() in that case
> (especially since it never did so in the past). That lets us still avoid
> duplicating the dev_err() itself.
I'm not worry too much about duplicating dev_err() - it's always good to know
the reason of failure.
So, may be for of_phy_connect() [1]:
dev_err(priv->dev, "phy \"%s\" not found on slave %d\n",
slave->data->phy_node->full_name,
slave->slave_num);
and for phy_connect() [2]:
dev_err(priv->dev, "phy %s not found on slave %d, err %d\n",
slave->data->phy_id, slave->slave_num, PTR_ERR(slave->phy));
Mugunthan, any comments?
>
>
>>
>>
>>
>>> slave->phy = NULL;
>>> } else {
>>> phy_attached_info(slave->phy);
>>>
>>> phy_start(slave->phy);
>>>
>>> /* Configure GMII_SEL register */
>>> @@ -2030,15 +2033,19 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
>>> /* This is no slave child node, continue */
>>> if (strcmp(slave_node->name, "slave"))
>>> continue;
>>>
>>> slave_data->phy_node = of_parse_phandle(slave_node,
>>> "phy-handle", 0);
>>> parp = of_get_property(slave_node, "phy_id", &lenp);
>>> - if (of_phy_is_fixed_link(slave_node)) {
>>> + if (slave_data->phy_node) {
>>> + dev_dbg(&pdev->dev,
>>> + "slave[%d] using phy-handle=\"%s\"\n",
>>> + i, slave_data->phy_node->full_name);
>>> + } else if (of_phy_is_fixed_link(slave_node)) {
>>> struct device_node *phy_node;
>>> struct phy_device *phy_dev;
>>>
>>> /* In the case of a fixed PHY, the DT node associated
>>> * to the PHY is the Ethernet MAC DT node.
>>> */
>>> ret = of_phy_register_fixed_link(slave_node);
>>> @@ -2067,15 +2074,17 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
>>> if (!mdio) {
>>> dev_err(&pdev->dev, "Missing mdio platform device\n");
>>> return -EINVAL;
>>> }
>>> snprintf(slave_data->phy_id, sizeof(slave_data->phy_id),
>>> PHY_ID_FMT, mdio->name, phyid);
>>> } else {
>>> - dev_err(&pdev->dev, "No slave[%d] phy_id or fixed-link property\n", i);
>>> + dev_err(&pdev->dev,
>>> + "No slave[%d] phy_id, phy-handle, or fixed-link property\n",
>>> + i);
>>> goto no_phy_slave;
>>> }
>>> slave_data->phy_if = of_get_phy_mode(slave_node);
Your change will allow the code to reach this point in case of phy-handle.
>>> if (slave_data->phy_if < 0) {
>>> dev_err(&pdev->dev, "Missing or malformed slave[%d] phy-mode property\n",
>>> i);
>>> return slave_data->phy_if;
>>>
>>
>>
--
regards,
-grygorii
^ permalink raw reply
* Re: [net-next 00/15][pull request] 10GbE Intel Wired LAN Driver Updates 2016-04-25
From: David Miller @ 2016-04-25 19:12 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene, john.ronciak
In-Reply-To: <1461588269-128728-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 25 Apr 2016 05:44:14 -0700
> This series contains updates to ixgbe and ixgbevf.
Pulled, thanks Jeff.
^ permalink raw reply
* Re: [PATCH net v2 1/3] drivers: net: cpsw: fix parsing of phy-handle DT property in dual_emac config
From: Grygorii Strashko @ 2016-04-25 19:14 UTC (permalink / raw)
To: David Rivshin (Allworx), netdev-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Miller, Mugunthan V N,
Andrew Goodbody, Markus Brunner, Nicolas Chauvet
In-Reply-To: <571A210E.2020607-l0cyMroinI0@public.gmane.org>
On 04/22/2016 04:03 PM, Grygorii Strashko wrote:
> On 04/21/2016 09:19 PM, David Rivshin (Allworx) wrote:
>> From: David Rivshin <drivshin-5fOYsn7Fw8lBDgjK7y7TUQ@public.gmane.org>
>>
>> Commit 9e42f715264ff158478fa30eaed847f6e131366b ("drivers: net: cpsw: add
>> phy-handle parsing") saved the "phy-handle" phandle into a new cpsw_priv
>> field. However, phy connections are per-slave, so the phy_node field
>> should
>> be in cpsw_slave_data rather than cpsw_priv.
>>
>> This would go unnoticed in a single emac configuration. But in dual_emac
>> mode, the last "phy-handle" property parsed for either slave would be
>> used
>> by both of them, causing them both to refer to the same phy_device.
>>
>> Fixes: 9e42f715264f ("drivers: net: cpsw: add phy-handle parsing")
>> Signed-off-by: David Rivshin <drivshin-5fOYsn7Fw8lBDgjK7y7TUQ@public.gmane.org>
>> Tested-by: Nicolas Chauvet <kwizart-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> I would suggest this for -stable. It should apply cleanly as far back
>> as 4.4.
>>
>> Changes since v1 [1]:
>> - Rebased (no conflicts)
>> - Added Tested-by from Nicolas Chauvet
>>
>> [1] https://patchwork.ozlabs.org/patch/560326/
>
> Reviewed-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
In my opinion, it will be good to have this patch merged as part of -rc cycle, since
it will fix "NULL pointer dereference" issue with current LKML as reported by Andrew Goodbody.
>
>>
>> drivers/net/ethernet/ti/cpsw.c | 13 ++++++-------
>> drivers/net/ethernet/ti/cpsw.h | 1 +
>> 2 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/cpsw.c
>> b/drivers/net/ethernet/ti/cpsw.c
>> index 42fdfd4..d69cb3f 100644
>> --- a/drivers/net/ethernet/ti/cpsw.c
>> +++ b/drivers/net/ethernet/ti/cpsw.c
>> @@ -363,15 +363,14 @@ static inline void slave_write(struct cpsw_slave
>> *slave, u32 val, u32 offset)
>> __raw_writel(val, slave->regs + offset);
>> }
>>
>> struct cpsw_priv {
>> spinlock_t lock;
>> struct platform_device *pdev;
>> struct net_device *ndev;
>> - struct device_node *phy_node;
>> struct napi_struct napi_rx;
>> struct napi_struct napi_tx;
>> struct device *dev;
>> struct cpsw_platform_data data;
>> struct cpsw_ss_regs __iomem *regs;
>> struct cpsw_wr_regs __iomem *wr_regs;
>> u8 __iomem *hw_stats;
>> @@ -1144,16 +1143,16 @@ static void cpsw_slave_open(struct cpsw_slave
>> *slave, struct cpsw_priv *priv)
>>
>> if (priv->data.dual_emac)
>> cpsw_add_dual_emac_def_ale_entries(priv, slave, slave_port);
>> else
>> cpsw_ale_add_mcast(priv->ale, priv->ndev->broadcast,
>> 1 << slave_port, 0, 0, ALE_MCAST_FWD_2);
>>
>> - if (priv->phy_node)
>> - slave->phy = of_phy_connect(priv->ndev, priv->phy_node,
>> + if (slave->data->phy_node)
>> + slave->phy = of_phy_connect(priv->ndev, slave->data->phy_node,
>> &cpsw_adjust_link, 0, slave->data->phy_if);
>> else
>> slave->phy = phy_connect(priv->ndev, slave->data->phy_id,
>> &cpsw_adjust_link, slave->data->phy_if);
>> if (IS_ERR(slave->phy)) {
>> dev_err(priv->dev, "phy %s not found on slave %d\n",
>> slave->data->phy_id, slave->slave_num);
>> @@ -1936,20 +1935,19 @@ static void cpsw_slave_init(struct cpsw_slave
>> *slave, struct cpsw_priv *priv,
>>
>> slave->data = data;
>> slave->regs = regs + slave_reg_ofs;
>> slave->sliver = regs + sliver_reg_ofs;
>> slave->port_vlan = data->dual_emac_res_vlan;
>> }
>>
[..]
--
regards,
-grygorii
--
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
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