* [PATCH] net: use skb_is_gso_sctp() instead of open-coding
@ 2018-03-09 3:06 Daniel Axtens
2018-03-09 9:46 ` Daniel Borkmann
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Daniel Axtens @ 2018-03-09 3:06 UTC (permalink / raw)
To: netdev; +Cc: Daniel Axtens, Daniel Borkmann, Marcelo Ricardo Leitner
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(-)
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] net: use skb_is_gso_sctp() instead of open-coding
2018-03-09 3:06 [PATCH] net: use skb_is_gso_sctp() instead of open-coding Daniel Axtens
@ 2018-03-09 9:46 ` Daniel Borkmann
2018-03-09 16:42 ` David Miller
2018-03-09 20:10 ` Marcelo Ricardo Leitner
2 siblings, 0 replies; 4+ messages in thread
From: Daniel Borkmann @ 2018-03-09 9:46 UTC (permalink / raw)
To: Daniel Axtens, netdev; +Cc: Marcelo Ricardo Leitner
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: use skb_is_gso_sctp() instead of open-coding
2018-03-09 3:06 [PATCH] net: use skb_is_gso_sctp() instead of open-coding Daniel Axtens
2018-03-09 9:46 ` Daniel Borkmann
@ 2018-03-09 16:42 ` David Miller
2018-03-09 20:10 ` Marcelo Ricardo Leitner
2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2018-03-09 16:42 UTC (permalink / raw)
To: dja; +Cc: netdev, daniel, marcelo.leitner
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: use skb_is_gso_sctp() instead of open-coding
2018-03-09 3:06 [PATCH] net: use skb_is_gso_sctp() instead of open-coding Daniel Axtens
2018-03-09 9:46 ` Daniel Borkmann
2018-03-09 16:42 ` David Miller
@ 2018-03-09 20:10 ` Marcelo Ricardo Leitner
2 siblings, 0 replies; 4+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-03-09 20:10 UTC (permalink / raw)
To: Daniel Axtens; +Cc: netdev, Daniel Borkmann
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-03-09 20:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-09 3:06 [PATCH] net: use skb_is_gso_sctp() instead of open-coding Daniel Axtens
2018-03-09 9:46 ` Daniel Borkmann
2018-03-09 16:42 ` David Miller
2018-03-09 20:10 ` Marcelo Ricardo Leitner
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.