Netdev List
 help / color / mirror / Atom feed
* Many USB ethernet devices are broken over xhci
From: David Laight @ 2014-01-27 16:06 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sarah Sharp

Many of the net/usb ethernet drivers (including common ones like
the smsc95xx) will fail to transmit all packet length properly
when connected to a USB3 port (ie using the xhci driver).

The underlying problem is that they assume the host controller
will honour the URB_ZERO_PACKET flag - which usbnet sets because
they specify FLAG_SEND_ZLP.

However no one has ever added support for URB_ZERO_PACKET to
the xhci driver - so packets that need padding (probably 512
bytes after any header is added) will not be sent correctly
and may have very adverse effects on the usb target.

The ax179_178a driver avoids this by not setting FLAG_SEND_ZLP,
modifying the packet header, and appending an extra zero byte.
(Which has been responsible for its own set of panics.)

I don't think this can be fixed by just clearing (or ignoring)
FLAG_SEND_ZLP as the extra byte will also confuse things.

It needs to be fixed in the xhci code.

I wrote this patch a while ago - worked for me with the ax179_178a
driver. http://www.spinics.net/lists/linux-usb/msg97370.html

The patch is a bit difficult to read, the v1 version contained a copy of
the new function. http://www.spinics.net/lists/linux-usb/msg97183.html

I don't think anything significant has changed (in the main kernel
sources) since I wrote the patch.

	David




--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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 1/3] net: ethoc: don't advertise gigabit speed on attached PHY
From: Max Filippov @ 2014-01-27 16:17 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: linux-xtensa@linux-xtensa.org, netdev, devicetree@vger.kernel.org,
	LKML, Chris Zankel, Marc Gauthier, David S. Miller, Grant Likely,
	Rob Herring
In-Reply-To: <1390817904.2735.127.camel@deadeye.wl.decadent.org.uk>

Hi Ben,

On Mon, Jan 27, 2014 at 2:18 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
> On Mon, 2014-01-27 at 07:59 +0400, Max Filippov wrote:
>> OpenCores 10/100 Mbps MAC does not support speeds above 100 Mbps, but does
>> not disable advertisement when PHY supports them. This results in
>> non-functioning network when the MAC is connected to a gigabit PHY connected
>> to a gigabit switch.
>>
>> The fix is to disable gigabit speed advertisement on attached PHY
>> unconditionally.
>>
>> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
>> ---
>>  drivers/net/ethernet/ethoc.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/ethoc.c b/drivers/net/ethernet/ethoc.c
>> index 4de8cfd..0aa1a05 100644
>> --- a/drivers/net/ethernet/ethoc.c
>> +++ b/drivers/net/ethernet/ethoc.c
>> @@ -712,6 +712,8 @@ static int ethoc_open(struct net_device *dev)
>>               netif_start_queue(dev);
>>       }
>>
>> +     priv->phy->advertising &= ~(ADVERTISED_1000baseT_Full |
>> +                                 ADVERTISED_1000baseT_Half);
>>       phy_start(priv->phy);
>>       napi_enable(&priv->napi);
>>
>
> This is not sufficient to disable gigabit speeds; the supported mask
> should also be limited.  And it should be done even before the net

I tried that, but when I also limit supported mask the phy driver doesn't
touch gigabit advertising register int the genphy_config_advert at all.
That's probably right for ethtool interface, but ethoc doesn't support
ethtool.

> device is registered.
>
> Rather than poking into the phy_device structure directly from this
> driver, I think you should add a function in phylib for this purpose.

Like below?

---8<---
>From 347331f399626ecaa9a8e54252f55e0b6788772f Mon Sep 17 00:00:00 2001
From: Max Filippov <jcmvbkbc@gmail.com>
Date: Mon, 27 Jan 2014 04:01:40 +0400
Subject: [PATCH 1/3] net: ethoc: don't advertise gigabit speed on attached PHY

OpenCores 10/100 Mbps MAC does not support speeds above 100 Mbps, but does
not disable advertisement when PHY supports them. This results in
non-functioning network when the MAC is connected to a gigabit PHY connected
to a gigabit switch.

The fix is to disable gigabit speed advertisement on attached PHY
unconditionally.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 drivers/net/ethernet/ethoc.c | 3 +++
 include/linux/phy.h          | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/ethoc.c b/drivers/net/ethernet/ethoc.c
index 4de8cfd..e817d58 100644
--- a/drivers/net/ethernet/ethoc.c
+++ b/drivers/net/ethernet/ethoc.c
@@ -688,6 +688,9 @@ static int ethoc_mdio_probe(struct net_device *dev)
 	}

 	priv->phy = phy;
+	phy_update_adv(phy,
+		       ~(ADVERTISED_1000baseT_Full |
+			 ADVERTISED_1000baseT_Half), 0);
 	return 0;
 }

diff --git a/include/linux/phy.h b/include/linux/phy.h
index 48a4dc3..0282a8d 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -559,6 +559,11 @@ static inline int phy_read_status(struct phy_device *phydev) {
 	return phydev->drv->read_status(phydev);
 }

+static inline void phy_update_adv(struct phy_device *phydev, u32 mask, u32 set)
+{
+	phydev->advertising = (phydev->advertising & mask) | set;
+}
+
 int genphy_setup_forced(struct phy_device *phydev);
 int genphy_restart_aneg(struct phy_device *phydev);
 int genphy_config_aneg(struct phy_device *phydev);
-- 
1.8.1.4


-- 
Thanks.
-- Max

^ permalink raw reply related

* [PATCH] AF_PACKET: Add documentation for queue mapping fanout mode
From: Neil Horman @ 2014-01-27 16:43 UTC (permalink / raw)
  To: netdev; +Cc: Neil Horman, David S. Miller, Daniel Borkmann

Recently I added a new AF_PACKET fanout operation mode in commit
2d36097, but I forgot to document it.  Add PACKET_FANOUT_QM as an available mode
in the af_packet documentation.  Applies to net-next.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: Daniel Borkmann <dborkman@redhat.com>
---
 Documentation/networking/packet_mmap.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/networking/packet_mmap.txt b/Documentation/networking/packet_mmap.txt
index 91ffe1d..1404674 100644
--- a/Documentation/networking/packet_mmap.txt
+++ b/Documentation/networking/packet_mmap.txt
@@ -583,6 +583,7 @@ Currently implemented fanout policies are:
   - PACKET_FANOUT_CPU: schedule to socket by CPU packet arrives on
   - PACKET_FANOUT_RND: schedule to socket by random selection
   - PACKET_FANOUT_ROLLOVER: if one socket is full, rollover to another
+  - PACKET_FANOUT_QM: schedule to socket by skbs recorded queue_mapping
 
 Minimal example code by David S. Miller (try things like "./test eth0 hash",
 "./test eth0 lb", etc.):
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH] AF_PACKET: Add documentation for queue mapping fanout mode
From: Daniel Borkmann @ 2014-01-27 16:59 UTC (permalink / raw)
  To: Neil Horman; +Cc: netdev, David S. Miller
In-Reply-To: <1390840984-22246-1-git-send-email-nhorman@tuxdriver.com>

On 01/27/2014 05:43 PM, Neil Horman wrote:
> Recently I added a new AF_PACKET fanout operation mode in commit
> 2d36097, but I forgot to document it.  Add PACKET_FANOUT_QM as an available mode
> in the af_packet documentation.  Applies to net-next.
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: Daniel Borkmann <dborkman@redhat.com>

Thanks a bunch, Neil.

Acked-by: Daniel Borkmann <dborkman@redhat.com>

^ permalink raw reply

* [PATCH V4] net: ip, ipv6: handle gso skbs in forwarding path
From: Florian Westphal @ 2014-01-27 17:58 UTC (permalink / raw)
  To: netdev; +Cc: Florian Westphal

Marcelo Ricardo Leitner reported problems when the forwarding link
path has a lower mtu than the incoming link if the inbound interface
supports GRO.

Given:
Host <mtu1500> R1 <mtu1200> R2

Host sends tcp stream which is routed via R1 and R2.  R1 performs GRO.

In this case, the kernel will fail to send ICMP fragmentation needed
messages (or pkt too big for ipv6), as gso packets currently bypass the
dst mtu checks in forward path. Instead, Linux tries to send out packets
exceeding R1-R2 link mtu.

When locking route MTU on Host (i.e., no ipv4 DF bit set), R1 does
not fragment the packets when forwarding, and again tries to send out
packets exceeding R1-R2 link mtu.

This alters the forwarding dstmtu checks to take the individual gso
segment lengths into account.

For ipv6, we send out pkt too big error for gso if the individual
segments are too big.

For ipv4, we either send icmp fragmentation needed, or, if the DF bit
is not set, perform software segmentation and let the output path
create fragments when the packet is leaving the machine.
It is not 100% correct as the error message will contain the headers of
the GRO skb instead of the original/segmented one, but it seems to
work fine in my (limited) tests.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
Changes since V3:
 - use ip_dst_mtu_maybe_forward instead of dst_mtu
 - add comment wrt. DF bit not being set

Changes since V2:
 - make this thing apply to current -net tree
 - kill unused variables in ip_forward/ip6_output

Changes since V1:
 suggestions from Eric Dumazet:
  - skip more expensive computation for small packets in fwd path
  - use netif_skb_features() feature mask and remove GSO flags
    instead of using 0 feature set.

 include/linux/skbuff.h | 17 ++++++++++++
 net/ipv4/ip_forward.c  | 71 ++++++++++++++++++++++++++++++++++++++++++++++++--
 net/ipv6/ip6_output.c  | 17 ++++++++++--
 3 files changed, 101 insertions(+), 4 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index f589c9a..3ebbbe7 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2916,5 +2916,22 @@ static inline bool skb_head_is_locked(const struct sk_buff *skb)
 {
 	return !skb->head_frag || skb_cloned(skb);
 }
+
+/**
+ * skb_gso_network_seglen - Return length of individual segments of a gso packet
+ *
+ * @skb: GSO skb
+ *
+ * skb_gso_network_seglen is used to determine the real size of the
+ * individual segments, including Layer3 (IP, IPv6) and L4 headers (TCP/UDP).
+ *
+ * The MAC/L2 header is not accounted for.
+ */
+static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb)
+{
+	unsigned int hdr_len = skb_transport_header(skb) -
+			       skb_network_header(skb);
+	return hdr_len + skb_gso_transport_seglen(skb);
+}
 #endif	/* __KERNEL__ */
 #endif	/* _LINUX_SKBUFF_H */
diff --git a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c
index e9f1217..2a9602f 100644
--- a/net/ipv4/ip_forward.c
+++ b/net/ipv4/ip_forward.c
@@ -39,6 +39,71 @@
 #include <net/route.h>
 #include <net/xfrm.h>
 
+static bool ip_may_fragment(const struct sk_buff *skb)
+{
+	return unlikely((ip_hdr(skb)->frag_off & htons(IP_DF)) == 0) ||
+	       !skb->local_df;
+}
+
+static bool ip_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu)
+{
+	if (skb->len <= mtu || skb->local_df)
+		return false;
+
+	if (skb_is_gso(skb) && skb_gso_network_seglen(skb) <= mtu)
+		return false;
+
+	return true;
+}
+
+static bool ip_gso_exceeds_dst_mtu(const struct sk_buff *skb)
+{
+	unsigned int mtu;
+
+	if (skb->local_df || !skb_is_gso(skb))
+		return false;
+
+	mtu = ip_dst_mtu_maybe_forward(skb_dst(skb), true);
+
+	/* if seglen is > mtu, we must do software segmentation to
+	 * enable proper IP fragmentation on output.
+	 *
+	 * In > mtu case DF bit cannot be set since we'd have already
+	 * dropped skb and sent icmp frag needed error in ip_forward().
+	 */
+	return skb_gso_network_seglen(skb) > mtu;
+}
+
+/* called if GSO skb needs to be fragmented on forward */
+static int ip_forward_finish_gso(struct sk_buff *skb)
+{
+	netdev_features_t features = netif_skb_features(skb);
+	struct sk_buff *segs;
+	int ret = 0;
+
+	segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK);
+	if (IS_ERR(segs)) {
+		kfree_skb(skb);
+		return -ENOMEM;
+	}
+
+	consume_skb(skb);
+
+	do {
+		struct sk_buff *nskb = segs->next;
+		int err;
+
+		segs->next = NULL;
+		err = dst_output(segs);
+
+		if (err && ret == 0)
+			ret = err;
+		segs = nskb;
+	} while (segs);
+
+	return ret;
+}
+
 static int ip_forward_finish(struct sk_buff *skb)
 {
 	struct ip_options *opt	= &(IPCB(skb)->opt);
@@ -49,6 +114,9 @@ static int ip_forward_finish(struct sk_buff *skb)
 	if (unlikely(opt->optlen))
 		ip_forward_options(skb);
 
+	if (ip_gso_exceeds_dst_mtu(skb))
+		return ip_forward_finish_gso(skb);
+
 	return dst_output(skb);
 }
 
@@ -91,8 +159,7 @@ int ip_forward(struct sk_buff *skb)
 
 	IPCB(skb)->flags |= IPSKB_FORWARDED;
 	mtu = ip_dst_mtu_maybe_forward(&rt->dst, true);
-	if (unlikely(skb->len > mtu && !skb_is_gso(skb) &&
-		     (ip_hdr(skb)->frag_off & htons(IP_DF))) && !skb->local_df) {
+	if (!ip_may_fragment(skb) && ip_exceeds_mtu(skb, mtu)) {
 		IP_INC_STATS(dev_net(rt->dst.dev), IPSTATS_MIB_FRAGFAILS);
 		icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
 			  htonl(mtu));
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index ef02b26..070a2fa 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -342,6 +342,20 @@ static unsigned int ip6_dst_mtu_forward(const struct dst_entry *dst)
 	return mtu;
 }
 
+static bool ip6_pkt_too_big(const struct sk_buff *skb, unsigned int mtu)
+{
+	if (skb->len <= mtu || skb->local_df)
+		return false;
+
+	if (IP6CB(skb)->frag_max_size && IP6CB(skb)->frag_max_size > mtu)
+		return true;
+
+	if (skb_is_gso(skb) && skb_gso_network_seglen(skb) <= mtu)
+		return false;
+
+	return true;
+}
+
 int ip6_forward(struct sk_buff *skb)
 {
 	struct dst_entry *dst = skb_dst(skb);
@@ -466,8 +480,7 @@ int ip6_forward(struct sk_buff *skb)
 	if (mtu < IPV6_MIN_MTU)
 		mtu = IPV6_MIN_MTU;
 
-	if ((!skb->local_df && skb->len > mtu && !skb_is_gso(skb)) ||
-	    (IP6CB(skb)->frag_max_size && IP6CB(skb)->frag_max_size > mtu)) {
+	if (ip6_pkt_too_big(skb, mtu)) {
 		/* Again, force OUTPUT device used as source address */
 		skb->dev = dst->dev;
 		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
-- 
1.8.1.5

^ permalink raw reply related

* Re: [PATCH 2/2] net: ip, ipv6: handle gso skbs in forwarding path
From: Eric Dumazet @ 2014-01-27 18:22 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netdev
In-Reply-To: <1390810971-23959-2-git-send-email-fw@strlen.de>

On Mon, 2014-01-27 at 09:22 +0100, Florian Westphal wrote:

> +/* called if GSO skb needs to be fragmented on forward.  */
> +static int ip_forward_finish_gso(struct sk_buff *skb)
> +{
> +	netdev_features_t features = netif_skb_features(skb);
> +	struct sk_buff *segs;
> +	int ret = 0;
> +
> +	segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK);
> +	if (IS_ERR(segs)) {
> +		kfree_skb(skb);
> +		return -ENOMEM;
> +	}
> +
> +	consume_skb(skb);
> +
> +	do {
> +		struct sk_buff *nskb = segs->next;
> +		int err;
> +
> +		segs->next = NULL;
> +		err = dst_output(segs);
> +
> +		if (err && ret == 0)
> +			ret = err;
> +		segs = nskb;
> +	} while (segs);
> +
> +	return ret;
> +}
> +

Its still unclear if this is the best strategy.

TCP stream not using DF flag are very unlikely to care if we adjust
their MTU (lowering gso_size) at this point ?

^ permalink raw reply

* Re: [PATCH 1/3] net: ethoc: don't advertise gigabit speed on attached PHY
From: Florian Fainelli @ 2014-01-27 19:36 UTC (permalink / raw)
  To: Max Filippov
  Cc: Ben Hutchings,
	linux-xtensa-PjhNF2WwrV/0Sa2dR60CXw@public.gmane.org, netdev,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, LKML,
	Chris Zankel, Marc Gauthier, David S. Miller, Grant Likely,
	Rob Herring
In-Reply-To: <52E6868C.3070401-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

2014-01-27 Max Filippov <jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> Hi Ben,
>
> On Mon, Jan 27, 2014 at 2:18 PM, Ben Hutchings <ben-/+tVBieCtBitmTQ+vhA3Yw@public.gmane.org> wrote:
>> On Mon, 2014-01-27 at 07:59 +0400, Max Filippov wrote:
>>> OpenCores 10/100 Mbps MAC does not support speeds above 100 Mbps, but does
>>> not disable advertisement when PHY supports them. This results in
>>> non-functioning network when the MAC is connected to a gigabit PHY connected
>>> to a gigabit switch.
>>>
>>> The fix is to disable gigabit speed advertisement on attached PHY
>>> unconditionally.
>>>
>>> Signed-off-by: Max Filippov <jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>>  drivers/net/ethernet/ethoc.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/ethoc.c b/drivers/net/ethernet/ethoc.c
>>> index 4de8cfd..0aa1a05 100644
>>> --- a/drivers/net/ethernet/ethoc.c
>>> +++ b/drivers/net/ethernet/ethoc.c
>>> @@ -712,6 +712,8 @@ static int ethoc_open(struct net_device *dev)
>>>               netif_start_queue(dev);
>>>       }
>>>
>>> +     priv->phy->advertising &= ~(ADVERTISED_1000baseT_Full |
>>> +                                 ADVERTISED_1000baseT_Half);
>>>       phy_start(priv->phy);
>>>       napi_enable(&priv->napi);
>>>
>>
>> This is not sufficient to disable gigabit speeds; the supported mask
>> should also be limited.  And it should be done even before the net
>
> I tried that, but when I also limit supported mask the phy driver doesn't
> touch gigabit advertising register int the genphy_config_advert at all.
> That's probably right for ethtool interface, but ethoc doesn't support
> ethtool.

This is not right for libphy either, phydev->supported must reflect
that you support Gigabit or not.

Since ethoc supports libphy, there really is no reason not to
implement the ethtool_{get,set}_settings callbacks using
phy_mii_ioctl().

>
>> device is registered.
>>
>> Rather than poking into the phy_device structure directly from this
>> driver, I think you should add a function in phylib for this purpose.

All drivers are currently modifying phydev->advertising and
phydev->supported directly, most of them do it correctly as far as I
checked. It does make some sense to add a helper function though.

>
> Like below?
>
> ---8<---
> From 347331f399626ecaa9a8e54252f55e0b6788772f Mon Sep 17 00:00:00 2001
> From: Max Filippov <jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Date: Mon, 27 Jan 2014 04:01:40 +0400
> Subject: [PATCH 1/3] net: ethoc: don't advertise gigabit speed on attached PHY
>
> OpenCores 10/100 Mbps MAC does not support speeds above 100 Mbps, but does
> not disable advertisement when PHY supports them. This results in
> non-functioning network when the MAC is connected to a gigabit PHY connected
> to a gigabit switch.
>
> The fix is to disable gigabit speed advertisement on attached PHY
> unconditionally.
>
> Signed-off-by: Max Filippov <jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/net/ethernet/ethoc.c | 3 +++
>  include/linux/phy.h          | 5 +++++
>  2 files changed, 8 insertions(+)
>
> diff --git a/drivers/net/ethernet/ethoc.c b/drivers/net/ethernet/ethoc.c
> index 4de8cfd..e817d58 100644
> --- a/drivers/net/ethernet/ethoc.c
> +++ b/drivers/net/ethernet/ethoc.c
> @@ -688,6 +688,9 @@ static int ethoc_mdio_probe(struct net_device *dev)
>         }
>
>         priv->phy = phy;
> +       phy_update_adv(phy,
> +                      ~(ADVERTISED_1000baseT_Full |
> +                        ADVERTISED_1000baseT_Half), 0);
>         return 0;
>  }
>
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 48a4dc3..0282a8d 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -559,6 +559,11 @@ static inline int phy_read_status(struct phy_device *phydev) {
>         return phydev->drv->read_status(phydev);
>  }
>
> +static inline void phy_update_adv(struct phy_device *phydev, u32 mask, u32 set)
> +{
> +       phydev->advertising = (phydev->advertising & mask) | set;
> +}
> +

This should be a preliminary patch to this patchset, so you first
introduce the function, then use it in the driver, but the idea looks
good. There is room for updating quite some drivers out there since
those used to modify phydev->advertising and phydev->supported
directly without using an accessor.

>  int genphy_setup_forced(struct phy_device *phydev);
>  int genphy_restart_aneg(struct phy_device *phydev);
>  int genphy_config_aneg(struct phy_device *phydev);
> --
> 1.8.1.4
>
>
> --
> Thanks.
> -- Max
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Florian
--
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 3/3] net: ethoc: document OF bindings
From: Florian Fainelli @ 2014-01-27 19:45 UTC (permalink / raw)
  To: Max Filippov
  Cc: Rob Herring, linux-xtensa-PjhNF2WwrV/0Sa2dR60CXw@public.gmane.org,
	netdev, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Chris Zankel, Marc Gauthier, David S. Miller, Grant Likely,
	Rob Herring
In-Reply-To: <CAMo8BfKm_rzDDs9BpfQkaew9DP9T8y0RnjzT_1gzs9Xrbx2CLA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

2014-01-27 Max Filippov <jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> Hi Rob,
>
> On Mon, Jan 27, 2014 at 6:10 PM, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Sun, Jan 26, 2014 at 9:59 PM, Max Filippov <jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> Signed-off-by: Max Filippov <jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>>  .../devicetree/bindings/net/opencores-ethoc.txt    | 25 ++++++++++++++++++++++
>>>  1 file changed, 25 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/net/opencores-ethoc.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/opencores-ethoc.txt b/Documentation/devicetree/bindings/net/opencores-ethoc.txt
>>> new file mode 100644
>>> index 0000000..f7c6c94
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/opencores-ethoc.txt
>>> @@ -0,0 +1,25 @@
>>> +* OpenCores MAC 10/100 Mbps
>>> +
>>> +Required properties:
>>> +- compatible: Should be "opencores,ethoc".
>>
>> There are not different versions of IP or is the version probeable?
>
> AFAIK there's single version of this 10/100 MAC.
> It doesn't have any identification registers.

Since this is an IP block that people can modify due to its open
source nature, it would have been good to define a revision register
or such which would allow software to gate specific code based on that
revision.
>
>>> +- reg: Address and length of the register set for the device and of its
>>> +  descriptor memory.
>>> +- interrupts: Should contain ethoc interrupt.
>>> +
>>> +Optional properties:
>>> +- local-mac-address: 6 bytes, mac address
>>
>> There's a patch in progress to move all the standard network
>> properties to a common location, so you can remove this.
>
> Will do.
>
>>> +- clock-frequency: basic MAC frequency.
>>> +- mii-mgmt-clock-frequency: frequency of MII management bus. Together
>>> +  with clock-frequency determines the value programmed into the CLKDIV
>>> +  field of MIIMODER register.
>>> +
>>> +Examples:
>>> +
>>> +       enet0: ethoc@fd030000 {
>>> +               compatible = "opencores,ethoc";
>>> +               reg = <0xfd030000 0x4000 0xfd800000 0x4000>;
>>> +               interrupts = <1>;
>>> +               local-mac-address = [00 50 c2 13 6f 00];
>>> +               clock-frequency = <50000000>;
>>> +               mii-mgmt-clock-frequency = <5000000>;

5Mhz management clock? Can't you make it work with the standard 2.5Mhz
management clock? Is not there a risk not to be able to "talk" to some
PHY chips out there which do not support > 2.5Mhz management clock?

Since this is an ethoc specific property, should it be prefixed with "ethoc,"?
-- 
Florian
--
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] ethtool.8: Add missing 1G and 10G link modes, and full names for all modes
From: Jesse Brandeburg @ 2014-01-27 19:55 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev
In-Reply-To: <1390785735.2735.96.camel@deadeye.wl.decadent.org.uk>

On Mon, 27 Jan 2014 01:22:15 +0000
Ben Hutchings <ben@decadent.org.uk> wrote:

> The 1G and 10G backplane link modes were missing from the table for
> the 'advertise' keyword.  Most link modes had only speed and duplex
> listed; change them to be consistent with ethtool's own output.
> 
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> I applied this on top of your patch.

FWIW (I know you already committed it)

Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

---

Thanks Ben!

^ permalink raw reply

* Re: [PATCH 1/3] net: ethoc: don't advertise gigabit speed on attached PHY
From: Max Filippov @ 2014-01-27 20:36 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Ben Hutchings, linux-xtensa@linux-xtensa.org, netdev,
	devicetree@vger.kernel.org, LKML, Chris Zankel, Marc Gauthier,
	David S. Miller, Grant Likely, Rob Herring
In-Reply-To: <CAGVrzcZ15Ov4zFDkaX=YuK0xVa+_QVjha1TN0cnhoMHYh1Jx4Q@mail.gmail.com>

On Mon, Jan 27, 2014 at 11:36 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> 2014-01-27 Max Filippov <jcmvbkbc@gmail.com>:
>> On Mon, Jan 27, 2014 at 2:18 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
>>> On Mon, 2014-01-27 at 07:59 +0400, Max Filippov wrote:
>>>> OpenCores 10/100 Mbps MAC does not support speeds above 100 Mbps, but does
>>>> not disable advertisement when PHY supports them. This results in
>>>> non-functioning network when the MAC is connected to a gigabit PHY connected
>>>> to a gigabit switch.
>>>>
>>>> The fix is to disable gigabit speed advertisement on attached PHY
>>>> unconditionally.
>>>>
>>>> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
>>>> ---
>>>>  drivers/net/ethernet/ethoc.c | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/net/ethernet/ethoc.c b/drivers/net/ethernet/ethoc.c
>>>> index 4de8cfd..0aa1a05 100644
>>>> --- a/drivers/net/ethernet/ethoc.c
>>>> +++ b/drivers/net/ethernet/ethoc.c
>>>> @@ -712,6 +712,8 @@ static int ethoc_open(struct net_device *dev)
>>>>               netif_start_queue(dev);
>>>>       }
>>>>
>>>> +     priv->phy->advertising &= ~(ADVERTISED_1000baseT_Full |
>>>> +                                 ADVERTISED_1000baseT_Half);
>>>>       phy_start(priv->phy);
>>>>       napi_enable(&priv->napi);
>>>>
>>>
>>> This is not sufficient to disable gigabit speeds; the supported mask
>>> should also be limited.  And it should be done even before the net
>>
>> I tried that, but when I also limit supported mask the phy driver doesn't
>> touch gigabit advertising register int the genphy_config_advert at all.
>> That's probably right for ethtool interface, but ethoc doesn't support
>> ethtool.
>
> This is not right for libphy either, phydev->supported must reflect
> that you support Gigabit or not.

I'm sorry, does that mean that I must not touch phydev->supported,
or that I must update it too and somehow fix genphy_config_advert?

> Since ethoc supports libphy, there really is no reason not to
> implement the ethtool_{get,set}_settings callbacks using
> phy_mii_ioctl().

Ok, I'll add that.

>>> device is registered.
>>>
>>> Rather than poking into the phy_device structure directly from this
>>> driver, I think you should add a function in phylib for this purpose.
>
> All drivers are currently modifying phydev->advertising and
> phydev->supported directly, most of them do it correctly as far as I
> checked. It does make some sense to add a helper function though.

[...]

>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>> index 48a4dc3..0282a8d 100644
>> --- a/include/linux/phy.h
>> +++ b/include/linux/phy.h
>> @@ -559,6 +559,11 @@ static inline int phy_read_status(struct phy_device *phydev) {
>>         return phydev->drv->read_status(phydev);
>>  }
>>
>> +static inline void phy_update_adv(struct phy_device *phydev, u32 mask, u32 set)
>> +{
>> +       phydev->advertising = (phydev->advertising & mask) | set;
>> +}
>> +
>
> This should be a preliminary patch to this patchset, so you first
> introduce the function, then use it in the driver, but the idea looks
> good. There is room for updating quite some drivers out there since
> those used to modify phydev->advertising and phydev->supported
> directly without using an accessor.

What about those that do something like this:

phydev->advertising = phydev->supported;

leave them as is, or provide read accessor for phydev->supported?

-- 
Thanks.
-- Max

^ permalink raw reply

* Re: [PATCH 3/3] net: ethoc: document OF bindings
From: Max Filippov @ 2014-01-27 20:45 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Rob Herring, linux-xtensa-PjhNF2WwrV/0Sa2dR60CXw@public.gmane.org,
	netdev, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Chris Zankel, Marc Gauthier, David S. Miller, Grant Likely,
	Rob Herring
In-Reply-To: <CAGVrzcb4Y8zu6Q24-0d-uKppGE=HrvEXPUD2-Mo8r5JC80nDSw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Mon, Jan 27, 2014 at 11:45 PM, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 2014-01-27 Max Filippov <jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>> On Mon, Jan 27, 2014 at 6:10 PM, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> On Sun, Jan 26, 2014 at 9:59 PM, Max Filippov <jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

[...]

>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/net/opencores-ethoc.txt
>>>> @@ -0,0 +1,25 @@
>>>> +* OpenCores MAC 10/100 Mbps
>>>> +
>>>> +Required properties:
>>>> +- compatible: Should be "opencores,ethoc".
>>>
>>> There are not different versions of IP or is the version probeable?
>>
>> AFAIK there's single version of this 10/100 MAC.
>> It doesn't have any identification registers.
>
> Since this is an IP block that people can modify due to its open
> source nature, it would have been good to define a revision register
> or such which would allow software to gate specific code based on that
> revision.

Probably yes, though I haven't heard of any modified versions of this MAC
out there.

[...]

>>>> +Examples:
>>>> +
>>>> +       enet0: ethoc@fd030000 {
>>>> +               compatible = "opencores,ethoc";
>>>> +               reg = <0xfd030000 0x4000 0xfd800000 0x4000>;
>>>> +               interrupts = <1>;
>>>> +               local-mac-address = [00 50 c2 13 6f 00];
>>>> +               clock-frequency = <50000000>;
>>>> +               mii-mgmt-clock-frequency = <5000000>;
>
> 5Mhz management clock? Can't you make it work with the standard 2.5Mhz
> management clock? Is not there a risk not to be able to "talk" to some
> PHY chips out there which do not support > 2.5Mhz management clock?

Yes I can, it just didn't occur to me that there is a standard 2.5MHz setting.

> Since this is an ethoc specific property, should it be prefixed with "ethoc,"?

Is it worth keeping this parameter at all, or just always default to 2.5MHz?

-- 
Thanks.
-- Max
--
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 V4] net: ip, ipv6: handle gso skbs in forwarding path
From: David Miller @ 2014-01-27 20:57 UTC (permalink / raw)
  To: fw; +Cc: netdev
In-Reply-To: <1390845492-18780-1-git-send-email-fw@strlen.de>

From: Florian Westphal <fw@strlen.de>
Date: Mon, 27 Jan 2014 18:58:12 +0100

> Marcelo Ricardo Leitner reported problems when the forwarding link
> path has a lower mtu than the incoming link if the inbound interface
> supports GRO.
> 
> Given:
> Host <mtu1500> R1 <mtu1200> R2
> 
> Host sends tcp stream which is routed via R1 and R2.  R1 performs GRO.
> 
> In this case, the kernel will fail to send ICMP fragmentation needed
> messages (or pkt too big for ipv6), as gso packets currently bypass the
> dst mtu checks in forward path. Instead, Linux tries to send out packets
> exceeding R1-R2 link mtu.
> 
> When locking route MTU on Host (i.e., no ipv4 DF bit set), R1 does
> not fragment the packets when forwarding, and again tries to send out
> packets exceeding R1-R2 link mtu.
> 
> This alters the forwarding dstmtu checks to take the individual gso
> segment lengths into account.
> 
> For ipv6, we send out pkt too big error for gso if the individual
> segments are too big.
> 
> For ipv4, we either send icmp fragmentation needed, or, if the DF bit
> is not set, perform software segmentation and let the output path
> create fragments when the packet is leaving the machine.
> It is not 100% correct as the error message will contain the headers of
> the GRO skb instead of the original/segmented one, but it seems to
> work fine in my (limited) tests.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH] AF_PACKET: Add documentation for queue mapping fanout mode
From: David Miller @ 2014-01-27 20:57 UTC (permalink / raw)
  To: nhorman; +Cc: netdev, dborkman
In-Reply-To: <1390840984-22246-1-git-send-email-nhorman@tuxdriver.com>

From: Neil Horman <nhorman@tuxdriver.com>
Date: Mon, 27 Jan 2014 11:43:04 -0500

> Recently I added a new AF_PACKET fanout operation mode in commit
> 2d36097, but I forgot to document it.  Add PACKET_FANOUT_QM as an available mode
> in the af_packet documentation.  Applies to net-next.
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>

Applied.

^ permalink raw reply

* Re: [PATCH 2/2] net: ip, ipv6: handle gso skbs in forwarding path
From: David Miller @ 2014-01-27 20:58 UTC (permalink / raw)
  To: eric.dumazet; +Cc: fw, netdev
In-Reply-To: <1390846967.27806.75.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 27 Jan 2014 10:22:47 -0800

> Its still unclear if this is the best strategy.
> 
> TCP stream not using DF flag are very unlikely to care if we adjust
> their MTU (lowering gso_size) at this point ?

It's better than what happens now when the destination link has a lower
MTU, wouldn't you say?

^ permalink raw reply

* Re: [PATCH 2/2] net: ip, ipv6: handle gso skbs in forwarding path
From: David Miller @ 2014-01-27 21:08 UTC (permalink / raw)
  To: eric.dumazet; +Cc: fw, netdev
In-Reply-To: <20140127.125838.2234405619673167551.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Mon, 27 Jan 2014 12:58:38 -0800 (PST)

> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon, 27 Jan 2014 10:22:47 -0800
> 
>> Its still unclear if this is the best strategy.
>> 
>> TCP stream not using DF flag are very unlikely to care if we adjust
>> their MTU (lowering gso_size) at this point ?
> 
> It's better than what happens now when the destination link has a lower
> MTU, wouldn't you say?

In the mean time I'll hold off on this patch while you guys discuss
this.

^ permalink raw reply

* Re: [Xen-devel] [PATCH net-next v5] xen-netfront: clean up code in xennet_release_rx_bufs
From: David Miller @ 2014-01-27 21:09 UTC (permalink / raw)
  To: david.vrabel
  Cc: Annie.li, xen-devel, netdev, konrad.wilk, ian.campbell, wei.liu2
In-Reply-To: <52E63382.6090503@citrix.com>

From: David Vrabel <david.vrabel@citrix.com>
Date: Mon, 27 Jan 2014 10:22:58 +0000

> I think this should be applied to net (and tagged as a stable candidate)
> rather than net-next as this fixes are very big resource leak.

Then this subject line and commit message must be fixed to make it clear
that this is a BUG fix, rather then just a "clean up".

^ permalink raw reply

* Re: [PATCH 1/1] net: ipv4: Use PTR_ERR_OR_ZERO
From: David Miller @ 2014-01-27 21:11 UTC (permalink / raw)
  To: sachin.kamat; +Cc: netdev, patches
In-Reply-To: <1390805037-20540-1-git-send-email-sachin.kamat@linaro.org>

From: Sachin Kamat <sachin.kamat@linaro.org>
Date: Mon, 27 Jan 2014 12:13:57 +0530

> PTR_RET is deprecated. Use PTR_ERR_OR_ZERO instead. While at it
> also include missing err.h header.
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>

Applied.

^ permalink raw reply

* Re: [PATCH 3/3] net: ethoc: document OF bindings
From: Florian Fainelli @ 2014-01-27 21:12 UTC (permalink / raw)
  To: Max Filippov
  Cc: Rob Herring, linux-xtensa@linux-xtensa.org, netdev,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Chris Zankel, Marc Gauthier, David S. Miller, Grant Likely,
	Rob Herring
In-Reply-To: <CAMo8BfLhFyOY1+FJ0qh8iL969totUCmHdJ0LzyqBAzwi1-Ht=A@mail.gmail.com>

2014-01-27 Max Filippov <jcmvbkbc@gmail.com>:
> On Mon, Jan 27, 2014 at 11:45 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>> 2014-01-27 Max Filippov <jcmvbkbc@gmail.com>:
>>> On Mon, Jan 27, 2014 at 6:10 PM, Rob Herring <robherring2@gmail.com> wrote:
>>>> On Sun, Jan 26, 2014 at 9:59 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>
> [...]
>
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/net/opencores-ethoc.txt
>>>>> @@ -0,0 +1,25 @@
>>>>> +* OpenCores MAC 10/100 Mbps
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible: Should be "opencores,ethoc".
>>>>
>>>> There are not different versions of IP or is the version probeable?
>>>
>>> AFAIK there's single version of this 10/100 MAC.
>>> It doesn't have any identification registers.
>>
>> Since this is an IP block that people can modify due to its open
>> source nature, it would have been good to define a revision register
>> or such which would allow software to gate specific code based on that
>> revision.
>
> Probably yes, though I haven't heard of any modified versions of this MAC
> out there.
>
> [...]
>
>>>>> +Examples:
>>>>> +
>>>>> +       enet0: ethoc@fd030000 {
>>>>> +               compatible = "opencores,ethoc";
>>>>> +               reg = <0xfd030000 0x4000 0xfd800000 0x4000>;
>>>>> +               interrupts = <1>;
>>>>> +               local-mac-address = [00 50 c2 13 6f 00];
>>>>> +               clock-frequency = <50000000>;
>>>>> +               mii-mgmt-clock-frequency = <5000000>;
>>
>> 5Mhz management clock? Can't you make it work with the standard 2.5Mhz
>> management clock? Is not there a risk not to be able to "talk" to some
>> PHY chips out there which do not support > 2.5Mhz management clock?
>
> Yes I can, it just didn't occur to me that there is a standard 2.5MHz setting.
>
>> Since this is an ethoc specific property, should it be prefixed with "ethoc,"?
>
> Is it worth keeping this parameter at all, or just always default to 2.5MHz?

Your example lists 5Mhz, so I assume this is of some use for you on
the platform you are working with? My concern is that this might not
be compatible with all PHY devices out there and might create hard to
debug issues where you get some MDIO transactions to succeeds and some
which don't.
-- 
Florian

^ permalink raw reply

* Re: [PATCH v3 net-next 0/2] bonding: fix locking in bond_ab_arp_prob
From: David Miller @ 2014-01-27 21:13 UTC (permalink / raw)
  To: vfalico; +Cc: netdev, fubar, andy
In-Reply-To: <1390829852-1943-1-git-send-email-vfalico@redhat.com>

From: Veaceslav Falico <vfalico@redhat.com>
Date: Mon, 27 Jan 2014 14:37:30 +0100

> After the latest patches, on every call of bond_ab_arp_probe() without an
> active slave I see the following warning:
...
> It happens because in bond_ab_arp_probe() we change the flags of a slave
> without holding the RTNL lock.
> 
> To fix this - remove the useless curr_active_lock, RCUify it and lock RTNL
> while changing the slave's flags. Also, remove bond_ab_arp_probe() from
> under any locks in bond_ab_arp_mon().
> 
> CC: Jay Vosburgh <fubar@us.ibm.com>
> CC: Andy Gospodarek <andy@greyhouse.net>
> CC: netdev@vger.kernel.org
> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>

Series applied, thanks Vaeceslav.

^ permalink raw reply

* Re: [PATCH net] bnx2x: More Shutdown revisions
From: David Miller @ 2014-01-27 21:13 UTC (permalink / raw)
  To: yuvalmin; +Cc: netdev, ariele
In-Reply-To: <1390835518-29652-1-git-send-email-yuvalmin@broadcom.com>

From: Yuval Mintz <yuvalmin@broadcom.com>
Date: Mon, 27 Jan 2014 17:11:58 +0200

> Submission d9aee59 "bnx2x: Don't release PCI bars on shutdown" separated
> the PCI remove and shutdown flows, but pci_disable_device() is still
> being called on both.
> As a result, a dev_WARN_ONCE will be hit during shutdown for every bnx2x
> VF probed on a hypervisor (as its shutdown callback will be called and later
> pci_disable_sriov() will call its remove callback).
> 
> This calls the pci_disable_device() only on the remove flow.
> 
> Signed-off-by: Yuval Mintz <yuvalmin@broadcom.com>
> Signed-off-by: Ariel Elior <ariele@broadcom.com>

Applied.

^ permalink raw reply

* Re: [PATCH] AF_PACKET: Add documentation for queue mapping fanout mode
From: Andi Kleen @ 2014-01-27 21:48 UTC (permalink / raw)
  To: Neil Horman; +Cc: netdev, David S. Miller, Daniel Borkmann
In-Reply-To: <1390840984-22246-1-git-send-email-nhorman@tuxdriver.com>

Neil Horman <nhorman@tuxdriver.com> writes:

> Recently I added a new AF_PACKET fanout operation mode in commit
> 2d36097, but I forgot to document it.  Add PACKET_FANOUT_QM as an available mode
> in the af_packet documentation.  Applies to net-next.

Please also send a man page patch.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

^ permalink raw reply

* RE: [PATCH net-next] hyperv: Add support for physically discontinuous receive buffer
From: Haiyang Zhang @ 2014-01-27 21:47 UTC (permalink / raw)
  To: KY Srinivasan, David Miller
  Cc: netdev@vger.kernel.org, olaf@aepfle.de, jasowang@redhat.com,
	linux-kernel@vger.kernel.org,
	driverdev-devel@linuxdriverproject.org
In-Reply-To: <18b9150b090043b7b296593af994f8ee@BY2PR03MB299.namprd03.prod.outlook.com>



> -----Original Message-----
> From: KY Srinivasan
> Sent: Monday, January 20, 2014 5:11 PM
> To: Haiyang Zhang; David Miller
> Cc: netdev@vger.kernel.org; olaf@aepfle.de; jasowang@redhat.com; linux-
> kernel@vger.kernel.org; driverdev-devel@linuxdriverproject.org
> Subject: RE: [PATCH net-next] hyperv: Add support for physically discontinuous
> receive buffer
> 
> 
> 
> > -----Original Message-----
> > From: Haiyang Zhang
> > Sent: Monday, January 20, 2014 2:06 PM
> > To: David Miller
> > Cc: netdev@vger.kernel.org; KY Srinivasan; olaf@aepfle.de;
> > jasowang@redhat.com; linux-kernel@vger.kernel.org; driverdev-
> > devel@linuxdriverproject.org
> > Subject: RE: [PATCH net-next] hyperv: Add support for physically
> > discontinuous receive buffer
> >
> >
> >
> > > -----Original Message-----
> > > From: David Miller [mailto:davem@davemloft.net]
> > > Sent: Tuesday, January 14, 2014 5:32 PM
> > > To: Haiyang Zhang
> > > Cc: netdev@vger.kernel.org; KY Srinivasan; olaf@aepfle.de;
> > > jasowang@redhat.com; linux-kernel@vger.kernel.org; driverdev-
> > > devel@linuxdriverproject.org
> > > Subject: Re: [PATCH net-next] hyperv: Add support for physically
> > > discontinuous receive buffer
> > >
> > > From: Haiyang Zhang <haiyangz@microsoft.com>
> > > Date: Thu,  9 Jan 2014 14:24:47 -0800
> > >
> > > > This will allow us to use bigger receive buffer, and prevent
> > > > allocation failure due to fragmented memory.
> > > >
> > > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > > Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
> > >
> > > Not until you start using paged SKBs in netvsc_recv_callback.
> > >
> > > Whatever fragmention you think you're avoiding in the hyperv layer,
> > > you're still going to get from the:
> > >
> > > skb = netdev_alloc_skb_ip_align(net, packet->total_data_buflen);
> > >
> > > call there.
> > >
> > > This change makes no sense in isolation, therefore I'm not applying
> > > it until you also include the appropriate changes to avoid the same
> > > exact fragmentation issue in netvsc_drv.c as stated above.
> >
> > The receive buffer currently requires multiple MB of physically
> > continuous memory, and may fail to be allocated when memory is
> > fragmented. The patch is created for this issue.
> >
> > The SKB buffer is usually less than 1500 bytes or up to several KB
> > with jumbo frame, so it's much less sensitive to fragmented memory. I
> > will work on another patch to use SKB buffer with discontinuous pages.
> >
> > Could you accept this patch separately?
> 
> Today, if we try to unload and load the network driver, the load may fail
> because we may not be able to allocate the receive buffers if memory is
> fragmented. This patch specifically addresses this problem.
> 
> Regards,
> 
> K. Y

Dave,

So, could this patch be taken first?

Thanks,
- Haiyang

^ permalink raw reply

* Re: [PATCH net-next] hyperv: Add support for physically discontinuous receive buffer
From: David Miller @ 2014-01-27 21:50 UTC (permalink / raw)
  To: haiyangz; +Cc: olaf, netdev, jasowang, driverdev-devel, linux-kernel
In-Reply-To: <0671791e2ca8487a86a551eadf2e2bcd@DFM-DB3MBX15-06.exchange.corp.microsoft.com>

From: Haiyang Zhang <haiyangz@microsoft.com>
Date: Mon, 27 Jan 2014 21:47:43 +0000

> So, could this patch be taken first?

You always need to resend patches that I've originally rejected and
you've made arguments for.

^ permalink raw reply

* RE: [PATCH net-next] hyperv: Add support for physically discontinuous receive buffer
From: Haiyang Zhang @ 2014-01-27 21:55 UTC (permalink / raw)
  To: David Miller
  Cc: KY Srinivasan, netdev@vger.kernel.org, olaf@aepfle.de,
	jasowang@redhat.com, linux-kernel@vger.kernel.org,
	driverdev-devel@linuxdriverproject.org
In-Reply-To: <20140127.135036.201976640275761993.davem@davemloft.net>



> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Monday, January 27, 2014 4:51 PM
> To: Haiyang Zhang
> Cc: KY Srinivasan; netdev@vger.kernel.org; olaf@aepfle.de;
> jasowang@redhat.com; linux-kernel@vger.kernel.org; driverdev-
> devel@linuxdriverproject.org
> Subject: Re: [PATCH net-next] hyperv: Add support for physically discontinuous
> receive buffer
> 
> From: Haiyang Zhang <haiyangz@microsoft.com>
> Date: Mon, 27 Jan 2014 21:47:43 +0000
> 
> > So, could this patch be taken first?
> 
> You always need to resend patches that I've originally rejected and you've
> made arguments for.

Sure, I will re-send it now.

Thanks,
- Haiyang

^ permalink raw reply

* Re: [PATCH net-next 0/7] qlcnic: Refactoring and enhancements for all adapters.
From: Zoltan Kiss @ 2014-01-27 22:04 UTC (permalink / raw)
  To: David Miller, himanshu.madhani; +Cc: netdev, Dept_NX_Linux_NIC_Driver
In-Reply-To: <20140123.161437.1632835743136251770.davem@davemloft.net>

On 24/01/14 00:14, David Miller wrote:
> From: Himanshu Madhani <himanshu.madhani@qlogic.com>
> Date: Thu, 23 Jan 2014 17:18:27 -0500
>
>> This patch series contains following patches.
>>
>> o Enhanced debug data collection when we are in Tx-timeout situation.
>> o Enhanced MSI-x vector calculation for defualt load path as well
>>    as for TSS/RSS ring change path.
>> o Refactored interrupt coalescing code for all adapters.
>> o Refactored interrupt handling as well as cleanup of poll controller
>>    code patch for all adapters.
>> o changed rx_mac_learn type to boolean.
>>
>> Please apply to net-next.
>
> Series applied, thanks.

Not that I reject fame, but the commit message contains my name as 
sign-off, while I've never seen a bit of that code :)

Zoli

^ permalink raw reply


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