On Wed, Oct 16, 2019 at 3:23 PM Jakub Kicinski
<jakub.kicin...@netronome.com> wrote:
>
> To corrupt a GSO frame we first perform segmentation. We then
> proceed using the first segment instead of the full GSO skb and
> requeue the rest of the segments as separate packets.
>
> If there are any issues with processing the first segment we
> still want to process the rest, therefore we jump to the
> finish_segs label.
>
> Commit 177b8007463c ("net: netem: fix backlog accounting for
> corrupted GSO frames") started using the pointer to the first
> segment in the "rest of segments processing", but as mentioned
> above the first segment may had already been freed at this point.
>
> Backlog corrections for parent qdiscs have to be adjusted.
> Note that if segmentation ever produced a single-skb list
> the backlog calculation will not take place (segs == NULL)
> but that should hopefully never happen..
>
> Fixes: 177b8007463c ("net: netem: fix backlog accounting for corrupted GSO
> frames")
> Reported-by: kbuild test robot <l...@intel.com>
> Reported-by: Dan Carpenter <dan.carpen...@oracle.com>
> Reported-by: Ben Hutchings <b...@decadent.org.uk>
> Signed-off-by: Jakub Kicinski <jakub.kicin...@netronome.com>
> Reviewed-by: Simon Horman <simon.hor...@netronome.com>
> ---
> net/sched/sch_netem.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
> index 0e44039e729c..31a6afd035b2 100644
> --- a/net/sched/sch_netem.c
> +++ b/net/sched/sch_netem.c
> @@ -509,6 +509,7 @@ static int netem_enqueue(struct sk_buff *skb, struct
> Qdisc *sch,
> if (skb->ip_summed == CHECKSUM_PARTIAL &&
> skb_checksum_help(skb)) {
> qdisc_drop(skb, sch, to_free);
> + skb = NULL;
> goto finish_segs;
> }
>
> @@ -595,7 +596,7 @@ static int netem_enqueue(struct sk_buff *skb, struct
> Qdisc *sch,
> unsigned int len, last_len;
> int nb = 0;
>
> - len = skb->len;
> + len = skb ? skb->len : 0;
>
> while (segs) {
> skb2 = segs->next;
> @@ -612,7 +613,7 @@ static int netem_enqueue(struct sk_buff *skb, struct
> Qdisc *sch,
> }
> segs = skb2;
> }
> - qdisc_tree_reduce_backlog(sch, -nb, prev_len - len);
> + qdisc_tree_reduce_backlog(sch, !skb - nb, prev_len - len);
Am I the only one has trouble to understand the expression
"!skb - nb"?
