[PATCHv2 net 1/2] fm10k: Check tunnel header length in encap offload

[PATCHv2 net 1/2] fm10k: Check tunnel header length in encap offload

[Joe Stringer]

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

[PATCHv2 net 2/2] fm10k: Implement ndo_gso_check()

[Joe Stringer]

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

Re: [PATCHv2 net 1/2] fm10k: Check tunnel header length in encap offload

[Jeff Kirsher]

[-- 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 --]

Re: [PATCHv2 net 2/2] fm10k: Implement ndo_gso_check()

[Jeff Kirsher]

[-- 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 --]

Re: [PATCHv2 net 2/2] fm10k: Implement ndo_gso_check()

[Joe Stringer]

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?

Re: [PATCHv2 net 2/2] fm10k: Implement ndo_gso_check()

[Jeff Kirsher]

[-- 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 --]

Re: [PATCHv2 net 2/2] fm10k: Implement ndo_gso_check()

[Joe Stringer]

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

Re: [PATCHv2 net 1/2] fm10k: Check tunnel header length in encap offload

[Vick, Matthew]

On 11/13/14, 3:36 PM, "Joe Stringer" <joestringer@nicira.com> 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.

You didn't miss it Joe--it just hasn't made it up yet. :) It's currently
in Jeff's tree for testing. You're on the CC for the patch, so you'll get
a notification once it goes up. It's basically the same as what you have,
except the #define I use is 184 and I use inner_tcp_hdrlen() to account
for the inner TCP header length.

Since your second patch should apply cleanly on top of mine, what do you
think about dropping the first patch in this series and Jeff can send our
two patches up together once they've passed testing?

Cheers,
Matthew

Re: [PATCHv2 net 1/2] fm10k: Check tunnel header length in encap offload

[Joe Stringer]

On Thursday, November 13, 2014 16:41:03 Vick, Matthew wrote:
> On 11/13/14, 3:36 PM, "Joe Stringer" <joestringer@nicira.com> 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.
> 
> You didn't miss it Joe--it just hasn't made it up yet. :) It's currently
> in Jeff's tree for testing. You're on the CC for the patch, so you'll get
> a notification once it goes up. It's basically the same as what you have,
> except the #define I use is 184 and I use inner_tcp_hdrlen() to account
> for the inner TCP header length.
> 
> Since your second patch should apply cleanly on top of mine, what do you
> think about dropping the first patch in this series and Jeff can send our
> two patches up together once they've passed testing?

Ah, that sounds great.

Cheers,
Joe