From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vlad Yasevich Subject: Re: [PATCHv2 1/2 ] {xfrm, sctp} Stick to software crc32 even if hardware is capable of that Date: Fri, 11 Oct 2013 13:12:03 -0400 Message-ID: <52583163.4000103@gmail.com> References: <1381384296-1821-1-git-send-email-fan.du@windriver.com> <20131010131116.GA16147@hmsreliant.think-freely.org> <5257A349.3010605@windriver.com> <5258055F.5060703@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: netdev@vger.kernel.org To: Fan Du , Neil Horman , steffen.klassert@secunet.com, davem@davemloft.net Return-path: Received: from mail-qe0-f51.google.com ([209.85.128.51]:43294 "EHLO mail-qe0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752235Ab3JKRMI (ORCPT ); Fri, 11 Oct 2013 13:12:08 -0400 Received: by mail-qe0-f51.google.com with SMTP id q19so3526858qeb.10 for ; Fri, 11 Oct 2013 10:12:07 -0700 (PDT) In-Reply-To: <5258055F.5060703@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: On 10/11/2013 10:04 AM, Vlad Yasevich wrote: > On 10/11/2013 03:05 AM, Fan Du wrote: >> >> >> On 2013=E5=B9=B410=E6=9C=8810=E6=97=A5 21:11, Neil Horman wrote: >>> On Thu, Oct 10, 2013 at 01:51:36PM +0800, Fan Du wrote: >>>> igb/ixgbe have hardware sctp checksum support, when this feature i= s >>>> 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 i= s >>>> fail >>>> establishment of sctp communication. >>>> >>> Shouldn't this be fixed in the xfrm code then? E.g. check the devi= ce >>> features >>> for SCTP checksum offloading and and skip the checksum during xfrm >>> output if its >>> available? >>> >>> Or am I missing something? >>> Neil >>> >>> >> >> >> From 014276de0877f11d46e1704114a7d91f19221a63 Mon Sep 17 00:00:00 2= 001 >> From: Fan Du >> Date: Fri, 11 Oct 2013 14:24:33 +0800 >> Subject: [PATCH 1/2] {xfrm, sctp} Stick to software crc32 even if >> hardware is >> capable of that >> >> igb/ixgbe have hardware sctp checksum support, when this feature is >> enabled >> and also IPsec is armed to protect sctp traffic, ugly things happene= d as >> xfrm_output checks CHECKSUM_PARTIAL to do check sum operation(sum ev= ery >> thing >> up and pack the 16bits result in the checksum field). The result is = fail >> establishment of sctp communication. >> >> Signed-off-by: Fan Du >> --- >> v2: >> Leave ip_summed as CHECKSUM_PARTIAL as before, the second patch w= ill >> fix this. >> >> --- >> net/sctp/output.c | 14 +++++++++++++- >> 1 file changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/net/sctp/output.c b/net/sctp/output.c >> index 0ac3a65..6de6402 100644 >> --- a/net/sctp/output.c >> +++ b/net/sctp/output.c >> @@ -372,6 +372,16 @@ static void sctp_packet_set_owner_w(struct sk_b= uff >> *skb, struct sock *sk) >> atomic_inc(&sk->sk_wmem_alloc); >> } >> >> +static int is_xfrm_armed(struct dst_entry *dst) >> +{ >> +#ifdef CONFIG_XFRM >> + /* If dst->xfrm is valid, this skb needs to be transformed */ >> + return dst->xfrm !=3D NULL; >> +#else >> + return 0; >> +#endif >> +} >> + > > I would really prefer to have an accessor function to dst->xfrm, but > I see that everyone codes it inside the #ifdef. Gack. > >> /* All packets are sent to the network through this function from >> * sctp_outq_tail(). >> * >> @@ -536,7 +546,9 @@ int sctp_packet_transmit(struct sctp_packet *pac= ket) >> * by CRC32-C as described in . >> */ >> if (!sctp_checksum_disable) { >> - if (!(dst->dev->features & NETIF_F_SCTP_CSUM)) { >> + if ((!(dst->dev->features & NETIF_F_SCTP_CSUM)) || >> + is_xfrm_armed(dst)) { >> + >> __u32 crc32 =3D sctp_start_cksum((__u8 *)sh, cksum_buf= _len); >> >> /* 3) Put the resultant value into the checksum field = in >> the > > Acked-by: Vlad Yasevich This patch doesn't seem to apply to net.git. -vlad