Message ID | 20180309030609.4806-1-dja@axtens.net |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
Series | net: use skb_is_gso_sctp() instead of open-coding | expand |
On 03/09/2018 04:06 AM, Daniel Axtens wrote: [...] > This depends on d02f51cbcf12 ("bpf: fix bpf_skb_adjust_net/bpf_skb_proto_xlat > to deal with gso sctp skbs") which introduces the required helper. > That is in the bpf tree at the moment. It's in net tree already by now: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/?id=d02f51cbcf12b09ab945873e35046045875eed9a
From: Daniel Axtens <dja@axtens.net> Date: Fri, 9 Mar 2018 14:06:09 +1100 > As well as the basic conversion, I noticed that a lot of the > SCTP code checks gso_type without first checking skb_is_gso() > so I have added that where appropriate. > > Also, document the helper. > > Cc: Daniel Borkmann <daniel@iogearbox.net> > Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > Signed-off-by: Daniel Axtens <dja@axtens.net> Applied, thank you. It might be nice to consolidate the TCP tests with a helper like "skb_is_gso_tcp()", but that's just a tidy up and not so pressing.
On Fri, Mar 09, 2018 at 02:06:09PM +1100, Daniel Axtens wrote: > As well as the basic conversion, I noticed that a lot of the > SCTP code checks gso_type without first checking skb_is_gso() > so I have added that where appropriate. > > Also, document the helper. Thanks Daniel. > > Cc: Daniel Borkmann <daniel@iogearbox.net> > Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > Signed-off-by: Daniel Axtens <dja@axtens.net> > > --- > > This depends on d02f51cbcf12 ("bpf: fix bpf_skb_adjust_net/bpf_skb_proto_xlat > to deal with gso sctp skbs") which introduces the required helper. > That is in the bpf tree at the moment. > --- > Documentation/networking/segmentation-offloads.txt | 5 ++++- > net/core/skbuff.c | 2 +- > net/sched/act_csum.c | 2 +- > net/sctp/input.c | 8 ++++---- > net/sctp/inqueue.c | 2 +- > net/sctp/offload.c | 2 +- > 6 files changed, 12 insertions(+), 9 deletions(-) > > diff --git a/Documentation/networking/segmentation-offloads.txt b/Documentation/networking/segmentation-offloads.txt > index 23a8dd91a9ec..36bb931b35e0 100644 > --- a/Documentation/networking/segmentation-offloads.txt > +++ b/Documentation/networking/segmentation-offloads.txt > @@ -155,7 +155,10 @@ Therefore, any code in the core networking stack must be aware of the > possibility that gso_size will be GSO_BY_FRAGS and handle that case > appropriately. > > -There are a couple of helpers to make this easier: > +There are some helpers to make this easier: > + > + - skb_is_gso(skb) && skb_is_gso_sctp(skb) is the best way to see if > + an skb is an SCTP GSO skb. > > - For size checks, the skb_gso_validate_*_len family of helpers correctly > considers GSO_BY_FRAGS. > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index 0bb0d8877954..baf990528943 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -4904,7 +4904,7 @@ static unsigned int skb_gso_transport_seglen(const struct sk_buff *skb) > thlen += inner_tcp_hdrlen(skb); > } else if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))) { > thlen = tcp_hdrlen(skb); > - } else if (unlikely(shinfo->gso_type & SKB_GSO_SCTP)) { > + } else if (unlikely(skb_is_gso_sctp(skb))) { > thlen = sizeof(struct sctphdr); > } > /* UFO sets gso_size to the size of the fragmentation > diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c > index b7ba9b06b147..24b2e8e681cf 100644 > --- a/net/sched/act_csum.c > +++ b/net/sched/act_csum.c > @@ -350,7 +350,7 @@ static int tcf_csum_sctp(struct sk_buff *skb, unsigned int ihl, > { > struct sctphdr *sctph; > > - if (skb_is_gso(skb) && skb_shinfo(skb)->gso_type & SKB_GSO_SCTP) > + if (skb_is_gso(skb) && skb_is_gso_sctp(skb)) > return 1; > > sctph = tcf_csum_skb_nextlayer(skb, ihl, ipl, sizeof(*sctph)); > diff --git a/net/sctp/input.c b/net/sctp/input.c > index 0247cc432e02..b381d78548ac 100644 > --- a/net/sctp/input.c > +++ b/net/sctp/input.c > @@ -106,6 +106,7 @@ int sctp_rcv(struct sk_buff *skb) > int family; > struct sctp_af *af; > struct net *net = dev_net(skb->dev); > + bool is_gso = skb_is_gso(skb) && skb_is_gso_sctp(skb); > > if (skb->pkt_type != PACKET_HOST) > goto discard_it; > @@ -123,8 +124,7 @@ int sctp_rcv(struct sk_buff *skb) > * it's better to just linearize it otherwise crc computing > * takes longer. > */ > - if ((!(skb_shinfo(skb)->gso_type & SKB_GSO_SCTP) && > - skb_linearize(skb)) || > + if ((!is_gso && skb_linearize(skb)) || > !pskb_may_pull(skb, sizeof(struct sctphdr))) > goto discard_it; > > @@ -135,7 +135,7 @@ int sctp_rcv(struct sk_buff *skb) > if (skb_csum_unnecessary(skb)) > __skb_decr_checksum_unnecessary(skb); > else if (!sctp_checksum_disable && > - !(skb_shinfo(skb)->gso_type & SKB_GSO_SCTP) && > + !is_gso && > sctp_rcv_checksum(net, skb) < 0) > goto discard_it; > skb->csum_valid = 1; > @@ -1218,7 +1218,7 @@ static struct sctp_association *__sctp_rcv_lookup_harder(struct net *net, > * issue as packets hitting this are mostly INIT or INIT-ACK and > * those cannot be on GSO-style anyway. > */ > - if ((skb_shinfo(skb)->gso_type & SKB_GSO_SCTP) == SKB_GSO_SCTP) > + if (skb_is_gso(skb) && skb_is_gso_sctp(skb)) > return NULL; > > ch = (struct sctp_chunkhdr *)skb->data; > diff --git a/net/sctp/inqueue.c b/net/sctp/inqueue.c > index 48392552ee7c..23ebc5318edc 100644 > --- a/net/sctp/inqueue.c > +++ b/net/sctp/inqueue.c > @@ -170,7 +170,7 @@ struct sctp_chunk *sctp_inq_pop(struct sctp_inq *queue) > > chunk = list_entry(entry, struct sctp_chunk, list); > > - if ((skb_shinfo(chunk->skb)->gso_type & SKB_GSO_SCTP) == SKB_GSO_SCTP) { > + if (skb_is_gso(chunk->skb) && skb_is_gso_sctp(chunk->skb)) { > /* GSO-marked skbs but without frags, handle > * them normally > */ > diff --git a/net/sctp/offload.c b/net/sctp/offload.c > index 35bc7106d182..123e9f2dc226 100644 > --- a/net/sctp/offload.c > +++ b/net/sctp/offload.c > @@ -45,7 +45,7 @@ static struct sk_buff *sctp_gso_segment(struct sk_buff *skb, > struct sk_buff *segs = ERR_PTR(-EINVAL); > struct sctphdr *sh; > > - if (!(skb_shinfo(skb)->gso_type & SKB_GSO_SCTP)) > + if (!skb_is_gso_sctp(skb)) > goto out; > > sh = sctp_hdr(skb); > -- > 2.14.1 >
diff --git a/Documentation/networking/segmentation-offloads.txt b/Documentation/networking/segmentation-offloads.txt index 23a8dd91a9ec..36bb931b35e0 100644 --- a/Documentation/networking/segmentation-offloads.txt +++ b/Documentation/networking/segmentation-offloads.txt @@ -155,7 +155,10 @@ Therefore, any code in the core networking stack must be aware of the possibility that gso_size will be GSO_BY_FRAGS and handle that case appropriately. -There are a couple of helpers to make this easier: +There are some helpers to make this easier: + + - skb_is_gso(skb) && skb_is_gso_sctp(skb) is the best way to see if + an skb is an SCTP GSO skb. - For size checks, the skb_gso_validate_*_len family of helpers correctly considers GSO_BY_FRAGS. diff --git a/net/core/skbuff.c b/net/core/skbuff.c index 0bb0d8877954..baf990528943 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -4904,7 +4904,7 @@ static unsigned int skb_gso_transport_seglen(const struct sk_buff *skb) thlen += inner_tcp_hdrlen(skb); } else if (likely(shinfo->gso_type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))) { thlen = tcp_hdrlen(skb); - } else if (unlikely(shinfo->gso_type & SKB_GSO_SCTP)) { + } else if (unlikely(skb_is_gso_sctp(skb))) { thlen = sizeof(struct sctphdr); } /* UFO sets gso_size to the size of the fragmentation diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c index b7ba9b06b147..24b2e8e681cf 100644 --- a/net/sched/act_csum.c +++ b/net/sched/act_csum.c @@ -350,7 +350,7 @@ static int tcf_csum_sctp(struct sk_buff *skb, unsigned int ihl, { struct sctphdr *sctph; - if (skb_is_gso(skb) && skb_shinfo(skb)->gso_type & SKB_GSO_SCTP) + if (skb_is_gso(skb) && skb_is_gso_sctp(skb)) return 1; sctph = tcf_csum_skb_nextlayer(skb, ihl, ipl, sizeof(*sctph)); diff --git a/net/sctp/input.c b/net/sctp/input.c index 0247cc432e02..b381d78548ac 100644 --- a/net/sctp/input.c +++ b/net/sctp/input.c @@ -106,6 +106,7 @@ int sctp_rcv(struct sk_buff *skb) int family; struct sctp_af *af; struct net *net = dev_net(skb->dev); + bool is_gso = skb_is_gso(skb) && skb_is_gso_sctp(skb); if (skb->pkt_type != PACKET_HOST) goto discard_it; @@ -123,8 +124,7 @@ int sctp_rcv(struct sk_buff *skb) * it's better to just linearize it otherwise crc computing * takes longer. */ - if ((!(skb_shinfo(skb)->gso_type & SKB_GSO_SCTP) && - skb_linearize(skb)) || + if ((!is_gso && skb_linearize(skb)) || !pskb_may_pull(skb, sizeof(struct sctphdr))) goto discard_it; @@ -135,7 +135,7 @@ int sctp_rcv(struct sk_buff *skb) if (skb_csum_unnecessary(skb)) __skb_decr_checksum_unnecessary(skb); else if (!sctp_checksum_disable && - !(skb_shinfo(skb)->gso_type & SKB_GSO_SCTP) && + !is_gso && sctp_rcv_checksum(net, skb) < 0) goto discard_it; skb->csum_valid = 1; @@ -1218,7 +1218,7 @@ static struct sctp_association *__sctp_rcv_lookup_harder(struct net *net, * issue as packets hitting this are mostly INIT or INIT-ACK and * those cannot be on GSO-style anyway. */ - if ((skb_shinfo(skb)->gso_type & SKB_GSO_SCTP) == SKB_GSO_SCTP) + if (skb_is_gso(skb) && skb_is_gso_sctp(skb)) return NULL; ch = (struct sctp_chunkhdr *)skb->data; diff --git a/net/sctp/inqueue.c b/net/sctp/inqueue.c index 48392552ee7c..23ebc5318edc 100644 --- a/net/sctp/inqueue.c +++ b/net/sctp/inqueue.c @@ -170,7 +170,7 @@ struct sctp_chunk *sctp_inq_pop(struct sctp_inq *queue) chunk = list_entry(entry, struct sctp_chunk, list); - if ((skb_shinfo(chunk->skb)->gso_type & SKB_GSO_SCTP) == SKB_GSO_SCTP) { + if (skb_is_gso(chunk->skb) && skb_is_gso_sctp(chunk->skb)) { /* GSO-marked skbs but without frags, handle * them normally */ diff --git a/net/sctp/offload.c b/net/sctp/offload.c index 35bc7106d182..123e9f2dc226 100644 --- a/net/sctp/offload.c +++ b/net/sctp/offload.c @@ -45,7 +45,7 @@ static struct sk_buff *sctp_gso_segment(struct sk_buff *skb, struct sk_buff *segs = ERR_PTR(-EINVAL); struct sctphdr *sh; - if (!(skb_shinfo(skb)->gso_type & SKB_GSO_SCTP)) + if (!skb_is_gso_sctp(skb)) goto out; sh = sctp_hdr(skb);
As well as the basic conversion, I noticed that a lot of the SCTP code checks gso_type without first checking skb_is_gso() so I have added that where appropriate. Also, document the helper. Cc: Daniel Borkmann <daniel@iogearbox.net> Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Signed-off-by: Daniel Axtens <dja@axtens.net> --- This depends on d02f51cbcf12 ("bpf: fix bpf_skb_adjust_net/bpf_skb_proto_xlat to deal with gso sctp skbs") which introduces the required helper. That is in the bpf tree at the moment. --- Documentation/networking/segmentation-offloads.txt | 5 ++++- net/core/skbuff.c | 2 +- net/sched/act_csum.c | 2 +- net/sctp/input.c | 8 ++++---- net/sctp/inqueue.c | 2 +- net/sctp/offload.c | 2 +- 6 files changed, 12 insertions(+), 9 deletions(-)