From: Tom Herbert <tom@herbertland.com> To: Davide Caratti <dcaratti@redhat.com> Cc: David Laight <David.Laight@aculab.com>, "David S . Miller" <davem@davemloft.net>, Linux Kernel Network Developers <netdev@vger.kernel.org>, "linux-sctp @ vger . kernel . org" <linux-sctp@vger.kernel.org>, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Subject: Re: [PATCH RFC net-next v2 3/4] net: more accurate checksumming in validate_xmit_skb Date: Tue, 28 Feb 2017 11:50:09 -0800 [thread overview] Message-ID: <CALx6S36A3Ai=6HJqpdQAm4W0parSG2_KmHZOAmHwomZY78ePvQ@mail.gmail.com> (raw) In-Reply-To: <03f4d4bb40e78f228923dd61bc3d30f737a7621e.1488199633.git.dcaratti@redhat.com> On Tue, Feb 28, 2017 at 2:32 AM, Davide Caratti <dcaratti@redhat.com> wrote: > Introduce skb->csum_not_inet to identify not-yet-checksummed SCTP packets. > Use this bit in combination with netdev feature bit in validate_xmit_skb, > to discriminate whether skb needs crc32c or 2-complement Internet Checksum > (or none of the two, when the underlying device can do checksum offload). > > Signed-off-by: Davide Caratti <dcaratti@redhat.com> > --- > include/linux/skbuff.h | 1 + > net/core/dev.c | 14 ++++++++++++-- > net/netfilter/ipvs/ip_vs_proto_sctp.c | 1 + > net/netfilter/nf_nat_proto_sctp.c | 1 + > net/sched/act_csum.c | 1 + > net/sctp/output.c | 1 + > 6 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > index 0671131..236b7d9 100644 > --- a/include/linux/skbuff.h > +++ b/include/linux/skbuff.h > @@ -759,6 +759,7 @@ struct sk_buff { > __u8 tc_redirected:1; > __u8 tc_from_ingress:1; > #endif > + __u8 csum_not_inet:1; > Unfortunately this potentially pushes the skbuf flags over 32 bits if I count correctly. I suggest that you rename csum_bad to csum_not_inet. Looks like csum_bad is only set by a grand total of one driver and I don't believe that is enough to justify its existence. It's probably a good time to remove it. > #ifdef CONFIG_NET_SCHED > __u16 tc_index; /* traffic control index */ > diff --git a/net/core/dev.c b/net/core/dev.c > index b9fb843..fae3217 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -2614,6 +2614,7 @@ int skb_sctp_csum_help(struct sk_buff *skb) > } > *(__le32 *)(skb->data + offset) = crc32c_csum; > skb->ip_summed = CHECKSUM_NONE; > + skb->csum_not_inet = 0; > out: > return ret; > } > @@ -2960,6 +2961,16 @@ static struct sk_buff *validate_xmit_vlan(struct sk_buff *skb, > return skb; > } > > +static int skb_csum_hwoffload_help(struct sk_buff *skb, > + netdev_features_t features) > +{ > + if (unlikely(skb->csum_not_inet)) > + return !(features & NETIF_F_SCTP_CRC) ? > + skb_sctp_csum_help(skb) : 0; > + Return value looks complex. Maybe we should just change skb_csum_*_help to return bool, true of checksum was handled false if not. > + return !(features & NETIF_F_CSUM_MASK) ? skb_checksum_help(skb) : 0; > +} > + > static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device *dev) > { > netdev_features_t features; > @@ -2995,8 +3006,7 @@ static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device > else > skb_set_transport_header(skb, > skb_checksum_start_offset(skb)); > - if (!(features & NETIF_F_CSUM_MASK) && > - skb_checksum_help(skb)) > + if (skb_csum_hwoffload_help(skb, features)) > goto out_kfree_skb; > } > } > diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c > index d952d67..4972a60 100644 > --- a/net/netfilter/ipvs/ip_vs_proto_sctp.c > +++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c > @@ -81,6 +81,7 @@ static void sctp_nat_csum(struct sk_buff *skb, sctp_sctphdr_t *sctph, > unsigned int sctphoff) > { > sctph->checksum = sctp_compute_cksum(skb, sctphoff); > + skb->csum_not_inet = 0; > skb->ip_summed = CHECKSUM_UNNECESSARY; > } > > diff --git a/net/netfilter/nf_nat_proto_sctp.c b/net/netfilter/nf_nat_proto_sctp.c > index 31d3586..9459b88 100644 > --- a/net/netfilter/nf_nat_proto_sctp.c > +++ b/net/netfilter/nf_nat_proto_sctp.c > @@ -49,6 +49,7 @@ sctp_manip_pkt(struct sk_buff *skb, > > if (skb->ip_summed != CHECKSUM_PARTIAL) { > hdr->checksum = sctp_compute_cksum(skb, hdroff); > + skb->csum_not_inet = 0; > skb->ip_summed = CHECKSUM_NONE; > } > > diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c > index e978ccd4..85cb150 100644 > --- a/net/sched/act_csum.c > +++ b/net/sched/act_csum.c > @@ -337,6 +337,7 @@ static int tcf_csum_sctp(struct sk_buff *skb, unsigned int ihl, > > sctph->checksum = sctp_compute_cksum(skb, > skb_network_offset(skb) + ihl); > + skb->csum_not_inet = 0; > skb->ip_summed = CHECKSUM_NONE; > > return 1; > diff --git a/net/sctp/output.c b/net/sctp/output.c > index 814eac0..0dc227b 100644 > --- a/net/sctp/output.c > +++ b/net/sctp/output.c > @@ -528,6 +528,7 @@ static int sctp_packet_pack(struct sctp_packet *packet, > } else { > chksum: > head->ip_summed = CHECKSUM_PARTIAL; > + head->csum_not_inet = 1; > head->csum_start = skb_transport_header(head) - head->head; > head->csum_offset = offsetof(struct sctphdr, checksum); > } > -- > 2.7.4 >
WARNING: multiple messages have this Message-ID (diff)
From: Tom Herbert <tom@herbertland.com> To: Davide Caratti <dcaratti@redhat.com> Cc: David Laight <David.Laight@aculab.com>, "David S . Miller" <davem@davemloft.net>, Linux Kernel Network Developers <netdev@vger.kernel.org>, "linux-sctp @ vger . kernel . org" <linux-sctp@vger.kernel.org>, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Subject: Re: [PATCH RFC net-next v2 3/4] net: more accurate checksumming in validate_xmit_skb Date: Tue, 28 Feb 2017 19:50:09 +0000 [thread overview] Message-ID: <CALx6S36A3Ai=6HJqpdQAm4W0parSG2_KmHZOAmHwomZY78ePvQ@mail.gmail.com> (raw) In-Reply-To: <03f4d4bb40e78f228923dd61bc3d30f737a7621e.1488199633.git.dcaratti@redhat.com> On Tue, Feb 28, 2017 at 2:32 AM, Davide Caratti <dcaratti@redhat.com> wrote: > Introduce skb->csum_not_inet to identify not-yet-checksummed SCTP packets. > Use this bit in combination with netdev feature bit in validate_xmit_skb, > to discriminate whether skb needs crc32c or 2-complement Internet Checksum > (or none of the two, when the underlying device can do checksum offload). > > Signed-off-by: Davide Caratti <dcaratti@redhat.com> > --- > include/linux/skbuff.h | 1 + > net/core/dev.c | 14 ++++++++++++-- > net/netfilter/ipvs/ip_vs_proto_sctp.c | 1 + > net/netfilter/nf_nat_proto_sctp.c | 1 + > net/sched/act_csum.c | 1 + > net/sctp/output.c | 1 + > 6 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > index 0671131..236b7d9 100644 > --- a/include/linux/skbuff.h > +++ b/include/linux/skbuff.h > @@ -759,6 +759,7 @@ struct sk_buff { > __u8 tc_redirected:1; > __u8 tc_from_ingress:1; > #endif > + __u8 csum_not_inet:1; > Unfortunately this potentially pushes the skbuf flags over 32 bits if I count correctly. I suggest that you rename csum_bad to csum_not_inet. Looks like csum_bad is only set by a grand total of one driver and I don't believe that is enough to justify its existence. It's probably a good time to remove it. > #ifdef CONFIG_NET_SCHED > __u16 tc_index; /* traffic control index */ > diff --git a/net/core/dev.c b/net/core/dev.c > index b9fb843..fae3217 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -2614,6 +2614,7 @@ int skb_sctp_csum_help(struct sk_buff *skb) > } > *(__le32 *)(skb->data + offset) = crc32c_csum; > skb->ip_summed = CHECKSUM_NONE; > + skb->csum_not_inet = 0; > out: > return ret; > } > @@ -2960,6 +2961,16 @@ static struct sk_buff *validate_xmit_vlan(struct sk_buff *skb, > return skb; > } > > +static int skb_csum_hwoffload_help(struct sk_buff *skb, > + netdev_features_t features) > +{ > + if (unlikely(skb->csum_not_inet)) > + return !(features & NETIF_F_SCTP_CRC) ? > + skb_sctp_csum_help(skb) : 0; > + Return value looks complex. Maybe we should just change skb_csum_*_help to return bool, true of checksum was handled false if not. > + return !(features & NETIF_F_CSUM_MASK) ? skb_checksum_help(skb) : 0; > +} > + > static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device *dev) > { > netdev_features_t features; > @@ -2995,8 +3006,7 @@ static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct net_device > else > skb_set_transport_header(skb, > skb_checksum_start_offset(skb)); > - if (!(features & NETIF_F_CSUM_MASK) && > - skb_checksum_help(skb)) > + if (skb_csum_hwoffload_help(skb, features)) > goto out_kfree_skb; > } > } > diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c > index d952d67..4972a60 100644 > --- a/net/netfilter/ipvs/ip_vs_proto_sctp.c > +++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c > @@ -81,6 +81,7 @@ static void sctp_nat_csum(struct sk_buff *skb, sctp_sctphdr_t *sctph, > unsigned int sctphoff) > { > sctph->checksum = sctp_compute_cksum(skb, sctphoff); > + skb->csum_not_inet = 0; > skb->ip_summed = CHECKSUM_UNNECESSARY; > } > > diff --git a/net/netfilter/nf_nat_proto_sctp.c b/net/netfilter/nf_nat_proto_sctp.c > index 31d3586..9459b88 100644 > --- a/net/netfilter/nf_nat_proto_sctp.c > +++ b/net/netfilter/nf_nat_proto_sctp.c > @@ -49,6 +49,7 @@ sctp_manip_pkt(struct sk_buff *skb, > > if (skb->ip_summed != CHECKSUM_PARTIAL) { > hdr->checksum = sctp_compute_cksum(skb, hdroff); > + skb->csum_not_inet = 0; > skb->ip_summed = CHECKSUM_NONE; > } > > diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c > index e978ccd4..85cb150 100644 > --- a/net/sched/act_csum.c > +++ b/net/sched/act_csum.c > @@ -337,6 +337,7 @@ static int tcf_csum_sctp(struct sk_buff *skb, unsigned int ihl, > > sctph->checksum = sctp_compute_cksum(skb, > skb_network_offset(skb) + ihl); > + skb->csum_not_inet = 0; > skb->ip_summed = CHECKSUM_NONE; > > return 1; > diff --git a/net/sctp/output.c b/net/sctp/output.c > index 814eac0..0dc227b 100644 > --- a/net/sctp/output.c > +++ b/net/sctp/output.c > @@ -528,6 +528,7 @@ static int sctp_packet_pack(struct sctp_packet *packet, > } else { > chksum: > head->ip_summed = CHECKSUM_PARTIAL; > + head->csum_not_inet = 1; > head->csum_start = skb_transport_header(head) - head->head; > head->csum_offset = offsetof(struct sctphdr, checksum); > } > -- > 2.7.4 >
next prev parent reply other threads:[~2017-02-28 19:51 UTC|newest] Thread overview: 104+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-01-23 16:52 [RFC PATCH net-next 0/5] net: improve support for SCTP checksums Davide Caratti 2017-01-23 16:52 ` Davide Caratti 2017-01-23 16:52 ` [RFC PATCH net-next 1/5] skbuff: add stub to help computing crc32c on SCTP packets Davide Caratti 2017-01-23 16:52 ` Davide Caratti 2017-01-23 16:52 ` [RFC PATCH net-next 2/5] net: split skb_checksum_help Davide Caratti 2017-01-23 16:52 ` Davide Caratti 2017-01-23 20:59 ` Tom Herbert 2017-01-23 20:59 ` Tom Herbert 2017-01-24 16:35 ` David Laight 2017-01-24 16:35 ` David Laight 2017-02-02 15:07 ` Davide Caratti 2017-02-02 15:07 ` Davide Caratti 2017-02-02 16:55 ` David Laight 2017-02-02 16:55 ` David Laight 2017-02-02 18:08 ` Tom Herbert 2017-02-02 18:08 ` Tom Herbert 2017-02-27 13:39 ` Davide Caratti 2017-02-27 13:39 ` Davide Caratti 2017-02-27 15:11 ` Tom Herbert 2017-02-27 15:11 ` Tom Herbert 2017-02-28 10:31 ` Davide Caratti 2017-02-28 10:31 ` Davide Caratti 2017-02-28 10:32 ` [PATCH RFC net-next v2 1/4] skbuff: add stub to help computing crc32c on SCTP packets Davide Caratti 2017-02-28 10:32 ` Davide Caratti 2017-02-28 10:32 ` [PATCH RFC net-next v2 2/4] net: introduce skb_sctp_csum_help Davide Caratti 2017-02-28 10:32 ` Davide Caratti 2017-02-28 10:32 ` [PATCH RFC net-next v2 3/4] net: more accurate checksumming in validate_xmit_skb Davide Caratti 2017-02-28 10:32 ` Davide Caratti 2017-02-28 19:50 ` Tom Herbert [this message] 2017-02-28 19:50 ` Tom Herbert 2017-02-28 10:32 ` [PATCH RFC net-next v2 4/4] Documentation: update notes on checksum offloading Davide Caratti 2017-02-28 10:32 ` Davide Caratti 2017-02-28 22:46 ` [PATCH RFC net-next v2 1/4] skbuff: add stub to help computing crc32c on SCTP packets Alexander Duyck 2017-02-28 22:46 ` Alexander Duyck 2017-03-01 3:17 ` Tom Herbert 2017-03-01 3:17 ` Tom Herbert 2017-03-01 10:53 ` David Laight 2017-03-01 10:53 ` David Laight 2017-03-06 21:51 ` Davide Caratti 2017-03-06 21:51 ` Davide Caratti 2017-03-07 18:06 ` Alexander Duyck 2017-03-07 18:06 ` Alexander Duyck 2017-03-18 13:17 ` Davide Caratti 2017-03-18 13:17 ` Davide Caratti 2017-03-18 22:35 ` Tom Herbert 2017-03-18 22:35 ` Tom Herbert 2017-04-07 14:16 ` [PATCH RFC net-next v3 0/7] improve CRC32c in the forwarding path Davide Caratti 2017-04-07 14:16 ` Davide Caratti 2017-04-07 14:16 ` [PATCH RFC net-next v3 1/7] skbuff: add stub to help computing crc32c on SCTP packets Davide Caratti 2017-04-07 14:16 ` Davide Caratti 2017-04-07 14:16 ` [PATCH RFC net-next v3 2/7] net: introduce skb_crc32c_csum_help Davide Caratti 2017-04-07 14:16 ` Davide Caratti 2017-04-07 14:16 ` [PATCH RFC net-next v3 3/7] sk_buff: remove support for csum_bad in sk_buff Davide Caratti 2017-04-07 14:16 ` Davide Caratti 2017-04-07 14:16 ` [PATCH RFC net-next v3 4/7] net: use skb->csum_algo to identify packets needing crc32c Davide Caratti 2017-04-07 14:16 ` Davide Caratti 2017-04-07 15:43 ` Tom Herbert 2017-04-07 15:43 ` Tom Herbert 2017-04-07 17:29 ` Davide Caratti 2017-04-07 17:29 ` Davide Caratti 2017-04-07 18:11 ` Tom Herbert 2017-04-07 18:11 ` Tom Herbert 2017-04-13 10:36 ` Davide Caratti 2017-04-13 10:36 ` Davide Caratti 2017-04-20 13:38 ` [PATCH RFC net-next v4 0/7] net: improve support for SCTP checksums Davide Caratti 2017-04-20 13:38 ` Davide Caratti 2017-04-20 13:38 ` [PATCH RFC net-next v4 1/7] skbuff: add stub to help computing crc32c on SCTP packets Davide Caratti 2017-04-20 13:38 ` Davide Caratti 2017-04-20 13:38 ` [PATCH RFC net-next v4 2/7] net: introduce skb_crc32c_csum_help Davide Caratti 2017-04-20 13:38 ` Davide Caratti 2017-04-27 12:29 ` Marcelo Ricardo Leitner 2017-04-27 12:29 ` Marcelo Ricardo Leitner 2017-04-20 13:38 ` [PATCH RFC net-next v4 3/7] sk_buff: remove support for csum_bad in sk_buff Davide Caratti 2017-04-20 13:38 ` Davide Caratti 2017-04-27 1:34 ` [sk_buff] 95510aef27: BUG:Bad_page_state_in_process kernel test robot 2017-04-27 1:34 ` kernel test robot 2017-04-29 20:21 ` [PATCH RFC net-next v4 3/7] sk_buff: remove support for csum_bad in sk_buff Tom Herbert 2017-04-29 20:21 ` Tom Herbert 2017-04-20 13:38 ` [PATCH RFC net-next v4 4/7] net: use skb->csum_not_inet to identify packets needing crc32c Davide Caratti 2017-04-20 13:38 ` Davide Caratti 2017-04-29 20:18 ` Tom Herbert 2017-04-29 20:18 ` Tom Herbert 2017-04-20 13:38 ` [PATCH RFC net-next v4 5/7] net: more accurate checksumming in validate_xmit_skb() Davide Caratti 2017-04-20 13:38 ` Davide Caratti 2017-04-20 13:38 ` [PATCH RFC net-next v4 6/7] openvswitch: more accurate checksumming in queue_userspace_packet() Davide Caratti 2017-04-20 13:38 ` Davide Caratti 2017-04-20 13:38 ` [PATCH RFC net-next v4 7/7] sk_buff.h: improve description of CHECKSUM_{COMPLETE,UNNECESSARY} Davide Caratti 2017-04-20 13:38 ` Davide Caratti 2017-04-29 20:20 ` Tom Herbert 2017-04-29 20:20 ` Tom Herbert 2017-04-27 12:41 ` [PATCH RFC net-next v4 0/7] net: improve support for SCTP checksums Marcelo Ricardo Leitner 2017-04-27 12:41 ` Marcelo Ricardo Leitner 2017-04-07 14:16 ` [PATCH RFC net-next v3 5/7] net: more accurate checksumming in validate_xmit_skb() Davide Caratti 2017-04-07 14:16 ` Davide Caratti 2017-04-07 14:16 ` [PATCH RFC net-next v3 6/7] openvswitch: more accurate checksumming in queue_userspace_packet() Davide Caratti 2017-04-07 14:16 ` Davide Caratti 2017-04-07 14:16 ` [PATCH RFC net-next v3 7/7] sk_buff.h: improve description of CHECKSUM_{COMPLETE,UNNECESSARY} Davide Caratti 2017-04-07 14:16 ` Davide Caratti 2017-01-23 16:52 ` [RFC PATCH net-next 3/5] net: introduce skb_sctp_csum_help Davide Caratti 2017-01-23 16:52 ` Davide Caratti 2017-01-23 16:52 ` [RFC PATCH net-next 4/5] net: more accurate checksumming in validate_xmit_skb Davide Caratti 2017-01-23 16:52 ` Davide Caratti 2017-01-23 16:52 ` [RFC PATCH net-next 5/5] Documentation: add description of skb_sctp_csum_help Davide Caratti 2017-01-23 16:52 ` Davide Caratti
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CALx6S36A3Ai=6HJqpdQAm4W0parSG2_KmHZOAmHwomZY78ePvQ@mail.gmail.com' \ --to=tom@herbertland.com \ --cc=David.Laight@aculab.com \ --cc=davem@davemloft.net \ --cc=dcaratti@redhat.com \ --cc=linux-sctp@vger.kernel.org \ --cc=marcelo.leitner@gmail.com \ --cc=netdev@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.