Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] net: Linn Ethernet Packet Sniffer driver
From: Stathis Voukelatos @ 2015-01-27 17:22 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, abrestic@chromium.org, f.fainelli
In-Reply-To: <54C7A4DD.7030109@redhat.com>


Hi Daniel,

On 27/01/15 14:46, Daniel Borkmann wrote:
>> Just wanted to clarify some implementation details for your approach.
>> - The driver would need to create and register two net_device instances.
>> One for sniffing Ethernet TX packets and one for RX.
>
> Hm, I would represent the whole device as a single monitoring-only 
> netdev.
> I'm somehow still missing the big advantage of all this as compared to
> using packet sockets on the normal netdev? I couldn't parse that from 
> your
> commit message.

This H/W module was developed to allow accurate timestamping of selected
outgoing or incoming data packets. Timestamp values are provided by an
external implementation-dependent clock or timer that is of suitable
quality for the application.
Example: multiple audio receivers synchronizing their clocks to a
single transmitter, for synchronized playback.

The TX and RX blocks of the sniffer can be operated (eg. started, stopped,
configured) independently, that is why I believe two netdev instances would
be a better match to the H/W architecture.

>
>> - Would the control interface for the sniffer in that case need to be
>> through private socket ioctls (ie SIOCDEVPRIVATE + x ioctl ids)?
>
> Nope, please have a look at Documentation/networking/packet_mmap.txt.
>
>
Thanks for the link to the document. That is the way forward for retrieving
data from sniffer match events from user space very efficiently.

However, I am not sure about configuration, where we want to eg set the
command string, or query device attributes such as the size of the command
memory. That looks more suitable to an ioctl or a netlink message to me
and better use the packet socket just for data from sniffer match events.

Thanks,
Stathis

^ permalink raw reply

* Re: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast promiscuous mode control
From: Alexander Duyck @ 2015-01-27 17:34 UTC (permalink / raw)
  To: Hiroshi Shimamoto, Skidmore, Donald C, Bjørn Mork
  Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
	Choi, Sy Jong, linux-kernel@vger.kernel.org, David Laight,
	Hayato Momma
In-Reply-To: <7F861DC0615E0C47A872E6F3C5FCDDBD05E1335D@BPXM14GP.gisp.nec.co.jp>

On 01/27/2015 04:53 AM, Hiroshi Shimamoto wrote:
>>> On 01/22/2015 04:32 PM, Hiroshi Shimamoto wrote:
>>>>> Subject: RE: [E1000-devel] [PATCH 1/2] if_link: Add VF multicast
>>>>> promiscuous mode control
>>>>>> "Skidmore, Donald C" <donald.c.skidmore@intel.com> writes:
>>>>>>
>>>>>>> My hang up is more related to: without the nob to enable it (off by
>>>>>>> default) we are letting one VF dictate policy for all the other VFs
>>>>>>> and the PF.  If one VF needs to be in promiscuous multicast so is
>>>>>>> everyone else.  Their stacks now needs to deal with all the extra
>>>>>>> multicast packets.  As you point out this might not be a direct
>>>>>>> concern for isolation in that the VM could have 'chosen' to join
>>>>>>> any Multicast group and seen this traffic.  My concern over
>>>>>>> isolation is one VF has chosen that all the other VM now have to
>>>>>>> see this multicast traffic.
>>>>>> Apologies if this question is stupid, but I just have to ask about
>>>>>> stuff I don't understand...
>>>>>>
>>>>>> Looking at the proposed implementation, the promiscous multicast
>>>>>> flag seems to be a per-VF flag:
>>>>>>
>>>>>> +int ixgbe_ndo_set_vf_mc_promisc(struct net_device *netdev, int vf,
>>>>>> +bool
>>>>>> +setting) {
>>>>>> +	struct ixgbe_adapter *adapter = netdev_priv(netdev);
>>>>>> +	struct ixgbe_hw *hw = &adapter->hw;
>>>>>> +	u32 vmolr;
>>>>>> +
>>>>>> +	if (vf >= adapter->num_vfs)
>>>>>> +		return -EINVAL;
>>>>>> +
>>>>>> +	adapter->vfinfo[vf].mc_promisc_enabled = setting;
>>>>>> +
>>>>>> +	vmolr = IXGBE_READ_REG(hw, IXGBE_VMOLR(vf));
>>>>>> +	if (setting) {
>>>>>> +		e_info(drv, "VF %u: enabling multicast promiscuous\n", vf);
>>>>>> +		vmolr |= IXGBE_VMOLR_MPE;
>>>>>> +	} else {
>>>>>> +		e_info(drv, "VF %u: disabling multicast promiscuous\n", vf);
>>>>>> +		vmolr &= ~IXGBE_VMOLR_MPE;
>>>>>> +	}
>>>>>> +
>>>>>> +	IXGBE_WRITE_REG(hw, IXGBE_VMOLR(vf), vmolr);
>>>>>> +
>>>>>> +	return 0;
>>>>>> +}
>>>>>> +
>>>>>>
>>>>>> I haven't read the data sheet, but I took a quick look at the
>>>>>> excellent high level driver docs:
>>>>>> http://www.intel.com/content/dam/doc/design-guide/82599-sr-iov-
>>> drive
>>>>>> r-
>>>>>> companion-guide.pdf
>>>>>>
>>>>>> It mentions "Multicast Promiscuous Enable" in its "Thoughts for
>>>>>> Customization" section:
>>>>>>
>>>>>>  7.1 Multicast Promiscuous Enable
>>>>>>
>>>>>>  The controller has provisions to allow each VF to be put into
>>>>>> Multicast Promiscuous mode.  The Intel reference driver does not
>>>>>> configure this option .
>>>>>>
>>>>>>  The capability can be enabled/disabled by manipulating the MPE
>>>>>> field  (bit
>>>>>> 28) of the PF VF L2 Control Register (PFVML2FLT – 0x0F000)
>>>>>>
>>>>>> and showing a section from the data sheet describing the "PF VM L2
>>>>>> Control Register - PFVML2FLT[n]  (0x0F000 + 4 * n, n=0...63; RW)"
>>>>>>
>>>>>> To me it looks like enabling Promiscuos Multicast for a VF won't
>>>>>> affect any other VF at all.  Is this really not the case?
>>>>>>
>>>>>>
>>>>>>
>>>>>> Bjørn
>>>>> Clearly not a dumb question at all and I'm glad you mentioned that.
>>>>> :)  I was going off the assumption, been awhile since I read the
>>>>> patch, that the patch was using FCTRL.MPE or MANC.MCST_PASS_L2
>>> which would turn multicast promiscuous on for everyone.  Since the patch is
>>> using PFVML2FLT.MPE this lessens my concern over effect on the entire
>>> system.
>>>> I believe the patches for this VF multicast promiscuous mode is per VF.
>>>>
>>>>> That said I still would prefer having a way to override this behavior on the
>>> PF, although I admit my argument is weaker.
>>>>> I'm still concerned about a VF changing the behavior of the PF
>>>>> without any way to prevent it.  This might be one part philosophical
>>>>> (PF sets policy not the VF) but this still could have a noticeable
>>>>> effect on the overall system.  If any other VFs (or the PF) are
>>>>> receiving MC packets these will have to be replicated which will be a
>>>>> performance hit.  When we use the MC hash this is limited vs. when
>>> anyone is in MC promiscuous every MC packet used by another pool would
>>> be replicated.  I could imagine in some environments (i.e. public clouds)
>>> where you don't trust what is running in your VM you might what to block
>>> this from happening.
>>>> I understand your request and I'm thinking to submit the patches
>>>>   1) Add new mbox API between ixgbe/ixgbevf to turn MC promiscuous on,
>>>>      and enables it when ixgbevf needs over 30 MC addresses.
>>>>   2) Add a policy knob to prevent enabling it from the PF.
>>>>
>>>> Does it seem okay?
>>> I would advise that if such a knob is added it should be set to disabled by
>>> default.  The main problem with supporting that many multicast addresses
>>> per VF is that you run the risk for flooding the PCIe interface on the network
>>> adapter if too many adapters were to go into such a mode.
>>>
>> This was my understanding as well.  Someone would have to "choose" to allow VM to enter this mode, meaning off by default.
> I see you'd like to keep it off unless the administrator wants to configure.

Correct.  This is a somewhat risky configuration so it is best to
control who has access to it.

One additional thought is that you may want to make it so that the VF
has some control over it from its side as well.  In the case of the igb
driver I believe there is already a control for setting the multicast
promiscuous in the event of exceeding 30 multicast addresses.  You may
want to look at that for ideas on how to have a mailbox message work in
conjunction with your control to allow enabling/disabling the multicast
promiscuous mode.

>>>> BTW, I'm bit worried about to use ndo interface for 2) because adding
>>>> a new hook makes core code complicated.
>>>> Is it really reasonable to do it with ndo?
>>>> I haven't find any other suitable method to do it, right now. And
>>>> using ndo VF hook looks standard way to control VF functionality.
>>>> Then, I think it's the best way to implement this policy in ndo hook.
>>> The ndo is probably the only way to go on this.  It is how past controls for the
>>> VF network functionality have been implemented.
>>>
>>>>> In some ways it is almost the mirror image of the issue you brought up:
>>>>>
>>>>> Adding a new hook for this seems over-complicated to me.  And it
>>>>> still doesn't solve the real problems that
>>>>>  a) the user has to know about this limit, and
>>>>>  b) manually configure the feature
>>>>>
>>>>> My reverse argument might be that if this happens automatically.  It
>>>>> might take the VM provider a long time to realize performance has
>>>>> taken a hit because some VM asked to join 31 multicast groups and
>>> entered MC promiscuous.  Then only to find that they have no way to block
>>> such behavior.
>>>>> Maybe I wouldn't as concerned if the patch author could provide some
>>>>> performance results to show this won't have as a negative effect as I'm
>>> afraid it might?
>>>> Hm, what kind of test case would you like to have?
>>>> The user A who has 30 MC addresses vs the user B who has 31 MC
>>>> addresses, which means that MC promiscuous mode, and coming MC
>>> packets the user doesn't expect?
>>>> thanks,
>>>> Hiroshi
>>> The question I would have is how many of these interfaces do you expect to
>>> have supporting the expanded multicast mode?  As it currently stands
> Right now, we're thinking 1 or 2 VFs per PF will be enabled this feature
> in our use case. Another representation, 1 or 2 VMs will be used VF device on
> one server, each VM has 2 VFs usually.

This seems reasonable.  If it is only one or two VFs then you probably
would be better of using the promiscuous mode.

>>> multicast traffic has the potential to flood the adapter, greatly reduce the
>>> overall throughput, and add extra workload to the PF and all VFs.
>>> For example if several VFs enable this feature, and then someone on the
>>> network sends a stream of multicast traffic what happens to the CPU load for
>>> the host system?
> I'm not sure why all VFs are affected.
> I can imagine that PF and VFs which are enabled this feature could receive
> flooding packets, but other VFs never receive such packets.

The other VFs would't be receiving the traffic, they would just have
reduced access to traffic because for each VF that can receive a packet
it multiplies the PCIe bandwidth.  So if you have 4 VFs in multicast
promiscuous mode then that means for each multicast frame receive the
PCIe bus will have to pass the frame 4 times in order to provide a copy
to each VF.   When you consider that the 82599 can support 64 VFs this
replication can have a huge impact on the PCIe bus bandwidth.

>>> Also how many addresses beyond 30 is it you require?  An alternative to
>>> adding multicast promiscuous might be to consider extending the mailbox
>>> API to support sending more than 30 addresses via something such as a
>>> multi-part multicast configuration message.  The fm10k driver already has
>>> logic similar to this as it adds addresses 1 at a time though a separate Switch
>>> interface API between the PF and the Switch.  You might be able to reuse
>>> some of that code to reach beyond the 30 address limit.
>> I think this would be a much safer approach and probably scale well for small sets.  However I don't think it would work
>> for Hiroshi's use case.  If I remember correctly he wanted 1000's of MC groups per VM.   I imagine there would be considerable
>> overhead even loading up our 4K hash table 1 address at a time, likewise with that many address wouldn't this just be
>> pretty much the same as being in multicast promiscuous.  Still this might be an interesting approach to support those
>> needing only a few MC groups over the limit.
> We would like to use 2000 IPs usually, and would extend to 4k which comes from VLAN limit.
> IIRC, mbox API has only 32 words, it doesn't scale, I think.
> And there is a single hash table per PF, not per VF. I guess if we set 4k hash entries
> from a specific VF, every VF will receive all MC packets.

This is true.  So if you only have one or two VFs that you expect to
need that many entries it might be preferable to go the multicast
promiscuous route to avoid excess replication.

>> Alex's point is worth reiterating, this will effect performance.  Have you tested this out under load and still see the
>> results you can live with?
> I don't have any number in MC load.
> In general case, we have enough performance in VM, almost wire rate in our scenario.
>
> thanks,
> Hiroshi
>

I'd say your best bet for this is to make it both an NDO (defaulted to
disabled) and a mailbox request.  If both are enabling it then enable
the feature.  That way you can avoid possibly sending unexpected packets
to a legacy VF, or allowing a VF to enable it on a system that hasn't
been configured for it.

- Alex

^ permalink raw reply

* Re: [PATCH net-next v1 05/18] net: tx4939: use __ethtool_get_ksettings
From: David Decotigny @ 2015-01-27 17:38 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: David S. Miller, Ben Hutchings, Amir Vadai,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Eric Dumazet,
	Eugenia Emantayev, Or Gerlitz, Ido Shamay, Joe Perches,
	Saeed Mahameed, Govindarajulu Varadarajan, Venkata Duvvuru,
	Jeff Kirsher, Eyal Perry, Pravin B Shelar, Ed Swierk
In-Reply-To: <54C78540.6080404-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>

On Tue, Jan 27, 2015 at 4:32 AM, Sergei Shtylyov
<sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org> wrote:
> linux-mips-6z/3iImG2C/i7sgoIIk9UQ@public.gmane.org

Thanks, added mips + usnic + fcoe in my copy for the next wave of
reviews. Also updated the subject line.

^ permalink raw reply

* Re: [PATCH net-next V1 04/11] net/mlx4_core: Adjust command timeouts to conform to the firmware spec
From: Jack Morgenstein @ 2015-01-27 18:06 UTC (permalink / raw)
  To: David Laight
  Cc: 'Amir Vadai', David S. Miller, netdev@vger.kernel.org,
	Or Gerlitz, Yevgeny Petrilin
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1CAD3EAA@AcuExch.aculab.com>

On Tue, 27 Jan 2015 14:30:31 +0000
David Laight <David.Laight@ACULAB.COM> wrote:

> From: Amir Vadai
> > From: Jack Morgenstein <jackm@dev.mellanox.co.il>
> > 
> > The firmware spec states that the timeout for all commands should
> > be 60 seconds.
> 
> Hmmm... 60 seconds seems a long time to wait for a device to do
> something.
> 
> I'll have given up and reset the machine before that expires.
> 
> 	David
> 

We have some automatic recovery features which involve resetting the
HCA upon command timeout, and we purposely give a long timeout
in order to be sure that there are no false positives
(i.e., unjustified resets).

-Jack

^ permalink raw reply

* Re: [PATCH 3/3] net: allwinner: sun4i-emac: fix emac SRAM mapping
From: Maxime Ripard @ 2015-01-27 18:13 UTC (permalink / raw)
  To: Jens Kuske
  Cc: Arnd Bergmann, Lee Jones, netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Chen-Yu Tsai,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <54C65089.70305-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 5025 bytes --]

Hi,

On Mon, Jan 26, 2015 at 03:34:49PM +0100, Jens Kuske wrote:
> Hi,
> 
> On 25/01/15 17:25, Maxime Ripard wrote:
> >Hi Jens,
> >
> >On Sun, Jan 25, 2015 at 04:49:19PM +0100, Jens Kuske wrote:
> >>The EMAC needs SRAM block A3_A4 being mapped to EMAC peripheral to
> >>work. This is done by the bootloader most of the time, but U-Boot
> >>Falcon Mode, for example, skips emac initialization and SRAM would
> >>stay mapped to the CPU.
> >
> >Thanks for reviving this.
> >
> >>Signed-off-by: Jens Kuske <jenskuske-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>---
> >>  drivers/net/ethernet/allwinner/Kconfig      |  1 +
> >>  drivers/net/ethernet/allwinner/sun4i-emac.c | 18 ++++++++++++++++++
> >>  2 files changed, 19 insertions(+)
> >>
> >>diff --git a/drivers/net/ethernet/allwinner/Kconfig b/drivers/net/ethernet/allwinner/Kconfig
> >>index d8d95d4..508a288 100644
> >>--- a/drivers/net/ethernet/allwinner/Kconfig
> >>+++ b/drivers/net/ethernet/allwinner/Kconfig
> >>@@ -28,6 +28,7 @@ config SUN4I_EMAC
> >>  	select MII
> >>  	select PHYLIB
> >>  	select MDIO_SUN4I
> >>+	select MFD_SYSCON
> >>          ---help---
> >>            Support for Allwinner A10 EMAC ethernet driver.
> >>
> >>diff --git a/drivers/net/ethernet/allwinner/sun4i-emac.c b/drivers/net/ethernet/allwinner/sun4i-emac.c
> >>index 1fcd556..86c891d 100644
> >>--- a/drivers/net/ethernet/allwinner/sun4i-emac.c
> >>+++ b/drivers/net/ethernet/allwinner/sun4i-emac.c
> >>@@ -18,6 +18,8 @@
> >>  #include <linux/gpio.h>
> >>  #include <linux/interrupt.h>
> >>  #include <linux/irq.h>
> >>+#include <linux/mfd/syscon.h>
> >>+#include <linux/mfd/syscon/sun4i-sc.h>
> >>  #include <linux/mii.h>
> >>  #include <linux/module.h>
> >>  #include <linux/netdevice.h>
> >>@@ -28,6 +30,7 @@
> >>  #include <linux/of_platform.h>
> >>  #include <linux/platform_device.h>
> >>  #include <linux/phy.h>
> >>+#include <linux/regmap.h>
> >>
> >>  #include "sun4i-emac.h"
> >>
> >>@@ -78,6 +81,7 @@ struct emac_board_info {
> >>
> >>  	struct phy_device	*phy_dev;
> >>  	struct device_node	*phy_node;
> >>+	struct regmap		*sc;
> >>  	unsigned int		link;
> >>  	unsigned int		speed;
> >>  	unsigned int		duplex;
> >>@@ -862,6 +866,18 @@ static int emac_probe(struct platform_device *pdev)
> >>  		goto out;
> >>  	}
> >>
> >>+	/* Map SRAM_A3_A4 to EMAC */
> >>+	db->sc = syscon_regmap_lookup_by_compatible(
> >>+						"allwinner,sun4i-a10-syscon");
> >>+	if (IS_ERR(db->sc)) {
> >>+		dev_err(&pdev->dev, "failed to find syscon regmap\n");
> >>+		ret = PTR_ERR(db->sc);
> >>+		goto out;
> >>+	}
> >>+
> >>+	regmap_update_bits(db->sc, SUN4I_SC1, SUN4I_SC1_SRAM_A3_A4_MAP_MASK,
> >>+						SUN4I_SC1_SRAM_A3_A4_MAP_EMAC);
> >>+
> >
> >I don't think that using a syscon is the right solution here.
> >
> >All this SRAM mapping thing is mutually exclusive, and will possibly
> >impact other drivers as well.
> 
> Each single SRAM area can only be mapped to a single peripheral, so as long
> as the driver only changes bits related to his own area nothing can go wrong
> I believe.

Which is exactly my point. This kind of assumption is very
fragile. Such mistakes might be made, will slip in under any review
and might get un-noticed for a very long time.

While adding a simple driver at least would make this obvious when two
drivers will contend for the same SRAM mapping.

> SRAM_C2 looks like it can be mapped do different devices (AE, CE, ACE), but
> as far as I understand this, they are all related to the ACE device, sharing
> a common register space, and would have to be handled by a single driver
> anyway (if that will ever happen without docs)
> https://linux-sunxi.org/ACE_Register_guide
> 
> >I think this is a more a case for a small driver in drivers/soc that
> >would take care of this, and make sure that client drivers don't step
> >on each other's toe.
> 
> I'm not convinced this is necessary, but what would this driver do different
> than a basic regmap? Check if the area is already mapped by any driver and
> deny mapping it again by a different driver? Which different driver, each
> area is only interesting for a single device/driver? Except maybe mapping it
> to CPU as general purpose sram, but that would need some direct agreement
> with the driver to steal its memory anyway.

Off the top of my head:
  - Make sure no one step on each other's toe, including the CPU that
    might require a SRAM for several things (suspend, cpuidle, PSCI,
    etc.)
  - Provide a comprehensive status of the various SRAM and what they
    are mapped to in debugfs
  - Provide an abstraction to the SRAM mapping IP. You make the
    assumption that the client drivers will always use on an A10 or an
    SoC that has the same SRAMs, and the same IP to control which
    device they are mapped to. This might not be true for other
    drivers, and other SoCs.
  - Make any adjustment to these registers that wouldn't fit in any
    client drivers.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply

* RE: Fw: [Bug 92081] New: skb->len=0 and getting "EOF on netlink" with "ip monitor all" (of iproute) when adding a vlan with "bridge vlan add"
From: Rosen, Rami @ 2015-01-27 18:38 UTC (permalink / raw)
  To: roopa, Stephen Hemminger; +Cc: netdev@vger.kernel.org
In-Reply-To: <54C7BEA4.9000406@cumulusnetworks.com>

Hi, Roopa,

> I think my below commit fixed one case of such error:
I am well aware of your commit (in fact I even sent cleanup patch on top of it, removing the oflags,  which was applied).

It seems to me that this commit of yours does not avoid the specific problem of getting EOF with "ip monitor all" which is described in the BUG I opened; it 
could be that it avoid problem with other scenarios, and with wrong message size when both SELF and MASTER flags are set.

> The reason for the zero length message in this case is that the user is sending
>  the setlink request to the bridge with self flag set.
> And since the getlink on the bridge device only returns bytes when its a  bridge port, there are no bytes in the skb.

> I will reconfirm that the above is true and submit a patch (I can update the bugzilla link below as well).

This is exactly so, I am fully confident about it, I checked it in depth with debug , and I had printed the skb->len before calling rtnl_notify() in 
rtnl_bridge_notify() in net/core/rtnetlink.c under such scenario described in the BUG mentioned in the bugzilla link and it was indeed 0.

For the sake of those who are interested in more implementation details and in the code walkthrough under such scenario, what happens when "bridge vlan add vid 1 dev br0 self" , you should follow this path:

Look at rtnl_bridge_setlink() method, it is invoked in this case.
http://lxr.free-electrons.com/source/net/core/rtnetlink.c#L2782

If the SELF flag is set it calls dev->netdev_ops->ndo_bridge_setlink()
See:
http://lxr.free-electrons.com/source/net/core/rtnetlink.c#L2840

and then it calls rtnl_bridge_notify()
See:
http://lxr.free-electrons.com/source/net/core/rtnetlink.c#L2850

Now, rtnl_bridge_notify() calls  dev->netdev_ops->ndo_bridge_getlink()
when the self flag is set.
See:
http://lxr.free-electrons.com/source/net/core/rtnetlink.c#L2767

Now, when running the "bridge vlan add" on a bridge device like we do (and **not on a bridge port**)
then the dev variable is an instance of a software bridge. So this calls the ndo_bridge_getlink() callback of the software bridge, which is br_getlink():
See:
http://lxr.free-electrons.com/source/net/bridge/br_netlink.c#L205

Now, br_getlink() first checks if the device is a bridge port:
struct net_bridge_port *port = br_port_get_rtnl(dev);

And it returns 0 if not.
So as a result, the skb->len is 0 and an empty notification is sent.

And when the rtneltnlink socket, which is opened by "ip monitor all" and listens to netlink messages, receives an
empty notification it terminates with the "EOF" message (as mentioned in the bugzilla link).

Sending a patch for resolving it and updating the bugzilla will be really great!

Regards,
Rami Rosen
Intel Corporation



-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf Of roopa
Sent: Tuesday, January 27, 2015 18:37
To: Stephen Hemminger
Cc: netdev@vger.kernel.org
Subject: Re: Fw: [Bug 92081] New: skb->len=0 and getting "EOF on netlink" with "ip monitor all" (of iproute) when adding a vlan with "bridge vlan add"

I noticed this during my  recent cleanup of rtnl_bridge_setlink/rtnl_bridge_dellink.

I think my below commit fixed one case of such error:

commit 02dba4388d1691a087f40fe8acd2e1ffd577a07f
Author: Roopa Prabhu <roopa@cumulusnetworks.com>
Date:   Wed Jan 14 20:02:25 2015 -0800

     bridge: fix setlink/dellink notifications



The reason for the zero length message in this case is that the user is sending
  the setlink request to the bridge with self flag set.
And since the getlink on the bridge device only returns bytes when its a  bridge port, there are no bytes in the skb.

I will reconfirm that the above is true and submit a patch (I can update the bugzilla link below as well).

Thanks,
Roopa



On 1/27/15, 4:01 AM, Stephen Hemminger wrote:
>
> Begin forwarded message:
>
> Date: Mon, 26 Jan 2015 10:15:12 -0800
> From: "bugzilla-daemon@bugzilla.kernel.org" 
> <bugzilla-daemon@bugzilla.kernel.org>
> To: "stephen@networkplumber.org" <stephen@networkplumber.org>
> Subject: [Bug 92081] New: skb->len=0 and getting "EOF on netlink" with "ip monitor all" (of iproute) when adding a vlan with "bridge vlan add"
>
>
> https://bugzilla.kernel.org/show_bug.cgi?id=92081
>
>              Bug ID: 92081
>             Summary: skb->len=0 and getting "EOF on netlink" with "ip
>                      monitor all" (of iproute) when adding a vlan with
>                      "bridge vlan add"
>             Product: Networking
>             Version: 2.5
>      Kernel Version: 3.17.6-300
>            Hardware: All
>                  OS: Linux
>                Tree: Fedora
>              Status: NEW
>            Severity: high
>            Priority: P1
>           Component: Other
>            Assignee: shemminger@linux-foundation.org
>            Reporter: ramirose@gmail.com
>          Regression: No
>
> On Fedora 21, with 3.17.6-300.fc21.x86_64, with iproute-3.16.0-3 
> (installed from rpm), ip -V:
> ip utility, iproute2-ss140804
>
> Running in one terminal:
> ip monitor all
>
> And then running in a second terminal this sequence:
> ip link add br0 type bridge
> bridge vlan add vid 10 dev br0 self
>
> causes the "ip monitor all" to terminate, with "EOF on netlink".
>
> This happens also on older distros of Fedora (Fedora 20 and downward) 
> with older kernels.
>
> It seems that the reason is that an skb->len is 0 for the netlink 
> notification which is sent from with rtnl_notify() which is invoked 
> from  rtnl_bridge_notify(), which in turn is invoked from  
> rtnl_bridge_setlink().
>
> See:
> http://lxr.free-electrons.com/source/net/core/rtnetlink.c#L2773
>
> Rami Rosen
>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH net-next v2 0/4] net: phy: prevent double suspend
From: Sergei Shtylyov @ 2015-01-27 19:04 UTC (permalink / raw)
  To: Florian Fainelli, netdev; +Cc: davem, s.hauer, b38611
In-Reply-To: <1422338740-18418-1-git-send-email-f.fainelli@gmail.com>

Hello.

On 01/27/2015 09:05 AM, Florian Fainelli wrote:

> This patch series addresses a problem that Fugang and I observed on different
> platforms where a given PHY device might end-up being suspended twice.

> Once as part of the call from ndo_open() all the way down to phy_detach() and
> phy_suspend() and a second time when the generic platform device/driver
> suspend/resume callbacks are called in drivers/net/phy/mdio_bus.c.

> Thanks to Fugang for giving this a quick try on i.MX6/FEC and reporting
> positive test results!

    I can now confirm this patchset fixed our issue with an external abort 
during suspend on R-Car platform (with the 'sh_eth' driver)!

> Florian Fainelli (4):
>    net: phy: utilize phy_suspend and phy_resume
>    net: phy: document has_fixups field
>    net: phy: keep track of the PHY suspend state
>    net: phy: avoid suspending twice a PHY

WBR, Sergei

^ permalink raw reply

* Re: [PATCH 0/2] sh_eth: massage PM code
From: Sergei Shtylyov @ 2015-01-27 19:06 UTC (permalink / raw)
  To: netdev, David Miller; +Cc: linux-sh
In-Reply-To: <5608250.lrFkJTjgSf@wasted.cogentembedded.com>

Hello.

On 01/22/2015 01:16 AM, Sergei Shtylyov wrote:

>     Here's a set of 2 patches against DaveM's 'net-next.git' repo. We're adding
> the support for suspend/hibernation as well as somewhat changing the existing
> code. There are still MDIO-related issue with suspend (kernel exception), we've
> been working on it and shall address it with a separate patch...

    It turned out that Florian Fainelli's recent series was the best approach 
to fixing this issue. We were only looking at our driver while it was a common 
MDIO problem...

> [1/2] sh_eth: use SET_RUNTIME_PM_OPS()
> [2/2] sh_eth: add more PM methods

WBR, Sergei


^ permalink raw reply

* Re: [PATCH net-next 06/10] net/mlx4_core: Fix struct mlx4_vhcr_cmd to make implicit padding explicit
From: David Miller @ 2015-01-27 19:13 UTC (permalink / raw)
  To: David.Laight; +Cc: amirv, netdev, ogerlitz, yevgenyp, jackm
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1CAD3B01@AcuExch.aculab.com>

From: David Laight <David.Laight@ACULAB.COM>
Date: Tue, 27 Jan 2015 09:43:27 +0000

>>  	__be64 out_param;
>>  	__be16 token;
>>  	u16 reserved;
>>  	u8 status;
>>  	u8 flags;
>>  	__be16 opcode;
>> -};
>> +} __packed;
> 
> Don't add '__packed' unless you expect the structure to be misaligned
> in memory.
> On systems that fault mis-aligned memory requests you've requested the
> compiler generate code to read/write everything using byte sized memory
> accesses and a lot of shifting and masking.

Indeed, I'm really sick of seeing these packed structures being
created all over the place.

They are to be used in absolutely extreme cases where no other
solution is possible.

^ permalink raw reply

* Re: [PATCH net-next v2 0/4] net: phy: prevent double suspend
From: Florian Fainelli @ 2015-01-27 19:16 UTC (permalink / raw)
  To: Sergei Shtylyov, netdev; +Cc: davem, s.hauer, b38611
In-Reply-To: <54C7E140.4080908@cogentembedded.com>

On 27/01/15 11:04, Sergei Shtylyov wrote:
> Hello.
> 
> On 01/27/2015 09:05 AM, Florian Fainelli wrote:
> 
>> This patch series addresses a problem that Fugang and I observed on
>> different
>> platforms where a given PHY device might end-up being suspended twice.
> 
>> Once as part of the call from ndo_open() all the way down to
>> phy_detach() and
>> phy_suspend() and a second time when the generic platform device/driver
>> suspend/resume callbacks are called in drivers/net/phy/mdio_bus.c.
> 
>> Thanks to Fugang for giving this a quick try on i.MX6/FEC and reporting
>> positive test results!
> 
>    I can now confirm this patchset fixed our issue with an external
> abort during suspend on R-Car platform (with the 'sh_eth' driver)!

Great, thanks Sergei!

> 
>> Florian Fainelli (4):
>>    net: phy: utilize phy_suspend and phy_resume
>>    net: phy: document has_fixups field
>>    net: phy: keep track of the PHY suspend state
>>    net: phy: avoid suspending twice a PHY
> 
> WBR, Sergei
> 


-- 
Florian

^ permalink raw reply

* Does arping update arp table?
From: Murali Karicheri @ 2015-01-27 19:23 UTC (permalink / raw)
  To: netdev; +Cc: David Miller

Experts,

The ARP code (net/ipv4/arp.c) seems to treat arp response resulting from 
a arping as an unsolicited response and doesn't update the arp table. 
This get updated if the per interface accept sys ctrl is enabled. This 
command seems to have been updating the arp table in older kernel. For 
example on a Ubuntu 12.04 based kernel I see

a0868495@ula0868495 ~/Project/linux-keystone $ sudo arping 192.168.1.10 
-c 1 -f
ARPING 192.168.1.10 from 192.168.1.9 eth0
Unicast reply from 192.168.1.10 [00:1B:A9:02:26:10]  5.411ms
Sent 1 probes (1 broadcast(s))
Received 1 response(s)
a0868495@ula0868495 ~/Project/linux-keystone $ arp -n -a
? (192.168.1.10) at 00:1b:a9:02:26:10 [ether] on eth0
? (192.168.1.1) at 00:1f:90:6d:5c:a8 [ether] on eth0

a0868495@ula0868495 ~/Project/linux-keystone $ uname -a
Linux ula0868495 3.2.0-23-generic #36-Ubuntu SMP Tue Apr 10 20:41:14 UTC 
2012 i686 i686 i386 GNU/Linux

However this command doesn't update the arp table on my Keystone EVM 
based on v3.19.x and also newer ubuntu machine based on v3.11. Is this 
expected behavior? I believe the arp response resulting from a arping 
command is controlled through per interface accept sys control like as 
in gratuitous arp. Please repond.

Thanks and regards,
-- 
Murali Karicheri
Linux Kernel, Texas Instruments

^ permalink raw reply

* Re: [PATCHv3 ipsec-next] xfrm: Do not parse 32bits compiled xfrm netlink msg on 64bits host
From: David Miller @ 2015-01-27 19:24 UTC (permalink / raw)
  To: David.Laight; +Cc: fan.du, steffen.klassert, herbert, netdev, fengyuleidian0615
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1CAD3B2B@AcuExch.aculab.com>

From: David Laight <David.Laight@ACULAB.COM>
Date: Tue, 27 Jan 2015 09:46:21 +0000

> From: Fan Du
>> structure like xfrm_usersa_info or xfrm_userpolicy_info
>> has different sizeof when compiled as 32bits and 64bits
>> due to not appending pack attribute in their definition.
> 
> Don't 'pack' the structure, just ensure that all the fields
> are fixed sized and on their natural boundary.

This horse went out of the door more than a decade ago, we can't
change the layout of any of these structures and must at some point
add code to translate instead.

^ permalink raw reply

* [PATCH net-next V2 1/1] dev: add per net_device packet type chains
From: Salam Noureddine @ 2015-01-27 19:35 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Tom Herbert, Jiri Pirko,
	Vlad Yasevich, netdev
  Cc: Salam Noureddine, Eric W. Biederman

When many pf_packet listeners are created on a lot of interfaces the
current implementation using global packet type lists scales poorly.
This patch adds per net_device packet type lists to fix this problem.

The patch was originally written by Eric Biederman for linux-2.6.29.
Tested on linux-3.16.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Salam Noureddine <noureddine@arista.com>
---
 include/linux/netdevice.h |   2 +
 net/core/dev.c            | 132 +++++++++++++++++++++++++++++-----------------
 2 files changed, 86 insertions(+), 48 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 642d426..3d37c6e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1514,6 +1514,8 @@ struct net_device {
 	struct list_head	napi_list;
 	struct list_head	unreg_list;
 	struct list_head	close_list;
+	struct list_head	ptype_all;
+	struct list_head	ptype_specific;
 
 	struct {
 		struct list_head upper;
diff --git a/net/core/dev.c b/net/core/dev.c
index 1e325ad..a7b6c62 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -371,9 +371,10 @@ static inline void netdev_set_addr_lockdep_class(struct net_device *dev)
 static inline struct list_head *ptype_head(const struct packet_type *pt)
 {
 	if (pt->type == htons(ETH_P_ALL))
-		return &ptype_all;
+		return pt->dev ? &pt->dev->ptype_all : &ptype_all;
 	else
-		return &ptype_base[ntohs(pt->type) & PTYPE_HASH_MASK];
+		return pt->dev ? &pt->dev->ptype_specific :
+				 &ptype_base[ntohs(pt->type) & PTYPE_HASH_MASK];
 }
 
 /**
@@ -1734,6 +1735,23 @@ static inline int deliver_skb(struct sk_buff *skb,
 	return pt_prev->func(skb, skb->dev, pt_prev, orig_dev);
 }
 
+static inline void deliver_ptype_list_skb(struct sk_buff *skb,
+					  struct packet_type **pt,
+					  struct net_device *dev, __be16 type,
+					  struct list_head *ptype_list)
+{
+	struct packet_type *ptype, *pt_prev = *pt;
+
+	list_for_each_entry_rcu(ptype, ptype_list, list) {
+		if (ptype->type != type)
+			continue;
+		if (pt_prev)
+			deliver_skb(skb, pt_prev, dev);
+		pt_prev = ptype;
+	}
+	*pt = pt_prev;
+}
+
 static inline bool skb_loop_sk(struct packet_type *ptype, struct sk_buff *skb)
 {
 	if (!ptype->af_packet_priv || !skb->sk)
@@ -1757,45 +1775,54 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
 	struct packet_type *ptype;
 	struct sk_buff *skb2 = NULL;
 	struct packet_type *pt_prev = NULL;
+	struct list_head *ptype_list = &ptype_all;
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(ptype, &ptype_all, list) {
+again:
+	list_for_each_entry_rcu(ptype, ptype_list, list) {
 		/* Never send packets back to the socket
 		 * they originated from - MvS (miquels@drinkel.ow.org)
 		 */
-		if ((ptype->dev == dev || !ptype->dev) &&
-		    (!skb_loop_sk(ptype, skb))) {
-			if (pt_prev) {
-				deliver_skb(skb2, pt_prev, skb->dev);
-				pt_prev = ptype;
-				continue;
-			}
+		if (skb_loop_sk(ptype, skb))
+			continue;
 
-			skb2 = skb_clone(skb, GFP_ATOMIC);
-			if (!skb2)
-				break;
+		if (pt_prev) {
+			deliver_skb(skb2, pt_prev, skb->dev);
+			pt_prev = ptype;
+			continue;
+		}
 
-			net_timestamp_set(skb2);
+		/* need to clone skb, done only once */
+		skb2 = skb_clone(skb, GFP_ATOMIC);
+		if (!skb2)
+			goto out_unlock;
 
-			/* skb->nh should be correctly
-			   set by sender, so that the second statement is
-			   just protection against buggy protocols.
-			 */
-			skb_reset_mac_header(skb2);
-
-			if (skb_network_header(skb2) < skb2->data ||
-			    skb_network_header(skb2) > skb_tail_pointer(skb2)) {
-				net_crit_ratelimited("protocol %04x is buggy, dev %s\n",
-						     ntohs(skb2->protocol),
-						     dev->name);
-				skb_reset_network_header(skb2);
-			}
+		net_timestamp_set(skb2);
 
-			skb2->transport_header = skb2->network_header;
-			skb2->pkt_type = PACKET_OUTGOING;
-			pt_prev = ptype;
+		/* skb->nh should be correctly
+		 * set by sender, so that the second statement is
+		 * just protection against buggy protocols.
+		 */
+		skb_reset_mac_header(skb2);
+
+		if (skb_network_header(skb2) < skb2->data ||
+		    skb_network_header(skb2) > skb_tail_pointer(skb2)) {
+			net_crit_ratelimited("protocol %04x is buggy, dev %s\n",
+					     ntohs(skb2->protocol),
+					     dev->name);
+			skb_reset_network_header(skb2);
 		}
+
+		skb2->transport_header = skb2->network_header;
+		skb2->pkt_type = PACKET_OUTGOING;
+		pt_prev = ptype;
+	}
+
+	if (ptype_list == &ptype_all) {
+		ptype_list = &dev->ptype_all;
+		goto again;
 	}
+out_unlock:
 	if (pt_prev)
 		pt_prev->func(skb2, skb->dev, pt_prev, skb->dev);
 	rcu_read_unlock();
@@ -2617,7 +2644,7 @@ static int xmit_one(struct sk_buff *skb, struct net_device *dev,
 	unsigned int len;
 	int rc;
 
-	if (!list_empty(&ptype_all))
+	if (!list_empty(&ptype_all) || !list_empty(&dev->ptype_all))
 		dev_queue_xmit_nit(skb, dev);
 
 	len = skb->len;
@@ -3615,7 +3642,6 @@ static int __netif_receive_skb_core(struct sk_buff *skb, bool pfmemalloc)
 	struct packet_type *ptype, *pt_prev;
 	rx_handler_func_t *rx_handler;
 	struct net_device *orig_dev;
-	struct net_device *null_or_dev;
 	bool deliver_exact = false;
 	int ret = NET_RX_DROP;
 	__be16 type;
@@ -3658,11 +3684,15 @@ another_round:
 		goto skip_taps;
 
 	list_for_each_entry_rcu(ptype, &ptype_all, list) {
-		if (!ptype->dev || ptype->dev == skb->dev) {
-			if (pt_prev)
-				ret = deliver_skb(skb, pt_prev, orig_dev);
-			pt_prev = ptype;
-		}
+		if (pt_prev)
+			ret = deliver_skb(skb, pt_prev, orig_dev);
+		pt_prev = ptype;
+	}
+
+	list_for_each_entry_rcu(ptype, &skb->dev->ptype_all, list) {
+		if (pt_prev)
+			ret = deliver_skb(skb, pt_prev, orig_dev);
+		pt_prev = ptype;
 	}
 
 skip_taps:
@@ -3718,19 +3748,21 @@ ncls:
 		skb->vlan_tci = 0;
 	}
 
+	type = skb->protocol;
+
 	/* deliver only exact match when indicated */
-	null_or_dev = deliver_exact ? skb->dev : NULL;
+	if (likely(!deliver_exact)) {
+		deliver_ptype_list_skb(skb, &pt_prev, orig_dev, type,
+				       &ptype_base[ntohs(type) &
+				       		   PTYPE_HASH_MASK]);
+	}
 
-	type = skb->protocol;
-	list_for_each_entry_rcu(ptype,
-			&ptype_base[ntohs(type) & PTYPE_HASH_MASK], list) {
-		if (ptype->type == type &&
-		    (ptype->dev == null_or_dev || ptype->dev == skb->dev ||
-		     ptype->dev == orig_dev)) {
-			if (pt_prev)
-				ret = deliver_skb(skb, pt_prev, orig_dev);
-			pt_prev = ptype;
-		}
+	deliver_ptype_list_skb(skb, &pt_prev, orig_dev, type,
+			       &orig_dev->ptype_specific);
+
+	if (unlikely(skb->dev != orig_dev)) {
+		deliver_ptype_list_skb(skb, &pt_prev, orig_dev, type,
+				       &skb->dev->ptype_specific);
 	}
 
 	if (pt_prev) {
@@ -6579,6 +6611,8 @@ void netdev_run_todo(void)
 
 		/* paranoia */
 		BUG_ON(netdev_refcnt_read(dev));
+		BUG_ON(!list_empty(&dev->ptype_all));
+		BUG_ON(!list_empty(&dev->ptype_specific));
 		WARN_ON(rcu_access_pointer(dev->ip_ptr));
 		WARN_ON(rcu_access_pointer(dev->ip6_ptr));
 		WARN_ON(dev->dn_ptr);
@@ -6761,6 +6795,8 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
 	INIT_LIST_HEAD(&dev->adj_list.lower);
 	INIT_LIST_HEAD(&dev->all_adj_list.upper);
 	INIT_LIST_HEAD(&dev->all_adj_list.lower);
+	INIT_LIST_HEAD(&dev->ptype_all);
+	INIT_LIST_HEAD(&dev->ptype_specific);
 	dev->priv_flags = IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM;
 	setup(dev);
 
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH v3 0/3] net: stmmac: Enable Intel Quark SoC X1000 Ethernet support
From: Andy Shevchenko @ 2015-01-27 19:44 UTC (permalink / raw)
  To: Giuseppe Cavallaro, netdev, David S . Miller; +Cc: Andy Shevchenko

This is third version of the patch series [1] to bring network card support to
Intel Quark SoC.

The series has been tested on Intel Galileo board.

Changelog v3:
 - rebase on top of recent net-next
 - rework an approach to get the custom configuration
 - rework an approach how to get unique bus_id
 - improve DMI lookup function

[1] http://www.spinics.net/lists/netdev/msg296010.html

Andy Shevchenko (1):
  stmmac: pci: introduce Intel Quark X1000 runtime detection

Kweh, Hock Leong (2):
  stmmac: pci: add support for Intel Quark X1000
  stmmac: pci: add MSI support for Intel Quark X1000

 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 113 ++++++++++++++++++++++-
 1 file changed, 112 insertions(+), 1 deletion(-)

-- 
2.1.4

^ permalink raw reply

* [PATCH v3 1/3] stmmac: pci: add support for Intel Quark X1000
From: Andy Shevchenko @ 2015-01-27 19:44 UTC (permalink / raw)
  To: Giuseppe Cavallaro, netdev, David S . Miller
  Cc: Kweh, Hock Leong, Andy Shevchenko
In-Reply-To: <1422387889-21559-1-git-send-email-andriy.shevchenko@linux.intel.com>

From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>

The Intel Quark SoC X1000 provides two 10/100 Mbps Ethernet MAC
controllers which may or may not be connected to PHY on board.
This MAC controller only supports RMII PHY. This patch add Quark
PCI ID as well as Quark default platform data info to this driver.

Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 51 +++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 054520d..a316187 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -26,6 +26,12 @@
 #include <linux/pci.h>
 #include "stmmac.h"
 
+struct stmmac_pci_info {
+	struct pci_dev *pdev;
+	int (*setup)(struct plat_stmmacenet_data *plat,
+		     struct stmmac_pci_info *info);
+};
+
 static void stmmac_default_data(struct plat_stmmacenet_data *plat)
 {
 	plat->bus_id = 1;
@@ -48,6 +54,38 @@ static void stmmac_default_data(struct plat_stmmacenet_data *plat)
 	plat->unicast_filter_entries = 1;
 }
 
+static int quark_default_data(struct plat_stmmacenet_data *plat,
+			      struct stmmac_pci_info *info)
+{
+	struct pci_dev *pdev = info->pdev;
+
+	plat->bus_id = PCI_DEVID(pdev->bus->number, pdev->devfn);
+	plat->phy_addr = 1;
+	plat->interface = PHY_INTERFACE_MODE_RMII;
+	plat->clk_csr = 2;
+	plat->has_gmac = 1;
+	plat->force_sf_dma_mode = 1;
+
+	plat->mdio_bus_data->phy_reset = NULL;
+	plat->mdio_bus_data->phy_mask = 0;
+
+	plat->dma_cfg->pbl = 16;
+	plat->dma_cfg->burst_len = DMA_AXI_BLEN_256;
+	plat->dma_cfg->fixed_burst = 1;
+
+	/* Set default value for multicast hash bins */
+	plat->multicast_filter_bins = HASH_TABLE_SIZE;
+
+	/* Set default value for unicast filter entries */
+	plat->unicast_filter_entries = 1;
+
+	return 0;
+}
+
+static struct stmmac_pci_info quark_pci_info = {
+	.setup = quark_default_data,
+};
+
 /**
  * stmmac_pci_probe
  *
@@ -63,6 +101,7 @@ static void stmmac_default_data(struct plat_stmmacenet_data *plat)
 static int stmmac_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *id)
 {
+	struct stmmac_pci_info *info = (struct stmmac_pci_info *)id->driver_data;
 	struct plat_stmmacenet_data *plat;
 	struct stmmac_priv *priv;
 	int i;
@@ -103,7 +142,15 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
 
 	pci_set_master(pdev);
 
-	stmmac_default_data(plat);
+	if (info) {
+		info->pdev = pdev;
+		if (info->setup) {
+			ret = info->setup(plat, info);
+			if (ret)
+				return ret;
+		}
+	} else
+		stmmac_default_data(plat);
 
 	priv = stmmac_dvr_probe(&pdev->dev, plat, pcim_iomap_table(pdev)[i]);
 	if (IS_ERR(priv)) {
@@ -155,11 +202,13 @@ static int stmmac_pci_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(stmmac_pm_ops, stmmac_pci_suspend, stmmac_pci_resume);
 
 #define STMMAC_VENDOR_ID 0x700
+#define STMMAC_QUARK_ID  0x0937
 #define STMMAC_DEVICE_ID 0x1108
 
 static const struct pci_device_id stmmac_id_table[] = {
 	{PCI_DEVICE(STMMAC_VENDOR_ID, STMMAC_DEVICE_ID)},
 	{PCI_DEVICE(PCI_VENDOR_ID_STMICRO, PCI_DEVICE_ID_STMICRO_MAC)},
+	{PCI_VDEVICE(INTEL, STMMAC_QUARK_ID), (kernel_ulong_t)&quark_pci_info},
 	{}
 };
 
-- 
2.1.4

^ permalink raw reply related

* [PATCH v3 3/3] stmmac: pci: add MSI support for Intel Quark X1000
From: Andy Shevchenko @ 2015-01-27 19:44 UTC (permalink / raw)
  To: Giuseppe Cavallaro, netdev, David S . Miller
  Cc: Kweh, Hock Leong, Andy Shevchenko
In-Reply-To: <1422387889-21559-1-git-send-email-andriy.shevchenko@linux.intel.com>

From: "Kweh, Hock Leong" <hock.leong.kweh@intel.com>

In Intel Quark SoC X1000, both of the Ethernet controllers support
MSI interrupt handling. This patch enables them to use MSI interrupt
servicing in stmmac_pci for Intel Quark X1000.

Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 50f3c50..3bca908 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -212,6 +212,8 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
 	} else
 		stmmac_default_data(plat);
 
+	pci_enable_msi(pdev);
+
 	priv = stmmac_dvr_probe(&pdev->dev, plat, pcim_iomap_table(pdev)[i]);
 	if (IS_ERR(priv)) {
 		dev_err(&pdev->dev, "%s: main driver probe failed\n", __func__);
-- 
2.1.4

^ permalink raw reply related

* [PATCH v3 2/3] stmmac: pci: introduce Intel Quark X1000 runtime detection
From: Andy Shevchenko @ 2015-01-27 19:44 UTC (permalink / raw)
  To: Giuseppe Cavallaro, netdev, David S . Miller
  Cc: Andy Shevchenko, Kweh, Hock Leong
In-Reply-To: <1422387889-21559-1-git-send-email-andriy.shevchenko@linux.intel.com>

This patch introduces run-time board detection through DMI and MAC-PHY
configuration function used by quark_default_data() during initialization. It
fills up the phy_addr for Galileo and Galileo Gen2 boards to indicate that the
Ethernet MAC controller is or is not connected to any PHY.

The implementation takes into consideration for future expansion in Quark
series boards that may have different PHY address that is linked to its MAC
controllers.

This piece of work is derived from Bryan O'Donoghue's initial work for Quark
X1000 enabling.

Signed-off-by: Kweh, Hock Leong <hock.leong.kweh@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 62 +++++++++++++++++++++++-
 1 file changed, 61 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index a316187..50f3c50 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -24,14 +24,50 @@
 *******************************************************************************/
 
 #include <linux/pci.h>
+#include <linux/dmi.h>
+
 #include "stmmac.h"
 
+/*
+ * This struct is used to associate PCI Function of MAC controller on a board,
+ * discovered via DMI, with the address of PHY connected to the MAC. The
+ * negative value of the address means that MAC controller is not connected
+ * with PHY.
+ */
+struct stmmac_pci_dmi_data {
+	const char *name;
+	unsigned int func;
+	int phy_addr;
+};
+
 struct stmmac_pci_info {
 	struct pci_dev *pdev;
 	int (*setup)(struct plat_stmmacenet_data *plat,
 		     struct stmmac_pci_info *info);
+	struct stmmac_pci_dmi_data *dmi;
 };
 
+static int stmmac_pci_find_phy_addr(struct stmmac_pci_info *info)
+{
+	const char *name = dmi_get_system_info(DMI_BOARD_NAME);
+	unsigned int func = PCI_FUNC(info->pdev->devfn);
+	struct stmmac_pci_dmi_data *dmi;
+
+	/*
+	 * Galileo boards with old firmware don't support DMI. We always return
+	 * 1 here, so at least first found MAC controller would be probed.
+	 */
+	if (!name)
+		return 1;
+
+	for (dmi = info->dmi; dmi->name && *dmi->name; dmi++) {
+		if (!strcmp(dmi->name, name) && dmi->func == func)
+			return dmi->phy_addr;
+	}
+
+	return -ENODEV;
+}
+
 static void stmmac_default_data(struct plat_stmmacenet_data *plat)
 {
 	plat->bus_id = 1;
@@ -58,9 +94,18 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
 			      struct stmmac_pci_info *info)
 {
 	struct pci_dev *pdev = info->pdev;
+	int ret;
+
+	/*
+	 * Refuse to load the driver and register net device if MAC controller
+	 * does not connect to any PHY interface.
+	 */
+	ret = stmmac_pci_find_phy_addr(info);
+	if (ret < 0)
+		return ret;
 
 	plat->bus_id = PCI_DEVID(pdev->bus->number, pdev->devfn);
-	plat->phy_addr = 1;
+	plat->phy_addr = ret;
 	plat->interface = PHY_INTERFACE_MODE_RMII;
 	plat->clk_csr = 2;
 	plat->has_gmac = 1;
@@ -82,8 +127,23 @@ static int quark_default_data(struct plat_stmmacenet_data *plat,
 	return 0;
 }
 
+static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = {
+	{
+		.name = "Galileo",
+		.func = 6,
+		.phy_addr = 1,
+	},
+	{
+		.name = "GalileoGen2",
+		.func = 6,
+		.phy_addr = 1,
+	},
+	{}
+};
+
 static struct stmmac_pci_info quark_pci_info = {
 	.setup = quark_default_data,
+	.dmi = quark_pci_dmi_data,
 };
 
 /**
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCH net-next 06/10] net/mlx4_core: Fix struct mlx4_vhcr_cmd to make implicit padding explicit
From: Jack Morgenstein @ 2015-01-27 19:50 UTC (permalink / raw)
  To: David Miller; +Cc: David.Laight, amirv, netdev, ogerlitz, yevgenyp
In-Reply-To: <20150127.111334.501141232585900738.davem@davemloft.net>

On Tue, 27 Jan 2015 11:13:34 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> Indeed, I'm really sick of seeing these packed structures being
> created all over the place.
> 
> They are to be used in absolutely extreme cases where no other
> solution is possible.

The V2 of this patch does not have "packed".

-Jack

^ permalink raw reply

* Interaction between GSO and TSQ for single TCP stream
From: Tom Herbert @ 2015-01-27 19:57 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Linux Netdev List

Eric,

I am looking at why we are unable to achieve line rate throughput with
a single connection using UDP encapsulation that supports GSO/GRO. I
discovered that even for a non-encpasulated TCP connection I'm not
able to get line rate using GSO/GRO (but do with TSO/LRO). There seems
to be some interaction between TSQ and GSO. Setting
tcp_limit_output_bytes to 1000000 gets single connection up to line
rate for both native and encapsulated TCP.

Some data:

- netperf TCP_STREAM for a single connection no encapsulation
  Using TSO/LRO
    9412.44 Gbps
  Using GSO/GRO (ethtool -K eth0 gso on gro on tso off lro off)
    9286.08 Gbps
  Using GSO/GRO with larger limit (echo 1000000 >
/proc/sys/net/ipv4/tcp_limit_output_bytes)
    9412.75

- netperf TCP_STREAM for a single connection for GUE-IPIP encapsulation with RCO
  Using GSO/GRO
    8842.89 Gbps
  Using GSO/GRO with larger limit (echo 1000000 >
/proc/sys/net/ipv4/tcp_limit_output_bytes)
    9143.99 Gbps

Any ideas on how to address this?

Thanks,
Tom

^ permalink raw reply

* Re: [PATCH 7/7] net: wireless: wcn36xx: handle new trigger_ba format
From: Eugene Krasnikov @ 2015-01-27 19:58 UTC (permalink / raw)
  To: Andy Green; +Cc: Kalle Valo, wcn36xx, linux-wireless, netdev
In-Reply-To: <20150118051117.31866.68634.stgit@114-36-241-182.dynamic.hinet.net>

arg... it looks like the code is starting to have to many
if(chip_version) cases. Mabe we should concider separate files for
chip specific logic.

2015-01-18 5:11 GMT+00:00 Andy Green <andy.green@linaro.org>:
> wcn3620 has a new message structure for the reply to trigger_ba
> We don't know what to do with the candidate list he sends back,
> but we can at least accept and ignore it nicely instead of dying.
>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---
>  drivers/net/wireless/ath/wcn36xx/smd.c |   28 ++++++++++++++++++++++++++--
>  drivers/net/wireless/ath/wcn36xx/smd.h |    9 +++++++++
>  2 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
> index 819741c..dc24e1b 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
> @@ -243,8 +243,31 @@ static int wcn36xx_smd_rsp_status_check(void *buf, size_t len)
>         rsp = (struct wcn36xx_fw_msg_status_rsp *)
>                 (buf + sizeof(struct wcn36xx_hal_msg_header));
>
> -       if (WCN36XX_FW_MSG_RESULT_SUCCESS != rsp->status)
> +       if (WCN36XX_FW_MSG_RESULT_SUCCESS != rsp->status) {
> +               pr_err("%s: bad status, len = %d\n", __func__, len);
> +               return rsp->status;
> +       }
> +
> +       return 0;
> +}
> +
> +static int wcn36xx_smd_rsp_status_check_bav2(struct wcn36xx *wcn, void *buf,
> +                                            size_t len)
> +{
> +       struct wcn36xx_fw_msg_status_rspv2 *rsp;
> +
> +       if (wcn->chip_version != WCN36XX_CHIP_3620)
> +               return wcn36xx_smd_rsp_status_check(buf, len);
> +
> +       if (len < sizeof(struct wcn36xx_hal_msg_header) + sizeof(*rsp))
> +               return -EIO;
> +
> +       rsp = buf + sizeof(struct wcn36xx_hal_msg_header);
> +
> +       if (WCN36XX_FW_MSG_RESULT_SUCCESS != rsp->status) {
> +               pr_err("%s: bad status, len = %d\n", __func__, len);
>                 return rsp->status;
> +       }
>
>         return 0;
>  }
> @@ -1884,7 +1907,8 @@ int wcn36xx_smd_trigger_ba(struct wcn36xx *wcn, u8 sta_index)
>                 wcn36xx_err("Sending hal_trigger_ba failed\n");
>                 goto out;
>         }
> -       ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
> +       ret = wcn36xx_smd_rsp_status_check_bav2(wcn, wcn->hal_buf,
> +                                               wcn->hal_rsp_len);
>         if (ret) {
>                 wcn36xx_err("hal_trigger_ba response failed err=%d\n", ret);
>                 goto out;
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
> index 008d034..432d3b8 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.h
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.h
> @@ -44,6 +44,15 @@ struct wcn36xx_fw_msg_status_rsp {
>         u32     status;
>  } __packed;
>
> +/* wcn3620 returns this for tigger_ba */
> +
> +struct wcn36xx_fw_msg_status_rspv2 {
> +       u8      bss_id[6];
> +       u32     status __packed;
> +       u16     count_following_candidates __packed;
> +       /* candidate list follows */
> +};
> +
>  struct wcn36xx_hal_ind_msg {
>         struct list_head list;
>         u8 *msg;
>



-- 
Best regards,
Eugene

^ permalink raw reply

* Re: [PATCH 4/7] net: wireless: wcn36xx: introduce WCN36XX_HAL_AVOID_FREQ_RANGE_IND
From: Eugene Krasnikov @ 2015-01-27 20:01 UTC (permalink / raw)
  To: Andy Green; +Cc: Kalle Valo, wcn36xx, linux-wireless, netdev
In-Reply-To: <20150118051100.31866.37997.stgit@114-36-241-182.dynamic.hinet.net>

Do you know when is this message used? sounds important.

2015-01-18 5:11 GMT+00:00 Andy Green <andy.green@linaro.org>:
> WCN3620 firmware introduces a new async indication, we need to
> add it as a known message type so we can accept it
>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---
>  drivers/net/wireless/ath/wcn36xx/hal.h |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
> index a1f1127..b947de0 100644
> --- a/drivers/net/wireless/ath/wcn36xx/hal.h
> +++ b/drivers/net/wireless/ath/wcn36xx/hal.h
> @@ -345,6 +345,8 @@ enum wcn36xx_hal_host_msg_type {
>         WCN36XX_HAL_DHCP_START_IND = 189,
>         WCN36XX_HAL_DHCP_STOP_IND = 190,
>
> +       WCN36XX_HAL_AVOID_FREQ_RANGE_IND = 233,
> +
>         WCN36XX_HAL_MSG_MAX = WCN36XX_HAL_MSG_TYPE_MAX_ENUM_SIZE
>  };
>
>



-- 
Best regards,
Eugene

^ permalink raw reply

* Re: Interaction between GSO and TSQ for single TCP stream
From: Eric Dumazet @ 2015-01-27 20:02 UTC (permalink / raw)
  To: Tom Herbert, Neal Cardwell, Yuchung Cheng; +Cc: Linux Netdev List
In-Reply-To: <CA+mtBx8QSg+3VSiDgY6iqiJaQgScSd9FP-uxH8S5zGGv7tN1Yw@mail.gmail.com>

On Tue, Jan 27, 2015 at 11:57 AM, Tom Herbert <therbert@google.com> wrote:
> Eric,
>
> I am looking at why we are unable to achieve line rate throughput with
> a single connection using UDP encapsulation that supports GSO/GRO. I
> discovered that even for a non-encpasulated TCP connection I'm not
> able to get line rate using GSO/GRO (but do with TSO/LRO). There seems
> to be some interaction between TSQ and GSO. Setting
> tcp_limit_output_bytes to 1000000 gets single connection up to line
> rate for both native and encapsulated TCP.
>
> Some data:
>
> - netperf TCP_STREAM for a single connection no encapsulation
>   Using TSO/LRO
>     9412.44 Gbps
>   Using GSO/GRO (ethtool -K eth0 gso on gro on tso off lro off)
>     9286.08 Gbps
>   Using GSO/GRO with larger limit (echo 1000000 >
> /proc/sys/net/ipv4/tcp_limit_output_bytes)
>     9412.75
>
> - netperf TCP_STREAM for a single connection for GUE-IPIP encapsulation with RCO
>   Using GSO/GRO
>     8842.89 Gbps
>   Using GSO/GRO with larger limit (echo 1000000 >
> /proc/sys/net/ipv4/tcp_limit_output_bytes)
>     9143.99 Gbps
>
> Any ideas on how to address this?
>
> Thanks,
> Tom

Hi Tom.

Yes, Eyal Perry  reported the issue a while back, we plan to send
upstream patches to fix the issue today.

Make sure you tweaked coalescing parameters if you use mlx4 :

ethtool -C eth0 tx-usecs 4 tx-frames 4

Otherwise, the NIC accumulates too many packets before sending an
interrupt for TX completion.

Then you can either wait the patches, or try the one I sent :
http://permalink.gmane.org/gmane.linux.network/347023

^ permalink raw reply

* Re: Interaction between GSO and TSQ for single TCP stream
From: Tom Herbert @ 2015-01-27 20:08 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Neal Cardwell, Yuchung Cheng, Linux Netdev List
In-Reply-To: <CANn89iJCOuQfixdSp6TsPxpyvTxQSTLBJLgnsvbrbG2Am_p0-Q@mail.gmail.com>

On Tue, Jan 27, 2015 at 12:02 PM, Eric Dumazet <edumazet@google.com> wrote:
> On Tue, Jan 27, 2015 at 11:57 AM, Tom Herbert <therbert@google.com> wrote:
>> Eric,
>>
>> I am looking at why we are unable to achieve line rate throughput with
>> a single connection using UDP encapsulation that supports GSO/GRO. I
>> discovered that even for a non-encpasulated TCP connection I'm not
>> able to get line rate using GSO/GRO (but do with TSO/LRO). There seems
>> to be some interaction between TSQ and GSO. Setting
>> tcp_limit_output_bytes to 1000000 gets single connection up to line
>> rate for both native and encapsulated TCP.
>>
>> Some data:
>>
>> - netperf TCP_STREAM for a single connection no encapsulation
>>   Using TSO/LRO
>>     9412.44 Gbps
>>   Using GSO/GRO (ethtool -K eth0 gso on gro on tso off lro off)
>>     9286.08 Gbps
>>   Using GSO/GRO with larger limit (echo 1000000 >
>> /proc/sys/net/ipv4/tcp_limit_output_bytes)
>>     9412.75
>>
>> - netperf TCP_STREAM for a single connection for GUE-IPIP encapsulation with RCO
>>   Using GSO/GRO
>>     8842.89 Gbps
>>   Using GSO/GRO with larger limit (echo 1000000 >
>> /proc/sys/net/ipv4/tcp_limit_output_bytes)
>>     9143.99 Gbps
>>
>> Any ideas on how to address this?
>>
>> Thanks,
>> Tom
>
> Hi Tom.
>
> Yes, Eyal Perry  reported the issue a while back, we plan to send
> upstream patches to fix the issue today.
>
Awesome, looking forward to seeing them!

> Make sure you tweaked coalescing parameters if you use mlx4 :
>
> ethtool -C eth0 tx-usecs 4 tx-frames 4
>
It's bnx2x.

> Otherwise, the NIC accumulates too many packets before sending an
> interrupt for TX completion.
>
> Then you can either wait the patches, or try the one I sent :
> http://permalink.gmane.org/gmane.linux.network/347023

^ permalink raw reply

* Re: [PATCH v1] stmmac: prevent probe drivers to crash kernel
From: David Miller @ 2015-01-27 20:24 UTC (permalink / raw)
  To: andriy.shevchenko; +Cc: peppe.cavallaro, netdev
In-Reply-To: <1422376683-30555-1-git-send-email-andriy.shevchenko@linux.intel.com>

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Tue, 27 Jan 2015 18:38:03 +0200

> In the case when alloc_netdev fails we return NULL to a caller. But there is no
> check for NULL in the probe drivers. This patch changes NULL to an error
> pointer. The function description is amended to reflect what we may get
> returned.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied, thank you.

^ permalink raw reply

* [PATCH] net: don't OOPS on socket aio
From: David Miller @ 2015-01-27 20:26 UTC (permalink / raw)
  To: netdev; +Cc: hch


From: Christoph Hellwig <hch@lst.de>

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/socket.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index a2c33a4..418795c 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -869,9 +869,6 @@ static ssize_t sock_splice_read(struct file *file, loff_t *ppos,
 static struct sock_iocb *alloc_sock_iocb(struct kiocb *iocb,
 					 struct sock_iocb *siocb)
 {
-	if (!is_sync_kiocb(iocb))
-		BUG();
-
 	siocb->kiocb = iocb;
 	iocb->private = siocb;
 	return siocb;
-- 
1.7.11.7

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox