* [PATCHv3 net] {xfrm, sctp} Stick to software crc32 even if hardware is capable of that
@ 2013-10-15 9:19 Fan Du
2013-10-16 1:05 ` Vlad Yasevich
0 siblings, 1 reply; 4+ messages in thread
From: Fan Du @ 2013-10-15 9:19 UTC (permalink / raw)
To: vyasevich, nhorman; +Cc: steffen.klassert, davem, netdev
igb/ixgbe have hardware sctp checksum support, when this feature is enabled
and also IPsec is armed to protect sctp traffic, ugly things happened as
xfrm_output checks CHECKSUM_PARTIAL to do check sum operation(sum every thing
up and pack the 16bits result in the checksum field). The result is fail
establishment of sctp communication.
Signed-off-by: Fan Du <fan.du@windriver.com>
Cc: Vlad Yasevich <vyasevich@gmail.com>
Cc: Neil Horman <nhorman@tuxdriver.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
---
v3:
- Rename is_xfrm_armed by dst_xfrm
- Move this funtion in include/net/dst.h
v2:
- Split v1 into two separate patches.
---
include/net/dst.h | 12 ++++++++++++
net/sctp/output.c | 3 ++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/include/net/dst.h b/include/net/dst.h
index 211dcf1..44995c1 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -478,10 +478,22 @@ static inline struct dst_entry *xfrm_lookup(struct net *net,
{
return dst_orig;
}
+
+static inline struct xfrm_state *dst_xfrm(const struct dst_entry *dst)
+{
+ return NULL;
+}
+
#else
struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig,
const struct flowi *fl, struct sock *sk,
int flags);
+
+/* skb attached with this dst needs transformation if dst->xfrm is valid */
+static inline struct xfrm_state *dst_xfrm(const struct dst_entry *dst)
+{
+ return dst->xfrm;
+}
#endif
#endif /* _NET_DST_H */
diff --git a/net/sctp/output.c b/net/sctp/output.c
index 0ac3a65..24b3718 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -536,7 +536,8 @@ int sctp_packet_transmit(struct sctp_packet *packet)
* by CRC32-C as described in <draft-ietf-tsvwg-sctpcsum-02.txt>.
*/
if (!sctp_checksum_disable) {
- if (!(dst->dev->features & NETIF_F_SCTP_CSUM)) {
+ if (!(dst->dev->features & NETIF_F_SCTP_CSUM) ||
+ (dst_xfrm(dst) != NULL)) {
__u32 crc32 = sctp_start_cksum((__u8 *)sh, cksum_buf_len);
/* 3) Put the resultant value into the checksum field in the
--
1.7.9.5
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCHv3 net] {xfrm, sctp} Stick to software crc32 even if hardware is capable of that
2013-10-15 9:19 [PATCHv3 net] {xfrm, sctp} Stick to software crc32 even if hardware is capable of that Fan Du
@ 2013-10-16 1:05 ` Vlad Yasevich
2013-10-16 1:52 ` Vlad Yasevich
0 siblings, 1 reply; 4+ messages in thread
From: Vlad Yasevich @ 2013-10-16 1:05 UTC (permalink / raw)
To: Fan Du, nhorman; +Cc: steffen.klassert, davem, netdev
On 10/15/2013 05:19 AM, Fan Du wrote:
> igb/ixgbe have hardware sctp checksum support, when this feature is enabled
> and also IPsec is armed to protect sctp traffic, ugly things happened as
> xfrm_output checks CHECKSUM_PARTIAL to do check sum operation(sum every thing
> up and pack the 16bits result in the checksum field). The result is fail
> establishment of sctp communication.
>
> Signed-off-by: Fan Du <fan.du@windriver.com>
> Cc: Vlad Yasevich <vyasevich@gmail.com>
> Cc: Neil Horman <nhorman@tuxdriver.com>
> Cc: Steffen Klassert <steffen.klassert@secunet.com>
> Acked-by: Vlad Yasevich <vyasevich@gmail.com>
Looks good to me.
-vlad
> ---
> v3:
> - Rename is_xfrm_armed by dst_xfrm
> - Move this funtion in include/net/dst.h
>
> v2:
> - Split v1 into two separate patches.
>
> ---
> include/net/dst.h | 12 ++++++++++++
> net/sctp/output.c | 3 ++-
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/dst.h b/include/net/dst.h
> index 211dcf1..44995c1 100644
> --- a/include/net/dst.h
> +++ b/include/net/dst.h
> @@ -478,10 +478,22 @@ static inline struct dst_entry *xfrm_lookup(struct net *net,
> {
> return dst_orig;
> }
> +
> +static inline struct xfrm_state *dst_xfrm(const struct dst_entry *dst)
> +{
> + return NULL;
> +}
> +
> #else
> struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig,
> const struct flowi *fl, struct sock *sk,
> int flags);
> +
> +/* skb attached with this dst needs transformation if dst->xfrm is valid */
> +static inline struct xfrm_state *dst_xfrm(const struct dst_entry *dst)
> +{
> + return dst->xfrm;
> +}
> #endif
>
> #endif /* _NET_DST_H */
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index 0ac3a65..24b3718 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -536,7 +536,8 @@ int sctp_packet_transmit(struct sctp_packet *packet)
> * by CRC32-C as described in <draft-ietf-tsvwg-sctpcsum-02.txt>.
> */
> if (!sctp_checksum_disable) {
> - if (!(dst->dev->features & NETIF_F_SCTP_CSUM)) {
> + if (!(dst->dev->features & NETIF_F_SCTP_CSUM) ||
> + (dst_xfrm(dst) != NULL)) {
> __u32 crc32 = sctp_start_cksum((__u8 *)sh, cksum_buf_len);
>
> /* 3) Put the resultant value into the checksum field in the
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCHv3 net] {xfrm, sctp} Stick to software crc32 even if hardware is capable of that
2013-10-16 1:05 ` Vlad Yasevich
@ 2013-10-16 1:52 ` Vlad Yasevich
2013-10-16 1:58 ` Fan Du
0 siblings, 1 reply; 4+ messages in thread
From: Vlad Yasevich @ 2013-10-16 1:52 UTC (permalink / raw)
To: Fan Du, nhorman; +Cc: steffen.klassert, davem, netdev
On 10/15/2013 09:05 PM, Vlad Yasevich wrote:
> On 10/15/2013 05:19 AM, Fan Du wrote:
>> igb/ixgbe have hardware sctp checksum support, when this feature is
>> enabled
>> and also IPsec is armed to protect sctp traffic, ugly things happened as
>> xfrm_output checks CHECKSUM_PARTIAL to do check sum operation(sum
>> every thing
>> up and pack the 16bits result in the checksum field). The result is fail
>> establishment of sctp communication.
>>
>> Signed-off-by: Fan Du <fan.du@windriver.com>
>> Cc: Vlad Yasevich <vyasevich@gmail.com>
>> Cc: Neil Horman <nhorman@tuxdriver.com>
>> Cc: Steffen Klassert <steffen.klassert@secunet.com>
>> Acked-by: Vlad Yasevich <vyasevich@gmail.com>
>
> Looks good to me.
>
Just tryied applying this and looks like you based this on net-next.
This fixes a rather ugly bug when checksum offloading is done.
I am going to rebase this and re-submit for net.
-vlad
> -vlad
>
>> ---
>> v3:
>> - Rename is_xfrm_armed by dst_xfrm
>> - Move this funtion in include/net/dst.h
>>
>> v2:
>> - Split v1 into two separate patches.
>>
>> ---
>> include/net/dst.h | 12 ++++++++++++
>> net/sctp/output.c | 3 ++-
>> 2 files changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/net/dst.h b/include/net/dst.h
>> index 211dcf1..44995c1 100644
>> --- a/include/net/dst.h
>> +++ b/include/net/dst.h
>> @@ -478,10 +478,22 @@ static inline struct dst_entry
>> *xfrm_lookup(struct net *net,
>> {
>> return dst_orig;
>> }
>> +
>> +static inline struct xfrm_state *dst_xfrm(const struct dst_entry *dst)
>> +{
>> + return NULL;
>> +}
>> +
>> #else
>> struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry
>> *dst_orig,
>> const struct flowi *fl, struct sock *sk,
>> int flags);
>> +
>> +/* skb attached with this dst needs transformation if dst->xfrm is
>> valid */
>> +static inline struct xfrm_state *dst_xfrm(const struct dst_entry *dst)
>> +{
>> + return dst->xfrm;
>> +}
>> #endif
>>
>> #endif /* _NET_DST_H */
>> diff --git a/net/sctp/output.c b/net/sctp/output.c
>> index 0ac3a65..24b3718 100644
>> --- a/net/sctp/output.c
>> +++ b/net/sctp/output.c
>> @@ -536,7 +536,8 @@ int sctp_packet_transmit(struct sctp_packet *packet)
>> * by CRC32-C as described in <draft-ietf-tsvwg-sctpcsum-02.txt>.
>> */
>> if (!sctp_checksum_disable) {
>> - if (!(dst->dev->features & NETIF_F_SCTP_CSUM)) {
>> + if (!(dst->dev->features & NETIF_F_SCTP_CSUM) ||
>> + (dst_xfrm(dst) != NULL)) {
>> __u32 crc32 = sctp_start_cksum((__u8 *)sh, cksum_buf_len);
>>
>> /* 3) Put the resultant value into the checksum field in
>> the
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCHv3 net] {xfrm, sctp} Stick to software crc32 even if hardware is capable of that
2013-10-16 1:52 ` Vlad Yasevich
@ 2013-10-16 1:58 ` Fan Du
0 siblings, 0 replies; 4+ messages in thread
From: Fan Du @ 2013-10-16 1:58 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: nhorman, steffen.klassert, davem, netdev
On 2013年10月16日 09:52, Vlad Yasevich wrote:
> On 10/15/2013 09:05 PM, Vlad Yasevich wrote:
>> On 10/15/2013 05:19 AM, Fan Du wrote:
>>> igb/ixgbe have hardware sctp checksum support, when this feature is
>>> enabled
>>> and also IPsec is armed to protect sctp traffic, ugly things happened as
>>> xfrm_output checks CHECKSUM_PARTIAL to do check sum operation(sum
>>> every thing
>>> up and pack the 16bits result in the checksum field). The result is fail
>>> establishment of sctp communication.
>>>
>>> Signed-off-by: Fan Du <fan.du@windriver.com>
>>> Cc: Vlad Yasevich <vyasevich@gmail.com>
>>> Cc: Neil Horman <nhorman@tuxdriver.com>
>>> Cc: Steffen Klassert <steffen.klassert@secunet.com>
>>> Acked-by: Vlad Yasevich <vyasevich@gmail.com>
>>
>> Looks good to me.
>>
>
> Just tryied applying this and looks like you based this on net-next.
> This fixes a rather ugly bug when checksum offloading is done.
> I am going to rebase this and re-submit for net.
:( Sorry for the inconvenience. Stable tree might hurts also.
--
浮沉随浪只记今朝笑
--fan
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-10-16 2:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-15 9:19 [PATCHv3 net] {xfrm, sctp} Stick to software crc32 even if hardware is capable of that Fan Du
2013-10-16 1:05 ` Vlad Yasevich
2013-10-16 1:52 ` Vlad Yasevich
2013-10-16 1:58 ` Fan Du
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).