* [PATCHv2 net 1/4] net: Add vxlan_gso_check() helper
From: Joe Stringer @ 2014-11-14 0:38 UTC (permalink / raw)
To: netdev
Cc: sathya.perla, shahed.shaikh, amirv, Dept-GELinuxNICDev, therbert,
gerlitz.or, alexander.duyck, linux-kernel
In-Reply-To: <1415925495-59312-1-git-send-email-joestringer@nicira.com>
Most NICs that report NETIF_F_GSO_UDP_TUNNEL support VXLAN, and not
other UDP-based encapsulation protocols where the format and size of the
header differs. This patch implements a generic ndo_gso_check() for
VXLAN which will only advertise GSO support when the skb looks like it
contains VXLAN (or no UDP tunnelling at all).
Implementation shamelessly stolen from Tom Herbert:
http://thread.gmane.org/gmane.linux.network/332428/focus=333111
Signed-off-by: Joe Stringer <joestringer@nicira.com>
---
v2: Merge helpers for be2net, mlx4, qlcnic
Use (sizeof(struct udphdr) + sizeof(struct vxlanhdr))
v1: Initial post
---
drivers/net/vxlan.c | 13 +++++++++++++
include/net/vxlan.h | 2 ++
2 files changed, 15 insertions(+)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index fa9dc45..6b65863 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1571,6 +1571,19 @@ static bool route_shortcircuit(struct net_device *dev, struct sk_buff *skb)
return false;
}
+bool vxlan_gso_check(struct sk_buff *skb)
+{
+ if ((skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL) &&
+ (skb->inner_protocol_type != ENCAP_TYPE_ETHER ||
+ skb->inner_protocol != htons(ETH_P_TEB) ||
+ (skb_inner_mac_header(skb) - skb_transport_header(skb) !=
+ sizeof(struct udphdr) + sizeof(struct vxlanhdr))))
+ return false;
+
+ return true;
+}
+EXPORT_SYMBOL_GPL(vxlan_gso_check);
+
#if IS_ENABLED(CONFIG_IPV6)
static int vxlan6_xmit_skb(struct vxlan_sock *vs,
struct dst_entry *dst, struct sk_buff *skb,
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index d5f59f3..afadf8e 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -45,6 +45,8 @@ int vxlan_xmit_skb(struct vxlan_sock *vs,
__be32 src, __be32 dst, __u8 tos, __u8 ttl, __be16 df,
__be16 src_port, __be16 dst_port, __be32 vni, bool xnet);
+bool vxlan_gso_check(struct sk_buff *skb);
+
/* IP header + UDP + VXLAN + Ethernet header */
#define VXLAN_HEADROOM (20 + 8 + 8 + 14)
/* IPv6 header + UDP + VXLAN + Ethernet header */
--
1.7.10.4
^ permalink raw reply related
* [PATCHv2 net 0/4] Implement ndo_gso_check() for vxlan nics
From: Joe Stringer @ 2014-11-14 0:38 UTC (permalink / raw)
To: netdev
Cc: sathya.perla, shahed.shaikh, amirv, Dept-GELinuxNICDev, therbert,
gerlitz.or, linux-kernel
Most NICs that report NETIF_F_GSO_UDP_TUNNEL support VXLAN, and not other
UDP-based encapsulation protocols where the format and size of the header may
differ. This patch series implements a generic ndo_gso_check() for detecting
VXLAN, then reuses it for these NICs.
Implementation shamelessly stolen from Tom Herbert (with minor fixups):
http://thread.gmane.org/gmane.linux.network/332428/focus=333111
v2: Drop i40e/fm10k patches (code diverged; handling separately).
Refactor common code into vxlan_gso_check() helper.
Minor style fixes.
Joe Stringer (4):
net: Add vxlan_gso_check() helper
be2net: Implement ndo_gso_check()
net/mlx4_en: Implement ndo_gso_check()
qlcnic: Implement ndo_gso_check()
drivers/net/ethernet/emulex/benet/be_main.c | 6 ++++++
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 6 ++++++
drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 6 ++++++
drivers/net/vxlan.c | 13 +++++++++++++
include/net/vxlan.h | 2 ++
5 files changed, 33 insertions(+)
--
1.7.10.4
^ permalink raw reply
* Re: [PATCHv2 net 2/2] fm10k: Implement ndo_gso_check()
From: Joe Stringer @ 2014-11-14 0:29 UTC (permalink / raw)
To: Jeff Kirsher
Cc: Shannon Nelson, Jesse Brandeburg, netdev, matthew.vick,
linux.nics, therbert, gerlitz.or, alexander.duyck, linux-kernel,
jesse
In-Reply-To: <1415923959.2454.44.camel@jtkirshe-mobl>
On Thursday, November 13, 2014 16:12:39 Jeff Kirsher wrote:
> On Thu, 2014-11-13 at 15:52 -0800, Joe Stringer wrote:
> > On Thu, Nov 13, 2014 at 03:41:56PM -0800, Jeff Kirsher wrote:
> > > On Thu, 2014-11-13 at 15:36 -0800, Joe Stringer wrote:
> > > > ndo_gso_check() was recently introduced to allow NICs to report
> >
> > the
> >
> > > > offloading support that they have on a per-skb basis. Add an
> > > > implementation for this driver which checks for something that
> >
> > looks
> >
> > > > like VXLAN.
> > > >
> > > > Signed-off-by: Joe Stringer <joestringer@nicira.com>
> > > > ---
> > > > v2: Reuse fm10k_tx_encap_offload().
> > > > ---
> > > >
> > > > drivers/net/ethernet/intel/fm10k/fm10k.h | 1 +
> > > > drivers/net/ethernet/intel/fm10k/fm10k_main.c | 2 +-
> > > > drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 8 ++++++++
> > > > 3 files changed, 10 insertions(+), 1 deletion(-)
> > >
> > > Same with this one as well, thanks Joe.
> >
> > Thanks Jeff.
> >
> > Could you remind me, is the equivalent i40e patch on your queue or
> > were
> > we still waiting on further feedback from Shannon/Jesse?
>
> Actually, looks like I dropped the patch due to community feedback and
> was expecting a v2. Was I incorrect in doing so?
That's fine. There were some unresolved questions for what that version should
look like, but I can repost to start the discussion again.
Cheers,
Joe
^ permalink raw reply
* (unknown),
From: Omar Hashim @ 2014-11-14 0:04 UTC (permalink / raw)
--
I have a business proposal with mutual
benefits for you.
Regards,
Omar Hashim
--
^ permalink raw reply
* My Private Email: liliane.hbettencourt@yahoo.fr
From: Hacer Selamoğlu Çağlayan @ 2014-11-14 0:10 UTC (permalink / raw)
I, Liliane authenticate this email, you can read about me on: http://en.wikipedia.org/wiki/Liliane_Bettencourt I write to you because I intend to give you a portion of my Net-worth which I have been banking. I want to cede it out as gift hoping it would be of help to you and others too. Respond for confirmation.
My Private Email: liliane.hbettencourt@yahoo.fr
With love,
Liliane H Bettencourt
________________________________
Kişiye özel bu mesaj ve içeriğindeki bilgiler gizlidir. Mesaj içeriğinde bulunan bilgi, fikir ve yorumlar, sadece göndericiye aittir. T.C. Çevre ve Şehircilik Bakanlığı bu mesajın içeriği ve ekleri ile ilgili olarak hukuksal hiçbir sorumluluk kabul etmez. Yetkili alıcılardan biri değilseniz, bu mesajın herhangi bir şekilde ifşa edilmesi, kullanılması, kopyalanması, yayılması veya mesajda yer alan hususlarla ilgili olarak herhangi bir işlem yapılmasının kesinlikle yasak olduğunu bildiririz. Böyle bir durumda lütfen hemen mesajın göndericisini bilgilendiriniz ve mesajı sisteminizden siliniz. İnternet ortamında gönderilen e-posta mesajlarındaki hata ve/veya eksikliklerden veya virüslerden dolayı mesajın göndericisi herhangi bir sorumluluk kabul etmemektedir.
Teşekkür ederiz.
*** Bu mail zararlı içeriğe karşı, T.C. Çevre ve Şehircilik Bakanlığı Antivirus Sistemleri tarafından taranmıştır. ***
The information contained in this communication may contain confidential or legally privileged information. Responsibility about sent contents belongs to the sender. The Ministry of Environment and Urbanism doesn't accept any legal responsibility for the contents and attachments of this message. If you are not the intended recipient you are hereby notified that any disclosure, use, copying, distribution or taking any action in reliance on the contents of this information is strictly prohibited. If you have received this communication in error, please notify the sender immediately by responding to this e-mail and then delete it from your system. The sender does not accept any liability for any errors or omissions or any viruses in the context of this message which arise as a result of internet transmission.
Thank you.
*** This mail was scanned for known viruses by The Ministry of Environment and Urbanism Antivirus systems. ***
^ permalink raw reply
* Re: [PATCHv2 net 2/2] fm10k: Implement ndo_gso_check()
From: Jeff Kirsher @ 2014-11-14 0:12 UTC (permalink / raw)
To: Joe Stringer
Cc: Shannon Nelson, Jesse Brandeburg, netdev, matthew.vick,
linux.nics, therbert, gerlitz.or, alexander.duyck, linux-kernel
In-Reply-To: <20141113235219.GA35957@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1156 bytes --]
On Thu, 2014-11-13 at 15:52 -0800, Joe Stringer wrote:
> On Thu, Nov 13, 2014 at 03:41:56PM -0800, Jeff Kirsher wrote:
> > On Thu, 2014-11-13 at 15:36 -0800, Joe Stringer wrote:
> > > ndo_gso_check() was recently introduced to allow NICs to report
> the
> > > offloading support that they have on a per-skb basis. Add an
> > > implementation for this driver which checks for something that
> looks
> > > like VXLAN.
> > >
> > > Signed-off-by: Joe Stringer <joestringer@nicira.com>
> > > ---
> > > v2: Reuse fm10k_tx_encap_offload().
> > > ---
> > > drivers/net/ethernet/intel/fm10k/fm10k.h | 1 +
> > > drivers/net/ethernet/intel/fm10k/fm10k_main.c | 2 +-
> > > drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 8 ++++++++
> > > 3 files changed, 10 insertions(+), 1 deletion(-)
> >
> > Same with this one as well, thanks Joe.
>
> Thanks Jeff.
>
> Could you remind me, is the equivalent i40e patch on your queue or
> were
> we still waiting on further feedback from Shannon/Jesse?
Actually, looks like I dropped the patch due to community feedback and
was expecting a v2. Was I incorrect in doing so?
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* linux-next: ath9k: build failure, ath_cmn_process_fft() redefinition
From: Jeremiah Mahler @ 2014-11-14 0:07 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Jeremiah Mahler, linux-kernel, ath9k-devel, linville,
linux-wireless, ath9k-devel, netdev
In version 20141113 of the linux-next kernel, if it is compiled with
CONFIG_ATH9K_DEBUGFS unset, an error about ath_cmn_process_fft() being
redefined will be produced.
make
...
LD [M] drivers/net/wireless/ath/ath9k/ath9k_hw.o
CC [M] drivers/net/wireless/ath/ath9k/common-spectral.o
CC lib/debug_locks.o
CC lib/random32.o
drivers/net/wireless/ath/ath9k/common-spectral.c:40:5: error:
redefinition of ‘ath_cmn_process_fft’
int ath_cmn_process_fft(struct ath_spec_scan_priv *spec_priv, struct
ieee80211_hdr *hdr,
^
In file included from drivers/net/wireless/ath/ath9k/common.h:27:0,
from drivers/net/wireless/ath/ath9k/ath9k.h:27,
from
drivers/net/wireless/ath/ath9k/common-spectral.c:18:
drivers/net/wireless/ath/ath9k/common-spectral.h:146:19: note: previous
definition of ‘ath_cmn_process_fft’ was here
static inline int ath_cmn_process_fft(struct ath_spec_scan_priv
*spec_priv,
^
scripts/Makefile.build:257: recipe for target
'drivers/net/wireless/ath/ath9k/common-spectral.o' failed
make[5]: *** [drivers/net/wireless/ath/ath9k/common-spectral.o] Error 1
scripts/Makefile.build:402: recipe for target
'drivers/net/wireless/ath/ath9k' failed
make[4]: *** [drivers/net/wireless/ath/ath9k] Error 2
scripts/Makefile.build:402: recipe for target 'drivers/net/wireless/ath'
failed
make[3]: *** [drivers/net/wireless/ath] Error 2
scripts/Makefile.build:402: recipe for target 'drivers/net/wireless'
failed
make[2]: *** [drivers/net/wireless] Error 2
scripts/Makefile.build:402: recipe for target 'drivers/net' failed
make[1]: *** [drivers/net] Error 2
Makefile:953: recipe for target 'drivers' failed
make: *** [drivers] Error 2
make: *** Waiting for unfinished jobs....
CC lib/bust_spinlocks.o
...
Bisecting the kernel found that the following patch was the cause.
commit 67dc74f15f147b9f88702de2952d2951e3e000ec
Author: Oleksij Rempel <linux@rempel-privat.de>
Date: Thu Nov 6 08:53:30 2014 +0100
ath9k: move spectral.* to common-spectral.*
and rename exports from ath9k_spectral_* to ath9k_cmn_spectral_*
Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
This patch mostly consists of renaming functions and moving code but
there was a functional change to the Makefile.
common-spectral.h uses CONFIG_ATH9K_DEBUGFS to conditionally provide a
prototype of ath_cmn_process_fft() when set or to define it as a noop
when it is unset. The Makefile was changed so that CONFIG_ATH9K_DEBUGFS
no longer applied to common-spectral and this will result in two
definitions of ath_cmn_process_fft().
> --- a/drivers/net/wireless/ath/ath9k/Makefile
> +++ b/drivers/net/wireless/ath/ath9k/Makefile
> @@ -16,8 +16,7 @@ ath9k-$(CONFIG_ATH9K_DFS_CERTIFIED) += dfs.o
> ath9k-$(CONFIG_ATH9K_TX99) += tx99.o
> ath9k-$(CONFIG_ATH9K_WOW) += wow.o
>
> -ath9k-$(CONFIG_ATH9K_DEBUGFS) += debug.o \
> - spectral.o
> +ath9k-$(CONFIG_ATH9K_DEBUGFS) += debug.o
>
> ath9k-$(CONFIG_ATH9K_STATION_STATISTICS) += debug_sta.o
>
> @@ -59,7 +58,8 @@ obj-$(CONFIG_ATH9K_COMMON) += ath9k_common.o
> ath9k_common-y:= common.o \
> common-init.o \
> common-beacon.o \
> - common-debug.o
> + common-debug.o \
> + common-spectral.o
Reverting the patch solves one error, but then a new one is produced.
make
...
MODPOST 185 modules
CC arch/x86/boot/edd.o
VOFFSET arch/x86/boot/voffset.h
ERROR: "ath9k_cmn_spectral_scan_trigger"
[drivers/net/wireless/ath/ath9k/ath9k.ko] undefined!
scripts/Makefile.modpost:90: recipe for target '__modpost' failed
make[1]: *** [__modpost] Error 1
...
This error is caused by the patch before it.
commit f00a422cc81ef665f5098c0bc43cb0c616e55a9b
Author: Oleksij Rempel <linux@rempel-privat.de>
Date: Thu Nov 6 08:53:29 2014 +0100
ath9k: move ath9k_spectral_scan_ from main.c to spectral.c
Now we should be ready to make this code common.
Signed-off-by: Oleksij Rempel <linux@rempel-privat.de>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
Since the code was moved from main.c to spectral.c, it is now involved
with CONFIG_ATH9K_DEBUGFS, which causes it to break.
Reverting both the above patches resolves the build errors.
--
Jeremiah Mahler
jmmahler@gmail.com
http://github.com/jmahler
^ permalink raw reply
* Re: arm64 allmodconfig failures in nft_reject_bridge.c
From: Mark Brown @ 2014-11-13 23:51 UTC (permalink / raw)
To: David Miller
Cc: pablo, linux, kaber, kadlec, stephen, linaro-kernel,
kernel-build-reports, netfilter-devel, coreteam, bridge, netdev
In-Reply-To: <20141113.152353.548265176661091467.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 579 bytes --]
On Thu, Nov 13, 2014 at 03:23:53PM -0500, David Miller wrote:
> Date: Thu, 13 Nov 2014 19:47:52 +0000
> > On Thu, Nov 13, 2014 at 02:35:13PM -0500, David Miller wrote:
> >> I hold changes in my tree for a week or more, because I want them to
> >> "cook" there before they go to Linus.
> > Hrm. Guess there must've been some other change in -next that pulled
> > the header in implicitly here :(
> -next pulls in my 'net' tree, so got the fix.
Right, but we didn't see the problem before you sent the tree with the
problematic patch to Linus - we didn't detect it in -next.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCHv2 net 2/2] fm10k: Implement ndo_gso_check()
From: Joe Stringer @ 2014-11-13 23:52 UTC (permalink / raw)
To: Jeff Kirsher, Shannon Nelson, Jesse Brandeburg
Cc: netdev, matthew.vick, linux.nics, therbert, gerlitz.or,
alexander.duyck, linux-kernel
In-Reply-To: <1415922116.2454.34.camel@jtkirshe-mobl>
On Thu, Nov 13, 2014 at 03:41:56PM -0800, Jeff Kirsher wrote:
> On Thu, 2014-11-13 at 15:36 -0800, Joe Stringer wrote:
> > ndo_gso_check() was recently introduced to allow NICs to report the
> > offloading support that they have on a per-skb basis. Add an
> > implementation for this driver which checks for something that looks
> > like VXLAN.
> >
> > Signed-off-by: Joe Stringer <joestringer@nicira.com>
> > ---
> > v2: Reuse fm10k_tx_encap_offload().
> > ---
> > drivers/net/ethernet/intel/fm10k/fm10k.h | 1 +
> > drivers/net/ethernet/intel/fm10k/fm10k_main.c | 2 +-
> > drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 8 ++++++++
> > 3 files changed, 10 insertions(+), 1 deletion(-)
>
> Same with this one as well, thanks Joe.
Thanks Jeff.
Could you remind me, is the equivalent i40e patch on your queue or were
we still waiting on further feedback from Shannon/Jesse?
^ permalink raw reply
* Re: [PATCHv2 net 2/2] fm10k: Implement ndo_gso_check()
From: Jeff Kirsher @ 2014-11-13 23:41 UTC (permalink / raw)
To: Joe Stringer
Cc: netdev, matthew.vick, linux.nics, therbert, gerlitz.or,
alexander.duyck, linux-kernel
In-Reply-To: <1415921801-10452-2-git-send-email-joestringer@nicira.com>
[-- Attachment #1: Type: text/plain, Size: 678 bytes --]
On Thu, 2014-11-13 at 15:36 -0800, Joe Stringer wrote:
> ndo_gso_check() was recently introduced to allow NICs to report the
> offloading support that they have on a per-skb basis. Add an
> implementation for this driver which checks for something that looks
> like VXLAN.
>
> Signed-off-by: Joe Stringer <joestringer@nicira.com>
> ---
> v2: Reuse fm10k_tx_encap_offload().
> ---
> drivers/net/ethernet/intel/fm10k/fm10k.h | 1 +
> drivers/net/ethernet/intel/fm10k/fm10k_main.c | 2 +-
> drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 8 ++++++++
> 3 files changed, 10 insertions(+), 1 deletion(-)
Same with this one as well, thanks Joe.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCHv2 net 1/2] fm10k: Check tunnel header length in encap offload
From: Jeff Kirsher @ 2014-11-13 23:41 UTC (permalink / raw)
To: Joe Stringer
Cc: netdev, matthew.vick, linux.nics, therbert, gerlitz.or,
alexander.duyck, linux-kernel
In-Reply-To: <1415921801-10452-1-git-send-email-joestringer@nicira.com>
[-- Attachment #1: Type: text/plain, Size: 579 bytes --]
On Thu, 2014-11-13 at 15:36 -0800, Joe Stringer wrote:
> fm10k supports up to 184 bytes of inner+outer headers. Add an initial
> check to fail encap offload if these are too large.
>
> Signed-off-by: Joe Stringer <joestringer@nicira.com>
> ---
> Matthew, I didn't see the equivalent patch on netdev so I went ahead
> and
> created it. If I've missed this somewhere, then please disregard.
>
> v2: First post.
> ---
> drivers/net/ethernet/intel/fm10k/fm10k_main.c | 6 ++++++
> 1 file changed, 6 insertions(+)
Thanks Joe, I will add your patch to my queue.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCHv2 net 2/2] fm10k: Implement ndo_gso_check()
From: Joe Stringer @ 2014-11-13 23:36 UTC (permalink / raw)
To: netdev
Cc: matthew.vick, jeffrey.t.kirsher, linux.nics, therbert, gerlitz.or,
alexander.duyck, linux-kernel
In-Reply-To: <1415921801-10452-1-git-send-email-joestringer@nicira.com>
ndo_gso_check() was recently introduced to allow NICs to report the
offloading support that they have on a per-skb basis. Add an
implementation for this driver which checks for something that looks
like VXLAN.
Signed-off-by: Joe Stringer <joestringer@nicira.com>
---
v2: Reuse fm10k_tx_encap_offload().
---
drivers/net/ethernet/intel/fm10k/fm10k.h | 1 +
drivers/net/ethernet/intel/fm10k/fm10k_main.c | 2 +-
drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 8 ++++++++
3 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k.h b/drivers/net/ethernet/intel/fm10k/fm10k.h
index 42eb434..d38f088 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k.h
+++ b/drivers/net/ethernet/intel/fm10k/fm10k.h
@@ -443,6 +443,7 @@ netdev_tx_t fm10k_xmit_frame_ring(struct sk_buff *skb,
struct fm10k_ring *tx_ring);
void fm10k_tx_timeout_reset(struct fm10k_intfc *interface);
bool fm10k_check_tx_hang(struct fm10k_ring *tx_ring);
+__be16 fm10k_tx_encap_offload(struct sk_buff *skb);
void fm10k_alloc_rx_buffers(struct fm10k_ring *rx_ring, u16 cleaned_count);
/* PCI */
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index 3a85291..1144e14 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -727,7 +727,7 @@ static struct ethhdr *fm10k_gre_is_nvgre(struct sk_buff *skb)
return (struct ethhdr *)(&nvgre_hdr->tni);
}
-static __be16 fm10k_tx_encap_offload(struct sk_buff *skb)
+__be16 fm10k_tx_encap_offload(struct sk_buff *skb)
{
struct ethhdr *eth_hdr;
u8 l4_hdr = 0;
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
index 8811364..6e8630a 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
@@ -1350,6 +1350,13 @@ static void fm10k_dfwd_del_station(struct net_device *dev, void *priv)
}
}
+static bool fm10k_gso_check(struct sk_buff *skb, struct net_device *dev)
+{
+ return (!(skb_shinfo(skb)->gso_type &
+ (SKB_GSO_UDP_TUNNEL | SKB_GSO_GRE)) ||
+ fm10k_tx_encap_offload(skb));
+}
+
static const struct net_device_ops fm10k_netdev_ops = {
.ndo_open = fm10k_open,
.ndo_stop = fm10k_close,
@@ -1372,6 +1379,7 @@ static const struct net_device_ops fm10k_netdev_ops = {
.ndo_do_ioctl = fm10k_ioctl,
.ndo_dfwd_add_station = fm10k_dfwd_add_station,
.ndo_dfwd_del_station = fm10k_dfwd_del_station,
+ .ndo_gso_check = fm10k_gso_check,
};
#define DEFAULT_DEBUG_LEVEL_SHIFT 3
--
1.7.10.4
^ permalink raw reply related
* [PATCHv2 net 1/2] fm10k: Check tunnel header length in encap offload
From: Joe Stringer @ 2014-11-13 23:36 UTC (permalink / raw)
To: netdev
Cc: matthew.vick, jeffrey.t.kirsher, linux.nics, therbert, gerlitz.or,
alexander.duyck, linux-kernel
fm10k supports up to 184 bytes of inner+outer headers. Add an initial
check to fail encap offload if these are too large.
Signed-off-by: Joe Stringer <joestringer@nicira.com>
---
Matthew, I didn't see the equivalent patch on netdev so I went ahead and
created it. If I've missed this somewhere, then please disregard.
v2: First post.
---
drivers/net/ethernet/intel/fm10k/fm10k_main.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index e645af4..3a85291 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -732,6 +732,12 @@ static __be16 fm10k_tx_encap_offload(struct sk_buff *skb)
struct ethhdr *eth_hdr;
u8 l4_hdr = 0;
+/* fm10k supports 184 octets of outer+inner headers. Minus 20 for inner L4. */
+#define FM10K_MAX_ENCAP_TRANSPORT_OFFSET 164
+ if (skb_inner_transport_header(skb) - skb_mac_header(skb) >
+ FM10K_MAX_ENCAP_TRANSPORT_OFFSET)
+ return 0;
+
switch (vlan_get_protocol(skb)) {
case htons(ETH_P_IP):
l4_hdr = ip_hdr(skb)->protocol;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH net-next 1/1] ipvlan: Initial check-in of the IPVLAN driver.
From: Alexei Starovoitov @ 2014-11-13 23:25 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: netdev, Eric Dumazet, Maciej Zenczykowski, Laurent Chavey,
Tim Hockin, David Miller, Brandon Philips, Pavel Emelianov
On Tue, Nov 11, 2014 at 2:29 PM, Mahesh Bandewar <maheshb@google.com> wrote:
> The device operates in two different modes and the difference
> in these two modes in primarily in the TX side.
>
> (a) L2 mode : In this mode, the device behaves as a L2 device.
> TX processing upto L2 happens on the stack of the virtual device
> associated with (namespace). Packets are switched after that
> into the main device (default-ns) and queued for xmit.
>
> RX processing is simple and all multicast, broadcast (if
> applicable), and unicast belonging to the address(es) are
> delivered to the virtual devices.
>
> (b) L3 mode : In this mode, the device behaves like a L3 device.
> TX processing upto L3 happens on the stack of the virtual device
> associated with (namespace). Packets are switched to the
> main-device (default-ns) for the L2 processing. Hence the routing
> table of the default-ns will be used in this mode.
>
> RX processins is somewhat similar to the L2 mode except that in
> this mode only Unicast packets are delivered to the virtual device
> while main-dev will handle all other packets.
great stuff. would be interesting to see a 'typical use'
scenario of l2 vs l3 mode. Why users would pick one
or another?
I can only think of different default ip in different ns
would force l2. Anything else?
Few comments:
> +++ b/drivers/net/ipvlan/ipvlan.h
...
> +#include <linux/kernel.h>
> +#include <linux/types.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/errno.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/rculist.h>
> +#include <linux/notifier.h>
> +#include <linux/netdevice.h>
> +#include <linux/etherdevice.h>
> +#include <linux/ethtool.h>
> +#include <linux/if_arp.h>
> +#include <linux/if_link.h>
> +#include <linux/atomic.h>
> +#include <linux/if_vlan.h>
> +#include <linux/inet.h>
> +#include <linux/hash.h>
> +#include <linux/ip.h>
> +#include <linux/inetdevice.h>
> +#include <net/rtnetlink.h>
> +#include <net/gre.h>
> +#include <net/route.h>
> +#include <net/addrconf.h>
I don't think it's a good style to put all headers that all
.c need into common .h
Rather put them into individual .c
> +static void *ipvlan_get_L3_hdr(struct sk_buff *skb, int *type)
> +{
> + void *lyr3h = NULL;
> +
> + switch (skb->protocol) {
> + case htons(ETH_P_ARP): {
> + struct arphdr *arph;
> +
> + if (unlikely(!pskb_may_pull(skb, sizeof(struct arphdr))))
> + return NULL;
> +
> + arph = arp_hdr(skb);
> + *type = IPVL_ARP;
> + lyr3h = arph;
> + break;
> + }
...
> +static struct ipvl_addr *ipvlan_addr_lookup(struct ipvl_port *port,
> + void *lyr3h, int addr_type,
> + bool use_dest)
> +{
> + struct ipvl_addr *addr = NULL;
> +
> + if (addr_type == IPVL_IPV6) {
> + struct ipv6hdr *ip6h = NULL;
> + struct in6_addr *i6addr;
> +
> + ip6h = (struct ipv6hdr *)lyr3h;
> + i6addr = use_dest ? &ip6h->daddr : &ip6h->saddr;
> + addr = ipvlan_ht_addr_lookup(port, i6addr, true);
imo it looks very artificial to split logically single
lookup function into two: get() that returns 'type'/
'void * lyr3h' and lookup() that uses them.
It feels error prone.
Also everywhere lookup() follows get() immediately.
I think single lookup() would be much cleaner.
^ permalink raw reply
* Re: [PATCH 2/3] r8169: Use load_acquire() and store_release() to reduce memory barrier overhead
From: Alexander Duyck @ 2014-11-13 23:11 UTC (permalink / raw)
To: Francois Romieu, Alexander Duyck
Cc: linux-arch, netdev, linux-kernel, mikey, tony.luck,
mathieu.desnoyers, donald.c.skidmore, peterz, benh,
heiko.carstens, oleg, will.deacon, davem, michael, matthew.vick,
nic_swsd, geert, jeffrey.t.kirsher, fweisbec, schwidefsky, linux,
paulmck, torvalds, mingo
In-Reply-To: <20141113213049.GA12297@electric-eye.fr.zoreil.com>
On 11/13/2014 01:30 PM, Francois Romieu wrote:
> Alexander Duyck <alexander.h.duyck@redhat.com> :
> [...]
>> In addition the r8169 uses a rmb() however I believe it is placed incorrectly
>> as I assume it supposed to be ordering descriptor reads after the check for
>> ownership.
> Not exactly. It's a barrier against compiler optimization from 2004.
> It should not matter.
Okay. Do you recall the kind of problem it was you were seeing?
The origin of the rmb() for the Intel drivers was a PowerPC issue in
which it was fetching the length of a buffer before it checked the DD
bit (equivalent of DescOwn). I'm wondering if the issue you were seeing
was something similar where it had reordered reads in the descriptor to
cause that type of result.
> However I disagree with the change below:
>
>> @@ -7284,11 +7280,11 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp, u32 budget
>> struct RxDesc *desc = tp->RxDescArray + entry;
>> u32 status;
>>
>> - rmb();
>> - status = le32_to_cpu(desc->opts1) & tp->opts1_mask;
>> -
>> + status = cpu_to_le32(load_acquire(&desc->opts1));
>> if (status & DescOwn)
>> break;
>> +
>> + status &= tp->opts1_mask;
> -> tp->opts1_mask is not __le32 tainted.
Sorry I just noticed I got my byte ordering messed up on that. It
should have been le32_to_cpu. desc->opts is le32, and status should be
CPU ordered. I will have that updated for v2.
> Btw, should I consider the sketch above as a skeleton in my r8169 closet ?
>
> NIC CPU0 CPU1
> | CPU | NIC | CPU | CPU |
>
> | CPU | NIC | CPU | CPU |
> ^ tx_dirty
>
> [start_xmit...
>
> | CPU | CPU | CPU | CPU |
> (NIC did it's job)
> [rtl_tx...
> | ... | ... | NIC | NIC |
> (ring update)
> (tx_dirty increases)
>
> | CPU | CPU | ??? | ??? |
> tx_dirty ?
> reaping about-to-be-sent
> buffers on some platforms ?
> ...start_xmit]
Actually it looks like that could be due to the placement of tp->cur_tx
update and the txd->opts1 being updated in the same spot in start_xmit
with no barrier to separate them. As such the compiler is free to
update tp->cur_tx first, and then update the desc->opts to set the
DescOwn bit.
I will move the update of tp->cur_tx down a few lines past where the
second wmb is/was. That should provide enough buffer to guarantee that
cur_tx update is only visible after the descriptors have been updated so
the reaping should only occur if the CPU has written back.
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH 16/16] rxrpc: Replace smp_read_barrier_depends() with lockless_dereference()
From: David Howells @ 2014-11-13 23:07 UTC (permalink / raw)
To: Pranith Kumar
Cc: dhowells, David S. Miller, Dan Carpenter,
open list:NETWORKING [GENERAL], open list, paulmck
In-Reply-To: <546528BF.5040902@gmail.com>
Pranith Kumar <bobby.prani@gmail.com> wrote:
> OK. Should I send in a patch removing these barriers then?
No. There need to be stronger barriers, at least in some of the cases.
circular-buffers.txt details what is required, but not all of the cases match
the pattern there, so it needs a bit more consideration.
David
^ permalink raw reply
* Re: [PATCH 3/3] sh_eth: Fix dma mapping issue
From: Sergei Shtylyov @ 2014-11-13 23:05 UTC (permalink / raw)
To: Yoshihiro Kaneko, netdev
Cc: David S. Miller, Simon Horman, Magnus Damm, linux-sh
In-Reply-To: <1415862301-28032-4-git-send-email-ykaneko0929@gmail.com>
On 11/13/2014 10:05 AM, Yoshihiro Kaneko wrote:
> From: Mitsuhiro Kimura <mitsuhiro.kimura.kc@renesas.com>
> When CONFIG_DMA_API_DEBUG=y, many DMA error messages reports.
> In order to use DMA debug, This patch fix following issues.
> Issue 1:
> If dma_mapping_error function is not called appropriately after
> DMA mapping, DMA debug will report error message when DMA unmap
> function is called.
> Issue 2:
> If skb_reserve function is called after DMA mapping, the relationship
> between mapping addr and mapping size will be broken.
> In this case, DMA debug will report error messages when DMA sync
> function and DMA unmap function are called.
> Issue 3:
> If the size of frame data is less than ETH_ZLEN, the size is resized
> to ETH_ZLEN after DMA map function is called.
> In the TX skb freeing function, dma unmap function is called with that
> resized value. So, unmap size error will reported.
> Issue 4:
> In the rx function, DMA map function is called without DMA unmap function
> is called for RX skb reallocating.
> It will case the DMA debug error that number of debug entry is full and
> DMA debug logic is stopped.
The rule of thumb is "fix one issue per patch". Please split accordingly.
> Signed-off-by: Mitsuhiro Kimura <mitsuhiro.kimura.kc@renesas.com>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
Thanks for beating me to it. Fixing these issues has been on my agenda for
a long time... :-)
> ---
> drivers/net/ethernet/renesas/sh_eth.c | 26 +++++++++++++++++++++++---
> 1 file changed, 23 insertions(+), 3 deletions(-)
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 0e4a407..23318cf 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -1136,6 +1136,11 @@ static void sh_eth_ring_format(struct net_device *ndev)
> dma_map_single(&ndev->dev, skb->data, rxdesc->buffer_length,
> DMA_FROM_DEVICE);
> rxdesc->addr = virt_to_phys(skb->data);
Can't we get rid of these bogus virt_to_phys() calls, while at it?
dma_map_single() returns a DMA address, no?
> + if (dma_mapping_error(&ndev->dev, rxdesc->addr)) {
> + dev_kfree_skb(mdp->rx_skbuff[i]);
> + mdp->rx_skbuff[i] = NULL;
> + break;
> + }
> rxdesc->status = cpu_to_edmac(mdp, RD_RACT | RD_RFP);
>
> /* Rx descriptor address set */
> @@ -1364,7 +1369,7 @@ static int sh_eth_txfree(struct net_device *ndev)
> if (mdp->tx_skbuff[entry]) {
> dma_unmap_single(&ndev->dev, txdesc->addr,
> txdesc->buffer_length, DMA_TO_DEVICE);
> - dev_kfree_skb_irq(mdp->tx_skbuff[entry]);
> + dev_kfree_skb_any(mdp->tx_skbuff[entry]);
Hm, I'm not sure where is this described in the changelog...
> mdp->tx_skbuff[entry] = NULL;
> free_num++;
> }
> @@ -1466,11 +1471,19 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
> if (skb == NULL)
> break; /* Better luck next round. */
> sh_eth_set_receive_align(skb);
> + dma_unmap_single(&ndev->dev, rxdesc->addr,
> + rxdesc->buffer_length,
> + DMA_FROM_DEVICE);
> dma_map_single(&ndev->dev, skb->data,
> rxdesc->buffer_length, DMA_FROM_DEVICE);
>
> skb_checksum_none_assert(skb);
> rxdesc->addr = virt_to_phys(skb->data);
Likewise, can we get rid of this bogu?
> + if (dma_mapping_error(&ndev->dev, rxdesc->addr)) {
> + dev_kfree_skb_any(mdp->rx_skbuff[entry]);
> + mdp->rx_skbuff[entry] = NULL;
> + break;
> + }
> }
> if (entry >= mdp->num_rx_ring - 1)
> rxdesc->status |=
> @@ -2104,12 +2117,18 @@ static int sh_eth_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> if (!mdp->cd->hw_swap)
> sh_eth_soft_swap(phys_to_virt(ALIGN(txdesc->addr, 4)),
> skb->len + 2);
> - txdesc->addr = dma_map_single(&ndev->dev, skb->data, skb->len,
> - DMA_TO_DEVICE);
> if (skb->len < ETH_ZLEN)
> txdesc->buffer_length = ETH_ZLEN;
> else
> txdesc->buffer_length = skb->len;
> + txdesc->addr = dma_map_single(&ndev->dev, skb->data,
> + txdesc->buffer_length,
> + DMA_TO_DEVICE);
> + if (dma_mapping_error(&ndev->dev, txdesc->addr)) {
> + dev_kfree_skb_any(mdp->tx_skbuff[entry]);
> + mdp->tx_skbuff[entry] = NULL;
> + goto out;
Why not just *return*?!
[...]
WBR, Sergei
^ permalink raw reply
* [PATCH net-next] icmp: Remove some spurious dropped packet profile hits from the ICMP path
From: Rick Jones @ 2014-11-13 22:54 UTC (permalink / raw)
To: netdev; +Cc: davem
From: Rick Jones <rick.jones2@hp.com>
If icmp_rcv() has successfully processed the incoming ICMP datagram, we
should use consume_skb() rather than kfree_skb() because a hit on the likes
of perf -e skb:kfree_skb is not called-for.
Signed-off-by: Rick Jones <rick.jones2@hp.com>
---
A test system hit with a flood ping hits on perf top -e ksb:kfre_skb before
the change and none after for the normal/success path. The IPv6 path would
be somewhat more ugly. For the time being, just deal with the overlap on
ping_rcv() between the two to avoid a possible double free of an skb.
diff --git a/include/net/ping.h b/include/net/ping.h
index 026479b..f074060 100644
--- a/include/net/ping.h
+++ b/include/net/ping.h
@@ -82,7 +82,7 @@ int ping_common_sendmsg(int family, struct msghdr *msg, size_t len,
int ping_v6_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
size_t len);
int ping_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);
-void ping_rcv(struct sk_buff *skb);
+bool ping_rcv(struct sk_buff *skb);
#ifdef CONFIG_PROC_FS
struct ping_seq_afinfo {
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 36b7bfa..b9f3653 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -190,7 +190,7 @@ EXPORT_SYMBOL(icmp_err_convert);
*/
struct icmp_control {
- void (*handler)(struct sk_buff *skb);
+ bool (*handler)(struct sk_buff *skb);
short error; /* This ICMP is classed as an error message */
};
@@ -746,7 +746,7 @@ static bool icmp_tag_validation(int proto)
* ICMP_PARAMETERPROB.
*/
-static void icmp_unreach(struct sk_buff *skb)
+static bool icmp_unreach(struct sk_buff *skb)
{
const struct iphdr *iph;
struct icmphdr *icmph;
@@ -839,10 +839,11 @@ static void icmp_unreach(struct sk_buff *skb)
icmp_socket_deliver(skb, info);
out:
- return;
+ return true;
out_err:
ICMP_INC_STATS_BH(net, ICMP_MIB_INERRORS);
- goto out;
+ kfree_skb(skb);
+ return false;
}
@@ -850,17 +851,22 @@ out_err:
* Handle ICMP_REDIRECT.
*/
-static void icmp_redirect(struct sk_buff *skb)
+static bool icmp_redirect(struct sk_buff *skb)
{
if (skb->len < sizeof(struct iphdr)) {
ICMP_INC_STATS_BH(dev_net(skb->dev), ICMP_MIB_INERRORS);
- return;
+ kfree_skb(skb);
+ return false;
}
- if (!pskb_may_pull(skb, sizeof(struct iphdr)))
- return;
+ if (!pskb_may_pull(skb, sizeof(struct iphdr))) {
+ /* there aught to be a stat */
+ kfree_skb(skb);
+ return false;
+ }
icmp_socket_deliver(skb, icmp_hdr(skb)->un.gateway);
+ return true;
}
/*
@@ -875,7 +881,7 @@ static void icmp_redirect(struct sk_buff *skb)
* See also WRT handling of options once they are done and working.
*/
-static void icmp_echo(struct sk_buff *skb)
+static bool icmp_echo(struct sk_buff *skb)
{
struct net *net;
@@ -891,6 +897,8 @@ static void icmp_echo(struct sk_buff *skb)
icmp_param.head_len = sizeof(struct icmphdr);
icmp_reply(&icmp_param, skb);
}
+ /* should there be an ICMP stat for ignored echos? */
+ return true;
}
/*
@@ -900,7 +908,7 @@ static void icmp_echo(struct sk_buff *skb)
* MUST be accurate to a few minutes.
* MUST be updated at least at 15Hz.
*/
-static void icmp_timestamp(struct sk_buff *skb)
+static bool icmp_timestamp(struct sk_buff *skb)
{
struct timespec tv;
struct icmp_bxm icmp_param;
@@ -927,15 +935,18 @@ static void icmp_timestamp(struct sk_buff *skb)
icmp_param.data_len = 0;
icmp_param.head_len = sizeof(struct icmphdr) + 12;
icmp_reply(&icmp_param, skb);
-out:
- return;
+ return true;
+
out_err:
ICMP_INC_STATS_BH(dev_net(skb_dst(skb)->dev), ICMP_MIB_INERRORS);
- goto out;
+ kfree_skb(skb);
+ return false;
}
-static void icmp_discard(struct sk_buff *skb)
+static bool icmp_discard(struct sk_buff *skb)
{
+ /* pretend it was a success */
+ return true;
}
/*
@@ -946,6 +957,7 @@ int icmp_rcv(struct sk_buff *skb)
struct icmphdr *icmph;
struct rtable *rt = skb_rtable(skb);
struct net *net = dev_net(rt->dst.dev);
+ bool success;
if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
struct sec_path *sp = skb_sec_path(skb);
@@ -1012,7 +1024,12 @@ int icmp_rcv(struct sk_buff *skb)
}
}
- icmp_pointers[icmph->type].handler(skb);
+ success = icmp_pointers[icmph->type].handler(skb);
+
+ if (success)
+ consume_skb(skb);
+
+ return 0;
drop:
kfree_skb(skb);
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index 736236c..7d54eed 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -955,7 +955,7 @@ EXPORT_SYMBOL_GPL(ping_queue_rcv_skb);
* All we need to do is get the socket.
*/
-void ping_rcv(struct sk_buff *skb)
+bool ping_rcv(struct sk_buff *skb)
{
struct sock *sk;
struct net *net = dev_net(skb->dev);
@@ -974,11 +974,13 @@ void ping_rcv(struct sk_buff *skb)
pr_debug("rcv on socket %p\n", sk);
ping_queue_rcv_skb(sk, skb_get(skb));
sock_put(sk);
- return;
+ return true;
}
pr_debug("no socket, dropping\n");
- /* We're called from icmp_rcv(). kfree_skb() is done there. */
+ /* Do the kfree_skb() here to get a better drop profile */
+ kfree_skb(skb);
+ return false;
}
EXPORT_SYMBOL_GPL(ping_rcv);
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 0929340..6c0f805 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -679,6 +679,7 @@ static int icmpv6_rcv(struct sk_buff *skb)
const struct in6_addr *saddr, *daddr;
struct icmp6hdr *hdr;
u8 type;
+ bool success = true;
if (!xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb)) {
struct sec_path *sp = skb_sec_path(skb);
@@ -726,7 +727,7 @@ static int icmpv6_rcv(struct sk_buff *skb)
break;
case ICMPV6_ECHO_REPLY:
- ping_rcv(skb);
+ success = ping_rcv(skb);
break;
case ICMPV6_PKT_TOOBIG:
@@ -790,7 +791,15 @@ static int icmpv6_rcv(struct sk_buff *skb)
icmpv6_notify(skb, type, hdr->icmp6_code, hdr->icmp6_mtu);
}
- kfree_skb(skb);
+ /* until the v6 path can be better sorted we may still need
+ * to kfree_sbk() here but want to avoid a double free from
+ * the ping_rcv() path, which shares code with IPv4. assume
+ * success and preserve the status quo behaviour for the rest
+ * of the paths to here
+ */
+ if (success)
+ kfree_skb(skb);
+
return 0;
csum_error:
^ permalink raw reply related
* Re: [PATCH 2/3] sh_eth: Fix skb alloc size and alignment adjust rule.
From: Sergei Shtylyov @ 2014-11-13 22:48 UTC (permalink / raw)
To: Yoshihiro Kaneko, netdev
Cc: David S. Miller, Simon Horman, Magnus Damm, linux-sh
In-Reply-To: <1415862301-28032-3-git-send-email-ykaneko0929@gmail.com>
On 11/13/2014 10:05 AM, Yoshihiro Kaneko wrote:
> From: Mitsuhiro Kimura <mitsuhiro.kimura.kc@renesas.com>
> In the current driver, allocation size of skb does not care the alignment
> adjust after allocation.
> And also, in the current implementation, buffer alignment method by
> sh_eth_set_receive_align function has a bug that this function displace
> buffer start address forcedly when the alignment is corrected.
> In the result, tail of the skb will exceed allocated area and kernel panic
> will be occurred.
Oh, have never seen panic but Geert has reported WARNINGs from the DMA
debug code...
> This patch fix this issue.
> Signed-off-by: Mitsuhiro Kimura <mitsuhiro.kimura.kc@renesas.com>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> ---
> drivers/net/ethernet/renesas/sh_eth.c | 35 ++++++++++++++---------------------
> drivers/net/ethernet/renesas/sh_eth.h | 2 ++
> 2 files changed, 16 insertions(+), 21 deletions(-)
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 49e963e..0e4a407 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -917,21 +917,12 @@ static int sh_eth_reset(struct net_device *ndev)
> return ret;
> }
>
> -#if defined(CONFIG_CPU_SH4) || defined(CONFIG_ARCH_SHMOBILE)
> static void sh_eth_set_receive_align(struct sk_buff *skb)
> {
> - int reserve;
> -
> - reserve = SH4_SKB_RX_ALIGN - ((u32)skb->data & (SH4_SKB_RX_ALIGN - 1));
> + u32 reserve = (u32)skb->data & (SH_ETH_RX_ALIGN - 1);
Please keep an empty line after declaration, as it was before this patch.
> if (reserve)
> - skb_reserve(skb, reserve);
> -}
> -#else
> -static void sh_eth_set_receive_align(struct sk_buff *skb)
> -{
> - skb_reserve(skb, SH2_SH3_SKB_RX_ALIGN);
> + skb_reserve(skb, SH_ETH_RX_ALIGN - reserve);
> }
> -#endif
>
>
> /* CPU <-> EDMAC endian convert */
> @@ -1119,6 +1110,7 @@ static void sh_eth_ring_format(struct net_device *ndev)
> struct sh_eth_txdesc *txdesc = NULL;
> int rx_ringsize = sizeof(*rxdesc) * mdp->num_rx_ring;
> int tx_ringsize = sizeof(*txdesc) * mdp->num_tx_ring;
> + int skbuff_size = mdp->rx_buf_sz + SH_ETH_RX_ALIGN - 1;
>
> mdp->cur_rx = 0;
> mdp->cur_tx = 0;
> @@ -1131,21 +1123,21 @@ static void sh_eth_ring_format(struct net_device *ndev)
> for (i = 0; i < mdp->num_rx_ring; i++) {
> /* skb */
> mdp->rx_skbuff[i] = NULL;
> - skb = netdev_alloc_skb(ndev, mdp->rx_buf_sz);
> + skb = netdev_alloc_skb(ndev, skbuff_size);
> mdp->rx_skbuff[i] = skb;
> if (skb == NULL)
> break;
> - dma_map_single(&ndev->dev, skb->data, mdp->rx_buf_sz,
> - DMA_FROM_DEVICE);
> sh_eth_set_receive_align(skb);
>
> /* RX descriptor */
> rxdesc = &mdp->rx_ring[i];
> + /* The size of the buffer is 16 byte boundary. */
Is *on* 16 byte boundary, you mean?
> + rxdesc->buffer_length = ALIGN(mdp->rx_buf_sz, 16);
> + dma_map_single(&ndev->dev, skb->data, rxdesc->buffer_length,
> + DMA_FROM_DEVICE);
> rxdesc->addr = virt_to_phys(skb->data);
> rxdesc->status = cpu_to_edmac(mdp, RD_RACT | RD_RFP);
>
> - /* The size of the buffer is 16 byte boundary. */
Ah, you're just copying an existent comment... well, seems a good time to
fix it then. :-)
[...]
> @@ -1448,8 +1441,8 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
> if (mdp->cd->rpadir)
> skb_reserve(skb, NET_IP_ALIGN);
> dma_sync_single_for_cpu(&ndev->dev, rxdesc->addr,
> - mdp->rx_buf_sz,
> - DMA_FROM_DEVICE);
> + ALIGN(mdp->rx_buf_sz, 16),
> + DMA_FROM_DEVICE);
Please keep the original alignment of the continuation lines.
> skb_put(skb, pkt_len);
> skb->protocol = eth_type_trans(skb, ndev);
> netif_receive_skb(skb);
> @@ -1468,13 +1461,13 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
> rxdesc->buffer_length = ALIGN(mdp->rx_buf_sz, 16);
>
> if (mdp->rx_skbuff[entry] == NULL) {
> - skb = netdev_alloc_skb(ndev, mdp->rx_buf_sz);
> + skb = netdev_alloc_skb(ndev, skbuff_size);
> mdp->rx_skbuff[entry] = skb;
> if (skb == NULL)
> break; /* Better luck next round. */
> - dma_map_single(&ndev->dev, skb->data, mdp->rx_buf_sz,
> - DMA_FROM_DEVICE);
> sh_eth_set_receive_align(skb);
> + dma_map_single(&ndev->dev, skb->data,
> + rxdesc->buffer_length, DMA_FROM_DEVICE);
>
> skb_checksum_none_assert(skb);
> rxdesc->addr = virt_to_phys(skb->data);
> diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
> index b37c427..d138ebe 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.h
> +++ b/drivers/net/ethernet/renesas/sh_eth.h
> @@ -163,8 +163,10 @@ enum {
> /* Driver's parameters */
> #if defined(CONFIG_CPU_SH4) || defined(CONFIG_ARCH_SHMOBILE)
> #define SH4_SKB_RX_ALIGN 32
> +#define SH_ETH_RX_ALIGN (SH4_SKB_RX_ALIGN)
() not needed.
> #else
> #define SH2_SH3_SKB_RX_ALIGN 2
> +#define SH_ETH_RX_ALIGN (SH2_SH3_SKB_RX_ALIGN)
Likewise.
And I don't think we still need {SH2_SH3|SH4}_SKB_RX_ALIGN after this patch.
[...]
WBR, Sergei
^ permalink raw reply
* Re: [PATCH 1/3] sh_eth: Remove redundant alignment adjustment
From: Sergei Shtylyov @ 2014-11-13 22:37 UTC (permalink / raw)
To: Yoshihiro Kaneko, netdev
Cc: David S. Miller, Simon Horman, Magnus Damm, linux-sh
In-Reply-To: <1415862301-28032-2-git-send-email-ykaneko0929@gmail.com>
On 11/13/2014 10:04 AM, Yoshihiro Kaneko wrote:
> From: Mitsuhiro Kimura <mitsuhiro.kimura.kc@renesas.com>
> PTR_ALIGN macro after skb_reserve is redundant, because skb_reserve
> function adjusts the alignment of skb->data.
OK, but where is the bug? There must be one if you base this patch on the
'net' tree...
> Signed-off-by: Mitsuhiro Kimura <mitsuhiro.kimura.kc@renesas.com>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
WBR, Sergei
^ permalink raw reply
* Re: [PATCH 1/2] sh_eth: Fix sleeping function called from invalid context
From: Sergei Shtylyov @ 2014-11-13 22:33 UTC (permalink / raw)
To: Yoshihiro Kaneko, netdev
Cc: David S. Miller, Simon Horman, Magnus Damm, linux-sh
In-Reply-To: <1415862135-27972-2-git-send-email-ykaneko0929@gmail.com>
On 11/13/2014 10:02 AM, Yoshihiro Kaneko wrote:
> From: Mitsuhiro Kimura <mitsuhiro.kimura.kc@renesas.com>
> Fix the bug as follows:
> ----
> [ 1238.161349] BUG: sleeping function called from invalid context at drivers/base/power/runtime.c:952
> [ 1238.188279] in_atomic(): 1, irqs_disabled(): 0, pid: 1388, name: cat
> [ 1238.207425] CPU: 0 PID: 1388 Comm: cat Not tainted 3.10.31-ltsi-00046-gefa0b46 #1087
> [ 1238.230737] Backtrace:
> [ 1238.238123] [<c0012e64>] (dump_backtrace+0x0/0x10c) from [<c0013000>] (show_stack+0x18/0x1c)
> [ 1238.263499] r6:000003b8 r5:c06160c0 r4:c0669e00 r3:00404000
> [ 1238.280583] [<c0012fe8>] (show_stack+0x0/0x1c) from [<c04515a4>] (dump_stack+0x20/0x28)
> [ 1238.304631] [<c0451584>] (dump_stack+0x0/0x28) from [<c004970c>] (__might_sleep+0xf8/0x118)
> [ 1238.329734] [<c0049614>] (__might_sleep+0x0/0x118) from [<c02465ac>] (__pm_runtime_resume+0x38/0x90)
> [ 1238.357170] r7:d616f000 r6:c049c458 r5:00000004 r4:d6a17210
> [ 1238.374251] [<c0246574>] (__pm_runtime_resume+0x0/0x90) from [<c029b1c4>] (sh_eth_get_stats+0x44/0x280)
> [ 1238.402468] r7:d616f000 r6:c049c458 r5:d5c21000 r4:d5c21000
> [ 1238.419552] [<c029b180>] (sh_eth_get_stats+0x0/0x280) from [<c03ae39c>] (dev_get_stats+0x54/0x88)
> [ 1238.446204] r5:d5c21000 r4:d5ed7e08
> [ 1238.456980] [<c03ae348>] (dev_get_stats+0x0/0x88) from [<c03c677c>] (netstat_show.isra.15+0x54/0x9c)
> [ 1238.484413] r6:d5c21000 r5:d5c21238 r4:00000028 r3:00000001
> [ 1238.501495] [<c03c6728>] (netstat_show.isra.15+0x0/0x9c) from [<c03c69b8>] (show_tx_errors+0x18/0x1c)
> [ 1238.529196] r7:d5f945d8 r6:d5f945c0 r5:c049716c r4:c0650e7c
> [ 1238.546279] [<c03c69a0>] (show_tx_errors+0x0/0x1c) from [<c023963c>] (dev_attr_show+0x24/0x50)
> [ 1238.572157] [<c0239618>] (dev_attr_show+0x0/0x50) from [<c010c148>] (sysfs_read_file+0xb0/0x140)
> [ 1238.598554] r5:c049716c r4:d5c21240
> [ 1238.609326] [<c010c098>] (sysfs_read_file+0x0/0x140) from [<c00b9ee4>] (vfs_read+0xb0/0x13c)
> [ 1238.634679] [<c00b9e34>] (vfs_read+0x0/0x13c) from [<c00ba0ac>] (SyS_read+0x44/0x74)
> [ 1238.657944] r8:bef45bf0 r7:00000000 r6:d6ac0600 r5:00000000 r4:00000000
> [ 1238.678172] [<c00ba068>] (SyS_read+0x0/0x74) from [<c000eec0>] (ret_fast_syscall+0x0/0x30)
> ----
How to reproduce this?
> Signed-off-by: Mitsuhiro Kimura <mitsuhiro.kimura.kc@renesas.com>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
[...]
> diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
> index b37c427..9a1c550 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.h
> +++ b/drivers/net/ethernet/renesas/sh_eth.h
> @@ -508,6 +508,7 @@ struct sh_eth_private {
> u32 rx_buf_sz; /* Based on MTU+slack. */
> int edmac_endian;
> struct napi_struct napi;
> + bool is_opened;
Placing it after 'vlan_num_ids' (and making it a bitfield?) would probably
allow to save some space.
WBR, Sergei
^ permalink raw reply
* Re: [PATCH] net: skb_fclone_busy() needs to detect orphaned skb
From: Luis Henriques @ 2014-11-13 22:32 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Neal Cardwell, Joseph Salisbury
In-Reply-To: <1415913622.17262.24.camel@edumazet-glaptop2.roam.corp.google.com>
On Thu, Nov 13, 2014 at 01:20:22PM -0800, Eric Dumazet wrote:
> On Thu, 2014-11-13 at 19:15 +0000, Luis Henriques wrote:
> > Hi Eric,
> >
> > On Thu, Oct 30, 2014 at 10:32:34AM -0700, Eric Dumazet wrote:
> > > From: Eric Dumazet <edumazet@google.com>
> > >
> > > Some drivers are unable to perform TX completions in a bound time.
> > > They instead call skb_orphan()
> > >
> > > Problem is skb_fclone_busy() has to detect this case, otherwise
> > > we block TCP retransmits and can freeze unlucky tcp sessions on
> > > mostly idle hosts.
> > >
> > > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > > Fixes: 1f3279ae0c13 ("tcp: avoid retransmits of TCP packets hanging in host queues")
> > > ---
> > > This is a stable candidate.
> > > This problem is known to hurt users of linux-3.16 kernels used by guests kernels.
> > > David, I can provide backports if you want.
> > > Thanks !
> > >
> >
> > We got a bug report[0] where a backport for 3.16 was provided. Since
> > I couldn't find the original backport post, I'm not sure who's the
> > actual author. Could you please confirm if this backport is correct?
> > (I'm copying the patch below).
> >
> > [0] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1390604
> >
> > Cheers,
> > --
> > Luís
> >
> >
>
> Sure ! I provided this patch indeed, I am 'The Google engineer'
> mentioned in this bug report ;)
>
Awesome, Thanks! I'll queue it for the 3.16 kernel. Since I couldn't
find the original patch, I could only guess who 'The Google engineer'
was :-)
Cheers,
--
Luís
> Signed-off-by: Eric Dumazet <edumazet@google.com>
>
>
> Thanks !
>
> > diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> > index 4e4932b5079b..a8794367cd20 100644
> > --- a/net/ipv4/tcp_output.c
> > +++ b/net/ipv4/tcp_output.c
> > @@ -2082,7 +2082,8 @@ static bool skb_still_in_host_queue(const struct sock *sk,
> > const struct sk_buff *fclone = skb + 1;
> >
> > if (unlikely(skb->fclone == SKB_FCLONE_ORIG &&
> > - fclone->fclone == SKB_FCLONE_CLONE)) {
> > + fclone->fclone == SKB_FCLONE_CLONE &&
> > + fclone->sk == sk)) {
> > NET_INC_STATS_BH(sock_net(sk),
> > LINUX_MIB_TCPSPURIOUS_RTX_HOSTQUEUES);
> > return true;
>
>
>
>
^ permalink raw reply
* Re: [PATCH] sh_eth: Optimization for RX excess judgement
From: Sergei Shtylyov @ 2014-11-13 22:27 UTC (permalink / raw)
To: Yoshihiro Kaneko, netdev
Cc: David S. Miller, Simon Horman, Magnus Damm, linux-sh
In-Reply-To: <1415862031-27925-1-git-send-email-ykaneko0929@gmail.com>
On 11/13/2014 10:00 AM, Yoshihiro Kaneko wrote:
> From: Mitsuhiro Kimura <mitsuhiro.kimura.kc@renesas.com>
> Both of 'boguscnt' and 'quota' have nearly meaning as the condition of
> the reception loop.
> In order to cut down redundant processing, this patch changes excess judgement.
> Signed-off-by: Mitsuhiro Kimura <mitsuhiro.kimura.kc@renesas.com>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> ---
> This patch is based on net tree.
> drivers/net/ethernet/renesas/sh_eth.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 60e9c2c..7d46326 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -1394,10 +1394,15 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
>
> int entry = mdp->cur_rx % mdp->num_rx_ring;
> int boguscnt = (mdp->dirty_rx + mdp->num_rx_ring) - mdp->cur_rx;
> + int limit = boguscnt;
> struct sk_buff *skb;
> u16 pkt_len = 0;
> u32 desc_status;
>
> + if (quota) {
> + boguscnt = min(boguscnt, *quota);
> + limit = boguscnt;
> + }
> rxdesc = &mdp->rx_ring[entry];
> while (!(rxdesc->status & cpu_to_edmac(mdp, RD_RACT))) {
> desc_status = edmac_to_cpu(mdp, rxdesc->status);
[...]
> @@ -1501,7 +1501,10 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
> sh_eth_write(ndev, EDRRR_R, EDRRR);
> }
>
> - return *quota <= 0;
> + if (quota)
> + *quota -= limit - (++boguscnt);
Just 'limit - boguscnt + 1'.
> +
> + return (boguscnt <= 0);
Hm... why change the *return* statement at all? I'm not sure this is at
all correct.
WBR, Sergei
^ permalink raw reply
* Re: [PATCH] net: sh_eth: Add RMII mode setting in probe
From: Sergei Shtylyov @ 2014-11-13 22:20 UTC (permalink / raw)
To: Yoshihiro Kaneko, netdev
Cc: David S. Miller, Simon Horman, Magnus Damm, linux-sh
In-Reply-To: <1415861645-27685-1-git-send-email-ykaneko0929@gmail.com>
Hello.
On 11/13/2014 09:54 AM, Yoshihiro Kaneko wrote:
> From: Hisashi Nakamura <hisashi.nakamura.ak@renesas.com>
> When using RMMI mode, it is necessary to change in probe.
I'd like this need to be explained in more detail.
> Signed-off-by: Hisashi Nakamura <hisashi.nakamura.ak@renesas.com>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> ---
> This patch is based on net-next tree.
> drivers/net/ethernet/renesas/sh_eth.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index dbe8606..1f79ed6 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -1,5 +1,6 @@
> /* SuperH Ethernet device driver
> *
> + * Copyright (C) 2014 Renesas Electronics Corporation
> * Copyright (C) 2006-2012 Nobuhiro Iwamatsu
> * Copyright (C) 2008-2014 Renesas Solutions Corp.
> * Copyright (C) 2013-2014 Cogent Embedded, Inc.
> @@ -2883,6 +2884,9 @@ static int sh_eth_drv_probe(struct platform_device *pdev)
> }
> }
>
> + if (mdp->cd->rmiimode)
> + sh_eth_write(ndev, 0x1, RMIIMODE);
> +
Does not such code need to be removed from sh_eth_dev_init() then?
WBR, Sergei
^ permalink raw reply
* Re: [PATCH] sh_eth: Optimization for RX excess judgement
From: Sergei Shtylyov @ 2014-11-13 22:09 UTC (permalink / raw)
To: Yoshihiro Kaneko, netdev
Cc: David S. Miller, Simon Horman, Magnus Damm, linux-sh
In-Reply-To: <1415862031-27925-1-git-send-email-ykaneko0929@gmail.com>
Hello.
On 11/13/2014 10:00 AM, Yoshihiro Kaneko wrote:
> From: Mitsuhiro Kimura <mitsuhiro.kimura.kc@renesas.com>
> Both of 'boguscnt' and 'quota' have nearly meaning as the condition of
> the reception loop.
> In order to cut down redundant processing, this patch changes excess judgement.
> Signed-off-by: Mitsuhiro Kimura <mitsuhiro.kimura.kc@renesas.com>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> ---
> This patch is based on net tree.
This is clearly 'net-next' material.
> drivers/net/ethernet/renesas/sh_eth.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> index 60e9c2c..7d46326 100644
> --- a/drivers/net/ethernet/renesas/sh_eth.c
> +++ b/drivers/net/ethernet/renesas/sh_eth.c
> @@ -1394,10 +1394,15 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
>
> int entry = mdp->cur_rx % mdp->num_rx_ring;
> int boguscnt = (mdp->dirty_rx + mdp->num_rx_ring) - mdp->cur_rx;
> + int limit = boguscnt;
> struct sk_buff *skb;
> u16 pkt_len = 0;
> u32 desc_status;
>
> + if (quota) {
I don't see what's the point in checking -- quota is always non-NULL.
> + boguscnt = min(boguscnt, *quota);
> + limit = boguscnt;
> + }
> rxdesc = &mdp->rx_ring[entry];
> while (!(rxdesc->status & cpu_to_edmac(mdp, RD_RACT))) {
> desc_status = edmac_to_cpu(mdp, rxdesc->status);
[...]
> @@ -1501,7 +1501,10 @@ static int sh_eth_rx(struct net_device *ndev, u32 intr_status, int *quota)
> sh_eth_write(ndev, EDRRR_R, EDRRR);
> }
>
> - return *quota <= 0;
> + if (quota)
Again, seeing no sense in this check.
> + *quota -= limit - (++boguscnt);
> +
> + return (boguscnt <= 0);
Parens not needed.
[...]
WBR, Sergei
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox