[net-next] vxlan: Advertise SCTP checksum offloads

[net-next] vxlan: Advertise SCTP checksum offloads

[Jeff Kirsher]

From: Greg Rose <gregory.v.rose@intel.com>

Some HW can offload encapsulated SCTP checksums.  Advertise the capability
for such cases.

Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
Acked-by: Joseph Gasparakis <joseph.gasparakis@intel.com>
Acked-by: Shannon Nelson <shannon.nelson@intel.com>
Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/vxlan.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 1dfee9a..988208f 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2293,6 +2293,7 @@ static void vxlan_setup(struct net_device *dev)
 	dev->features	|= NETIF_F_SG | NETIF_F_HW_CSUM;
 	dev->features   |= NETIF_F_RXCSUM;
 	dev->features   |= NETIF_F_GSO_SOFTWARE;
+	dev->features   |= NETIF_F_SCTP_CSUM;
 
 	dev->vlan_features = dev->features;
 	dev->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;
-- 
1.9.0

Re: [net-next] vxlan: Advertise SCTP checksum offloads

[Ben Hutchings]

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

On Mon, 2014-04-28 at 12:52 -0700, Jeff Kirsher wrote:
> From: Greg Rose <gregory.v.rose@intel.com>
> 
> Some HW can offload encapsulated SCTP checksums.  Advertise the capability
> for such cases.

Don't we need a software fallback on the output path before doing this?

Ben.

> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> Acked-by: Joseph Gasparakis <joseph.gasparakis@intel.com>
> Acked-by: Shannon Nelson <shannon.nelson@intel.com>
> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  drivers/net/vxlan.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 1dfee9a..988208f 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -2293,6 +2293,7 @@ static void vxlan_setup(struct net_device *dev)
>  	dev->features	|= NETIF_F_SG | NETIF_F_HW_CSUM;
>  	dev->features   |= NETIF_F_RXCSUM;
>  	dev->features   |= NETIF_F_GSO_SOFTWARE;
> +	dev->features   |= NETIF_F_SCTP_CSUM;
>  
>  	dev->vlan_features = dev->features;
>  	dev->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;

-- 
Ben Hutchings
Q.  Which is the greater problem in the world today, ignorance or apathy?
A.  I don't know and I couldn't care less.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

RE: [net-next] vxlan: Advertise SCTP checksum offloads

[Rose, Gregory V]

> -----Original Message-----
> From: Ben Hutchings [mailto:ben@decadent.org.uk]
> Sent: Tuesday, April 29, 2014 5:39 AM
> To: Kirsher, Jeffrey T
> Cc: davem@davemloft.net; Rose, Gregory V; netdev@vger.kernel.org;
> gospo@redhat.com; sassmann@redhat.com
> Subject: Re: [net-next] vxlan: Advertise SCTP checksum offloads
> 
> On Mon, 2014-04-28 at 12:52 -0700, Jeff Kirsher wrote:
> > From: Greg Rose <gregory.v.rose@intel.com>
> >
> > Some HW can offload encapsulated SCTP checksums.  Advertise the
> > capability for such cases.
> 
> Don't we need a software fallback on the output path before doing this?

I believe that the vxlan driver will compare offload features with the lower device hw_enc_feature flags and if the HW doesn't have the offload then it won't be advertised to the sctp driver.  In that case the sctp driver will do its own checksumming.  At least that is the behavior I saw.

I'm adding Joseph Gasparakis.  He might have a better answer.

- Greg

> 
> Ben.
> 
> > Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> > Acked-by: Joseph Gasparakis <joseph.gasparakis@intel.com>
> > Acked-by: Shannon Nelson <shannon.nelson@intel.com>
> > Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > ---
> >  drivers/net/vxlan.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index
> > 1dfee9a..988208f 100644
> > --- a/drivers/net/vxlan.c
> > +++ b/drivers/net/vxlan.c
> > @@ -2293,6 +2293,7 @@ static void vxlan_setup(struct net_device *dev)
> >  	dev->features	|= NETIF_F_SG | NETIF_F_HW_CSUM;
> >  	dev->features   |= NETIF_F_RXCSUM;
> >  	dev->features   |= NETIF_F_GSO_SOFTWARE;
> > +	dev->features   |= NETIF_F_SCTP_CSUM;
> >
> >  	dev->vlan_features = dev->features;
> >  	dev->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;
> 
> --
> Ben Hutchings
> Q.  Which is the greater problem in the world today, ignorance or apathy?
> A.  I don't know and I couldn't care less.

Re: [net-next] vxlan: Advertise SCTP checksum offloads

[Vlad Yasevich]

On 04/29/2014 12:10 PM, Rose, Gregory V wrote:
> 
>> -----Original Message-----
>> From: Ben Hutchings [mailto:ben@decadent.org.uk]
>> Sent: Tuesday, April 29, 2014 5:39 AM
>> To: Kirsher, Jeffrey T
>> Cc: davem@davemloft.net; Rose, Gregory V; netdev@vger.kernel.org;
>> gospo@redhat.com; sassmann@redhat.com
>> Subject: Re: [net-next] vxlan: Advertise SCTP checksum offloads
>>
>> On Mon, 2014-04-28 at 12:52 -0700, Jeff Kirsher wrote:
>>> From: Greg Rose <gregory.v.rose@intel.com>
>>>
>>> Some HW can offload encapsulated SCTP checksums.  Advertise the
>>> capability for such cases.
>>
>> Don't we need a software fallback on the output path before doing this?
> 
> I believe that the vxlan driver will compare offload features with the lower device hw_enc_feature flags and if the HW doesn't have the offload then it won't be advertised to the sctp driver.  In that case the sctp driver will do its own checksumming.  At least that is the behavior I saw.
> 

I've been looking at this and can't see vxlan doing any feature
comparisons/disabling...

At the time SCTP tries to do checksum, it will see hw offload
support and set checksum as partial.  That will be propagated
all the way to vxlan which will set encapsulation and pass it on.
At next dev_hard_start_xmit(), we'll look at hw_enc_features,
but it won't matter since SCTP packets are never GSO.

So, we'll use the non-gso code path and still send with partial
checksum set.  Now, if the driver supports encapsulated SCTP,
then it will work.  If it doesn't, the udp checksum will be computed,
but not the sctp one.

-vlad

> I'm adding Joseph Gasparakis.  He might have a better answer.
> 
> - Greg
> 
>>
>> Ben.
>>
>>> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
>>> Acked-by: Joseph Gasparakis <joseph.gasparakis@intel.com>
>>> Acked-by: Shannon Nelson <shannon.nelson@intel.com>
>>> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
>>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>>> ---
>>>  drivers/net/vxlan.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index
>>> 1dfee9a..988208f 100644
>>> --- a/drivers/net/vxlan.c
>>> +++ b/drivers/net/vxlan.c
>>> @@ -2293,6 +2293,7 @@ static void vxlan_setup(struct net_device *dev)
>>>  	dev->features	|= NETIF_F_SG | NETIF_F_HW_CSUM;
>>>  	dev->features   |= NETIF_F_RXCSUM;
>>>  	dev->features   |= NETIF_F_GSO_SOFTWARE;
>>> +	dev->features   |= NETIF_F_SCTP_CSUM;
>>>
>>>  	dev->vlan_features = dev->features;
>>>  	dev->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;
>>
>> --
>> Ben Hutchings
>> Q.  Which is the greater problem in the world today, ignorance or apathy?
>> A.  I don't know and I couldn't care less.
> N�����r��y���b�X��ǧv�^�)޺{.n�+���z�^�)���w*\x1fjg���\x1e�����ݢj/���z�ޖ��2�ޙ���&�)ߡ�a��\x7f��\x1e�G���h�\x0f�j:+v���w�٥
> 

Re: [net-next] vxlan: Advertise SCTP checksum offloads

[Joseph Gasparakis]

On Tue, 29 Apr 2014, Vlad Yasevich wrote:

> On 04/29/2014 12:10 PM, Rose, Gregory V wrote:
> > 
> >> -----Original Message-----
> >> From: Ben Hutchings [mailto:ben@decadent.org.uk]
> >> Sent: Tuesday, April 29, 2014 5:39 AM
> >> To: Kirsher, Jeffrey T
> >> Cc: davem@davemloft.net; Rose, Gregory V; netdev@vger.kernel.org;
> >> gospo@redhat.com; sassmann@redhat.com
> >> Subject: Re: [net-next] vxlan: Advertise SCTP checksum offloads
> >>
> >> On Mon, 2014-04-28 at 12:52 -0700, Jeff Kirsher wrote:
> >>> From: Greg Rose <gregory.v.rose@intel.com>
> >>>
> >>> Some HW can offload encapsulated SCTP checksums.  Advertise the
> >>> capability for such cases.
> >>
> >> Don't we need a software fallback on the output path before doing this?
> > 
> > I believe that the vxlan driver will compare offload features with the lower device hw_enc_feature flags and if the HW doesn't have the offload then it won't be advertised to the sctp driver.  In that case the sctp driver will do its own checksumming.  At least that is the behavior I saw.
> > 
> 
> I've been looking at this and can't see vxlan doing any feature
> comparisons/disabling...
> 
> At the time SCTP tries to do checksum, it will see hw offload
> support and set checksum as partial.  That will be propagated
> all the way to vxlan which will set encapsulation and pass it on.
> At next dev_hard_start_xmit(), we'll look at hw_enc_features,
> but it won't matter since SCTP packets are never GSO.
> 
> So, we'll use the non-gso code path and still send with partial
> checksum set.  Now, if the driver supports encapsulated SCTP,
> then it will work.  If it doesn't, the udp checksum will be computed,
> but not the sctp one.
> 
> -vlad

That is exactly the way it works, dev_hard_start_xmit() is where 
the (inner) unchecksummed packets will get checksummed if the Tx'ing 
interface does not support inner SCTP checksumming.

In effect by "falsely" advertising vxlan being capable to "offload" a 
protocol, we just postpone the software csum calculation until the last 
minute, which is dev_hard_start_xmit().

Joseph

> 
> > I'm adding Joseph Gasparakis.  He might have a better answer.
> > 
> > - Greg
> > 
> >>
> >> Ben.
> >>
> >>> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> >>> Acked-by: Joseph Gasparakis <joseph.gasparakis@intel.com>
> >>> Acked-by: Shannon Nelson <shannon.nelson@intel.com>
> >>> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> >>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> >>> ---
> >>>  drivers/net/vxlan.c | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index
> >>> 1dfee9a..988208f 100644
> >>> --- a/drivers/net/vxlan.c
> >>> +++ b/drivers/net/vxlan.c
> >>> @@ -2293,6 +2293,7 @@ static void vxlan_setup(struct net_device *dev)
> >>>  	dev->features	|= NETIF_F_SG | NETIF_F_HW_CSUM;
> >>>  	dev->features   |= NETIF_F_RXCSUM;
> >>>  	dev->features   |= NETIF_F_GSO_SOFTWARE;
> >>> +	dev->features   |= NETIF_F_SCTP_CSUM;
> >>>
> >>>  	dev->vlan_features = dev->features;
> >>>  	dev->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;
> >>
> >> --
> >> Ben Hutchings
> >> Q.  Which is the greater problem in the world today, ignorance or apathy?
> >> A.  I don't know and I couldn't care less.
> > N???????????????r??????y?????????b???X??????ǧv???^???)޺{.n???+?????????z???^???)?????????w*\x1fjg?????????\x1e???????????????ݢj/?????????z???ޖ??????2???ޙ?????????&???)ߡ???a??????\x7f??????\x1e???G?????????h???\x0f???j:+v?????????w???٥
> > 
> 
> 

Re: [net-next] vxlan: Advertise SCTP checksum offloads

[Vlad Yasevich]

On 04/29/2014 01:34 PM, Joseph Gasparakis wrote:
> 
> 
> On Tue, 29 Apr 2014, Vlad Yasevich wrote:
> 
>> On 04/29/2014 12:10 PM, Rose, Gregory V wrote:
>>>
>>>> -----Original Message-----
>>>> From: Ben Hutchings [mailto:ben@decadent.org.uk]
>>>> Sent: Tuesday, April 29, 2014 5:39 AM
>>>> To: Kirsher, Jeffrey T
>>>> Cc: davem@davemloft.net; Rose, Gregory V; netdev@vger.kernel.org;
>>>> gospo@redhat.com; sassmann@redhat.com
>>>> Subject: Re: [net-next] vxlan: Advertise SCTP checksum offloads
>>>>
>>>> On Mon, 2014-04-28 at 12:52 -0700, Jeff Kirsher wrote:
>>>>> From: Greg Rose <gregory.v.rose@intel.com>
>>>>>
>>>>> Some HW can offload encapsulated SCTP checksums.  Advertise the
>>>>> capability for such cases.
>>>>
>>>> Don't we need a software fallback on the output path before doing this?
>>>
>>> I believe that the vxlan driver will compare offload features with the lower device hw_enc_feature flags and if the HW doesn't have the offload then it won't be advertised to the sctp driver.  In that case the sctp driver will do its own checksumming.  At least that is the behavior I saw.
>>>
>>
>> I've been looking at this and can't see vxlan doing any feature
>> comparisons/disabling...
>>
>> At the time SCTP tries to do checksum, it will see hw offload
>> support and set checksum as partial.  That will be propagated
>> all the way to vxlan which will set encapsulation and pass it on.
>> At next dev_hard_start_xmit(), we'll look at hw_enc_features,
>> but it won't matter since SCTP packets are never GSO.
>>
>> So, we'll use the non-gso code path and still send with partial
>> checksum set.  Now, if the driver supports encapsulated SCTP,
>> then it will work.  If it doesn't, the udp checksum will be computed,
>> but not the sctp one.
>>
>> -vlad
> 
> That is exactly the way it works, dev_hard_start_xmit() is where 
> the (inner) unchecksummed packets will get checksummed if the Tx'ing 
> interface does not support inner SCTP checksumming.

No, it will not.  dev_hard_start_xmit() calls skb_checksum_help()
which only knows how to calculate TCP/UDP checksums.  It doesn't know
how to compute CRC32c for SCTP protocol.

-vlad

> 
> In effect by "falsely" advertising vxlan being capable to "offload" a 
> protocol, we just postpone the software csum calculation until the last 
> minute, which is dev_hard_start_xmit().
> 
> Joseph
> 
>>
>>> I'm adding Joseph Gasparakis.  He might have a better answer.
>>>
>>> - Greg
>>>
>>>>
>>>> Ben.
>>>>
>>>>> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
>>>>> Acked-by: Joseph Gasparakis <joseph.gasparakis@intel.com>
>>>>> Acked-by: Shannon Nelson <shannon.nelson@intel.com>
>>>>> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
>>>>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>>>>> ---
>>>>>  drivers/net/vxlan.c | 1 +
>>>>>  1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index
>>>>> 1dfee9a..988208f 100644
>>>>> --- a/drivers/net/vxlan.c
>>>>> +++ b/drivers/net/vxlan.c
>>>>> @@ -2293,6 +2293,7 @@ static void vxlan_setup(struct net_device *dev)
>>>>>  	dev->features	|= NETIF_F_SG | NETIF_F_HW_CSUM;
>>>>>  	dev->features   |= NETIF_F_RXCSUM;
>>>>>  	dev->features   |= NETIF_F_GSO_SOFTWARE;
>>>>> +	dev->features   |= NETIF_F_SCTP_CSUM;
>>>>>
>>>>>  	dev->vlan_features = dev->features;
>>>>>  	dev->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;
>>>>
>>>> --
>>>> Ben Hutchings
>>>> Q.  Which is the greater problem in the world today, ignorance or apathy?
>>>> A.  I don't know and I couldn't care less.
>>> N???????????????r??????y?????????b???X??????ǧv???^???)޺{.n???+?????????z???^???)?????????w*\x1fjg?????????\x1e???????????????ݢj/?????????z???ޖ??????2???ޙ?????????&???)ߡ???a??????\x7f??????\x1e???G?????????h???\x0f???j:+v?????????w???٥
>>>
>>
>>

Re: [net-next] vxlan: Advertise SCTP checksum offloads

[Joseph Gasparakis]

On Tue, 29 Apr 2014, Vlad Yasevich wrote:

> On 04/29/2014 01:34 PM, Joseph Gasparakis wrote:
> > 
> > 
> > On Tue, 29 Apr 2014, Vlad Yasevich wrote:
> > 
> >> On 04/29/2014 12:10 PM, Rose, Gregory V wrote:
> >>>
> >>>> -----Original Message-----
> >>>> From: Ben Hutchings [mailto:ben@decadent.org.uk]
> >>>> Sent: Tuesday, April 29, 2014 5:39 AM
> >>>> To: Kirsher, Jeffrey T
> >>>> Cc: davem@davemloft.net; Rose, Gregory V; netdev@vger.kernel.org;
> >>>> gospo@redhat.com; sassmann@redhat.com
> >>>> Subject: Re: [net-next] vxlan: Advertise SCTP checksum offloads
> >>>>
> >>>> On Mon, 2014-04-28 at 12:52 -0700, Jeff Kirsher wrote:
> >>>>> From: Greg Rose <gregory.v.rose@intel.com>
> >>>>>
> >>>>> Some HW can offload encapsulated SCTP checksums.  Advertise the
> >>>>> capability for such cases.
> >>>>
> >>>> Don't we need a software fallback on the output path before doing this?
> >>>
> >>> I believe that the vxlan driver will compare offload features with the lower device hw_enc_feature flags and if the HW doesn't have the offload then it won't be advertised to the sctp driver.  In that case the sctp driver will do its own checksumming.  At least that is the behavior I saw.
> >>>
> >>
> >> I've been looking at this and can't see vxlan doing any feature
> >> comparisons/disabling...
> >>
> >> At the time SCTP tries to do checksum, it will see hw offload
> >> support and set checksum as partial.  That will be propagated
> >> all the way to vxlan which will set encapsulation and pass it on.
> >> At next dev_hard_start_xmit(), we'll look at hw_enc_features,
> >> but it won't matter since SCTP packets are never GSO.
> >>
> >> So, we'll use the non-gso code path and still send with partial
> >> checksum set.  Now, if the driver supports encapsulated SCTP,
> >> then it will work.  If it doesn't, the udp checksum will be computed,
> >> but not the sctp one.
> >>
> >> -vlad
> > 
> > That is exactly the way it works, dev_hard_start_xmit() is where 
> > the (inner) unchecksummed packets will get checksummed if the Tx'ing 
> > interface does not support inner SCTP checksumming.
> 
> No, it will not.  dev_hard_start_xmit() calls skb_checksum_help()
> which only knows how to calculate TCP/UDP checksums.  It doesn't know
> how to compute CRC32c for SCTP protocol.
> 
> -vlad
> 

Interesting, thanks for the clarification! Then how do things work with 
non-encapsulated SCTP traffic? Where is the csumming for SCTP take place 
if the NIC cannot offload the protocol?

> > 
> > In effect by "falsely" advertising vxlan being capable to "offload" a 
> > protocol, we just postpone the software csum calculation until the last 
> > minute, which is dev_hard_start_xmit().
> > 
> > Joseph
> > 
> >>
> >>> I'm adding Joseph Gasparakis.  He might have a better answer.
> >>>
> >>> - Greg
> >>>
> >>>>
> >>>> Ben.
> >>>>
> >>>>> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> >>>>> Acked-by: Joseph Gasparakis <joseph.gasparakis@intel.com>
> >>>>> Acked-by: Shannon Nelson <shannon.nelson@intel.com>
> >>>>> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> >>>>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> >>>>> ---
> >>>>>  drivers/net/vxlan.c | 1 +
> >>>>>  1 file changed, 1 insertion(+)
> >>>>>
> >>>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index
> >>>>> 1dfee9a..988208f 100644
> >>>>> --- a/drivers/net/vxlan.c
> >>>>> +++ b/drivers/net/vxlan.c
> >>>>> @@ -2293,6 +2293,7 @@ static void vxlan_setup(struct net_device *dev)
> >>>>>  	dev->features	|= NETIF_F_SG | NETIF_F_HW_CSUM;
> >>>>>  	dev->features   |= NETIF_F_RXCSUM;
> >>>>>  	dev->features   |= NETIF_F_GSO_SOFTWARE;
> >>>>> +	dev->features   |= NETIF_F_SCTP_CSUM;
> >>>>>
> >>>>>  	dev->vlan_features = dev->features;
> >>>>>  	dev->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;
> >>>>
> >>>> --
> >>>> Ben Hutchings
> >>>> Q.  Which is the greater problem in the world today, ignorance or apathy?
> >>>> A.  I don't know and I couldn't care less.
> >>> N???????????????r??????y?????????b???X??????ǧv???^???)޺{.n???+?????????z???^???)?????????w*\x1fjg?????????\x1e???????????????ݢj/?????????z???ޖ??????2???ޙ?????????&???)ߡ???a??????\x7f??????\x1e???G?????????h???\x0f???j:+v?????????w???٥
> >>>
> >>
> >>
> 
> 

Re: [net-next] vxlan: Advertise SCTP checksum offloads

[Vlad Yasevich]

On 04/29/2014 03:25 PM, Joseph Gasparakis wrote:
> 
> 
> On Tue, 29 Apr 2014, Vlad Yasevich wrote:
> 
>> On 04/29/2014 01:34 PM, Joseph Gasparakis wrote:
>>>
>>>
>>> On Tue, 29 Apr 2014, Vlad Yasevich wrote:
>>>
>>>> On 04/29/2014 12:10 PM, Rose, Gregory V wrote:
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Ben Hutchings [mailto:ben@decadent.org.uk]
>>>>>> Sent: Tuesday, April 29, 2014 5:39 AM
>>>>>> To: Kirsher, Jeffrey T
>>>>>> Cc: davem@davemloft.net; Rose, Gregory V; netdev@vger.kernel.org;
>>>>>> gospo@redhat.com; sassmann@redhat.com
>>>>>> Subject: Re: [net-next] vxlan: Advertise SCTP checksum offloads
>>>>>>
>>>>>> On Mon, 2014-04-28 at 12:52 -0700, Jeff Kirsher wrote:
>>>>>>> From: Greg Rose <gregory.v.rose@intel.com>
>>>>>>>
>>>>>>> Some HW can offload encapsulated SCTP checksums.  Advertise the
>>>>>>> capability for such cases.
>>>>>>
>>>>>> Don't we need a software fallback on the output path before doing this?
>>>>>
>>>>> I believe that the vxlan driver will compare offload features with the lower device hw_enc_feature flags and if the HW doesn't have the offload then it won't be advertised to the sctp driver.  In that case the sctp driver will do its own checksumming.  At least that is the behavior I saw.
>>>>>
>>>>
>>>> I've been looking at this and can't see vxlan doing any feature
>>>> comparisons/disabling...
>>>>
>>>> At the time SCTP tries to do checksum, it will see hw offload
>>>> support and set checksum as partial.  That will be propagated
>>>> all the way to vxlan which will set encapsulation and pass it on.
>>>> At next dev_hard_start_xmit(), we'll look at hw_enc_features,
>>>> but it won't matter since SCTP packets are never GSO.
>>>>
>>>> So, we'll use the non-gso code path and still send with partial
>>>> checksum set.  Now, if the driver supports encapsulated SCTP,
>>>> then it will work.  If it doesn't, the udp checksum will be computed,
>>>> but not the sctp one.
>>>>
>>>> -vlad
>>>
>>> That is exactly the way it works, dev_hard_start_xmit() is where 
>>> the (inner) unchecksummed packets will get checksummed if the Tx'ing 
>>> interface does not support inner SCTP checksumming.
>>
>> No, it will not.  dev_hard_start_xmit() calls skb_checksum_help()
>> which only knows how to calculate TCP/UDP checksums.  It doesn't know
>> how to compute CRC32c for SCTP protocol.
>>
>> -vlad
>>
> 
> Interesting, thanks for the clarification! Then how do things work with 
> non-encapsulated SCTP traffic? Where is the csumming for SCTP take place 
> if the NIC cannot offload the protocol?

In SCTP itself.  Look at sctp_packet_transmit().   There are also
fix-ups in netfilter to correct the checksum if the packet has
to be rerouted somewhere else.

We've tossed around the idea of making skb_checksum have pluggable
components.  Daniel started us down that path by adding checksum ops.
We just need registration and proper lookup support.

-vlad

> 
>>>
>>> In effect by "falsely" advertising vxlan being capable to "offload" a 
>>> protocol, we just postpone the software csum calculation until the last 
>>> minute, which is dev_hard_start_xmit().
>>>
>>> Joseph
>>>
>>>>
>>>>> I'm adding Joseph Gasparakis.  He might have a better answer.
>>>>>
>>>>> - Greg
>>>>>
>>>>>>
>>>>>> Ben.
>>>>>>
>>>>>>> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
>>>>>>> Acked-by: Joseph Gasparakis <joseph.gasparakis@intel.com>
>>>>>>> Acked-by: Shannon Nelson <shannon.nelson@intel.com>
>>>>>>> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
>>>>>>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>>>>>>> ---
>>>>>>>  drivers/net/vxlan.c | 1 +
>>>>>>>  1 file changed, 1 insertion(+)
>>>>>>>
>>>>>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index
>>>>>>> 1dfee9a..988208f 100644
>>>>>>> --- a/drivers/net/vxlan.c
>>>>>>> +++ b/drivers/net/vxlan.c
>>>>>>> @@ -2293,6 +2293,7 @@ static void vxlan_setup(struct net_device *dev)
>>>>>>>  	dev->features	|= NETIF_F_SG | NETIF_F_HW_CSUM;
>>>>>>>  	dev->features   |= NETIF_F_RXCSUM;
>>>>>>>  	dev->features   |= NETIF_F_GSO_SOFTWARE;
>>>>>>> +	dev->features   |= NETIF_F_SCTP_CSUM;
>>>>>>>
>>>>>>>  	dev->vlan_features = dev->features;
>>>>>>>  	dev->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;
>>>>>>
>>>>>> --
>>>>>> Ben Hutchings
>>>>>> Q.  Which is the greater problem in the world today, ignorance or apathy?
>>>>>> A.  I don't know and I couldn't care less.
>>>>> N???????????????r??????y?????????b???X??????ǧv???^???)޺{.n???+?????????z???^???)?????????w*\x1fjg?????????\x1e???????????????ݢj/?????????z???ޖ??????2???ޙ?????????&???)ߡ???a??????\x7f??????\x1e???G?????????h???\x0f???j:+v?????????w???٥
>>>>>
>>>>
>>>>
>>
>>

Re: [net-next] vxlan: Advertise SCTP checksum offloads

[Joseph Gasparakis]

On Tue, 29 Apr 2014, Vlad Yasevich wrote:

> On 04/29/2014 03:25 PM, Joseph Gasparakis wrote:
> > 
> > 
> > On Tue, 29 Apr 2014, Vlad Yasevich wrote:
> > 
> >> On 04/29/2014 01:34 PM, Joseph Gasparakis wrote:
> >>>
> >>>
> >>> On Tue, 29 Apr 2014, Vlad Yasevich wrote:
> >>>
> >>>> On 04/29/2014 12:10 PM, Rose, Gregory V wrote:
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Ben Hutchings [mailto:ben@decadent.org.uk]
> >>>>>> Sent: Tuesday, April 29, 2014 5:39 AM
> >>>>>> To: Kirsher, Jeffrey T
> >>>>>> Cc: davem@davemloft.net; Rose, Gregory V; netdev@vger.kernel.org;
> >>>>>> gospo@redhat.com; sassmann@redhat.com
> >>>>>> Subject: Re: [net-next] vxlan: Advertise SCTP checksum offloads
> >>>>>>
> >>>>>> On Mon, 2014-04-28 at 12:52 -0700, Jeff Kirsher wrote:
> >>>>>>> From: Greg Rose <gregory.v.rose@intel.com>
> >>>>>>>
> >>>>>>> Some HW can offload encapsulated SCTP checksums.  Advertise the
> >>>>>>> capability for such cases.
> >>>>>>
> >>>>>> Don't we need a software fallback on the output path before doing this?
> >>>>>
> >>>>> I believe that the vxlan driver will compare offload features with the lower device hw_enc_feature flags and if the HW doesn't have the offload then it won't be advertised to the sctp driver.  In that case the sctp driver will do its own checksumming.  At least that is the behavior I saw.
> >>>>>
> >>>>
> >>>> I've been looking at this and can't see vxlan doing any feature
> >>>> comparisons/disabling...
> >>>>
> >>>> At the time SCTP tries to do checksum, it will see hw offload
> >>>> support and set checksum as partial.  That will be propagated
> >>>> all the way to vxlan which will set encapsulation and pass it on.
> >>>> At next dev_hard_start_xmit(), we'll look at hw_enc_features,
> >>>> but it won't matter since SCTP packets are never GSO.
> >>>>
> >>>> So, we'll use the non-gso code path and still send with partial
> >>>> checksum set.  Now, if the driver supports encapsulated SCTP,
> >>>> then it will work.  If it doesn't, the udp checksum will be computed,
> >>>> but not the sctp one.
> >>>>
> >>>> -vlad
> >>>
> >>> That is exactly the way it works, dev_hard_start_xmit() is where 
> >>> the (inner) unchecksummed packets will get checksummed if the Tx'ing 
> >>> interface does not support inner SCTP checksumming.
> >>
> >> No, it will not.  dev_hard_start_xmit() calls skb_checksum_help()
> >> which only knows how to calculate TCP/UDP checksums.  It doesn't know
> >> how to compute CRC32c for SCTP protocol.
> >>
> >> -vlad
> >>
> > 
> > Interesting, thanks for the clarification! Then how do things work with 
> > non-encapsulated SCTP traffic? Where is the csumming for SCTP take place 
> > if the NIC cannot offload the protocol?
> 
> In SCTP itself.  Look at sctp_packet_transmit().   There are also
> fix-ups in netfilter to correct the checksum if the packet has
> to be rerouted somewhere else.
> 
> We've tossed around the idea of making skb_checksum have pluggable
> components.  Daniel started us down that path by adding checksum ops.
> We just need registration and proper lookup support.
> 
> -vlad
> 

I now totaly see your point. It seems like registering customized 
csumming/CRCing in skb_checksum is the right way to go. Thanks!

Joseph
> > 
> >>>
> >>> In effect by "falsely" advertising vxlan being capable to "offload" a 
> >>> protocol, we just postpone the software csum calculation until the last 
> >>> minute, which is dev_hard_start_xmit().
> >>>
> >>> Joseph
> >>>
> >>>>
> >>>>> I'm adding Joseph Gasparakis.  He might have a better answer.
> >>>>>
> >>>>> - Greg
> >>>>>
> >>>>>>
> >>>>>> Ben.
> >>>>>>
> >>>>>>> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> >>>>>>> Acked-by: Joseph Gasparakis <joseph.gasparakis@intel.com>
> >>>>>>> Acked-by: Shannon Nelson <shannon.nelson@intel.com>
> >>>>>>> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> >>>>>>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> >>>>>>> ---
> >>>>>>>  drivers/net/vxlan.c | 1 +
> >>>>>>>  1 file changed, 1 insertion(+)
> >>>>>>>
> >>>>>>> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index
> >>>>>>> 1dfee9a..988208f 100644
> >>>>>>> --- a/drivers/net/vxlan.c
> >>>>>>> +++ b/drivers/net/vxlan.c
> >>>>>>> @@ -2293,6 +2293,7 @@ static void vxlan_setup(struct net_device *dev)
> >>>>>>>  	dev->features	|= NETIF_F_SG | NETIF_F_HW_CSUM;
> >>>>>>>  	dev->features   |= NETIF_F_RXCSUM;
> >>>>>>>  	dev->features   |= NETIF_F_GSO_SOFTWARE;
> >>>>>>> +	dev->features   |= NETIF_F_SCTP_CSUM;
> >>>>>>>
> >>>>>>>  	dev->vlan_features = dev->features;
> >>>>>>>  	dev->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;
> >>>>>>
> >>>>>> --
> >>>>>> Ben Hutchings
> >>>>>> Q.  Which is the greater problem in the world today, ignorance or apathy?
> >>>>>> A.  I don't know and I couldn't care less.
> >>>>> N???????????????r??????y?????????b???X??????ǧv???^???)޺{.n???+?????????z???^???)?????????w*\x1fjg?????????\x1e???????????????ݢj/?????????z???ޖ??????2???ޙ?????????&???)ߡ???a??????\x7f??????\x1e???G?????????h???\x0f???j:+v?????????w???٥
> >>>>>
> >>>>
> >>>>
> >>
> >>
> 
> 

Re: [net-next] vxlan: Advertise SCTP checksum offloads

[Daniel Borkmann]

On 04/29/2014 10:59 PM, Joseph Gasparakis wrote:
> On Tue, 29 Apr 2014, Vlad Yasevich wrote:
...
>>>>>> At the time SCTP tries to do checksum, it will see hw offload
>>>>>> support and set checksum as partial.  That will be propagated
>>>>>> all the way to vxlan which will set encapsulation and pass it on.
>>>>>> At next dev_hard_start_xmit(), we'll look at hw_enc_features,
>>>>>> but it won't matter since SCTP packets are never GSO.
>>>>>>
>>>>>> So, we'll use the non-gso code path and still send with partial
>>>>>> checksum set.  Now, if the driver supports encapsulated SCTP,
>>>>>> then it will work.  If it doesn't, the udp checksum will be computed,
>>>>>> but not the sctp one.
>>>>>
>>>>> That is exactly the way it works, dev_hard_start_xmit() is where
>>>>> the (inner) unchecksummed packets will get checksummed if the Tx'ing
>>>>> interface does not support inner SCTP checksumming.
>>>>
>>>> No, it will not.  dev_hard_start_xmit() calls skb_checksum_help()
>>>> which only knows how to calculate TCP/UDP checksums.  It doesn't know
>>>> how to compute CRC32c for SCTP protocol.
>>>
>>> Interesting, thanks for the clarification! Then how do things work with
>>> non-encapsulated SCTP traffic? Where is the csumming for SCTP take place
>>> if the NIC cannot offload the protocol?
>>
>> In SCTP itself.  Look at sctp_packet_transmit().   There are also
>> fix-ups in netfilter to correct the checksum if the packet has
>> to be rerouted somewhere else.
>>
>> We've tossed around the idea of making skb_checksum have pluggable
>> components.  Daniel started us down that path by adding checksum ops.
>> We just need registration and proper lookup support.

Absolutely, Vlad is right here. That's also why we currently have this
ugly xfrm test in SCTP output path for csumming. I've started something
a while ago in that direction but got distracted with other things. If
someone is faster than me, I appreciate it. ;) In case not, I've kept
it on my todo list so far. Anyway, that would be the right way to go.

Re: [net-next] vxlan: Advertise SCTP checksum offloads

[Tom Herbert]

Hi Jeff,

Since SCTP is using a variant of CRC32 in a fairly generic manner,
would it be possible to provide a general API to offload it like in
NETIF_F_HW_CSUM? A strong CRC offloaded might be useful for other
protocols.

Thanks,
Tom


On Mon, Apr 28, 2014 at 12:52 PM, Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> From: Greg Rose <gregory.v.rose@intel.com>
>
> Some HW can offload encapsulated SCTP checksums.  Advertise the capability
> for such cases.
>
> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> Acked-by: Joseph Gasparakis <joseph.gasparakis@intel.com>
> Acked-by: Shannon Nelson <shannon.nelson@intel.com>
> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  drivers/net/vxlan.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 1dfee9a..988208f 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -2293,6 +2293,7 @@ static void vxlan_setup(struct net_device *dev)
>         dev->features   |= NETIF_F_SG | NETIF_F_HW_CSUM;
>         dev->features   |= NETIF_F_RXCSUM;
>         dev->features   |= NETIF_F_GSO_SOFTWARE;
> +       dev->features   |= NETIF_F_SCTP_CSUM;
>
>         dev->vlan_features = dev->features;
>         dev->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;
> --
> 1.9.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html