Re: [PATCH v4 net-next] net:sched: add action inheritdsfield to skbedit
From: Davide Caratti <hidden>
Date: 2018-06-20 16:08:46
On Tue, 2018-06-12 at 15:42 +0000, Fu, Qiaobin wrote:
The new action inheritdsfield copies the field DS of IPv4 and IPv6 packets into skb->priority. This enables later classification of packets based on the DS field. v4: *Not allow setting flags other than the expected ones. *Allow dumping the pure flags. Original idea by Jamal Hadi Salim [off-list ref] Signed-off-by: Qiaobin Fu <redacted> Reviewed-by: Michel Machado <redacted> ---
[...]
quoted hunk ↗ jump to hunk
@@ -41,6 +44,25 @@ static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a, if (d->flags & SKBEDIT_F_PRIORITY) skb->priority = d->priority; + if (d->flags & SKBEDIT_F_INHERITDSFIELD) { + int wlen = skb_network_offset(skb); + + switch (tc_skb_protocol(skb)) { + case htons(ETH_P_IP): + wlen += sizeof(struct iphdr); + if (!pskb_may_pull(skb, wlen)) + goto err; + skb->priority = ipv4_get_dsfield(ip_hdr(skb)) >> 2; + break; + + case htons(ETH_P_IPV6): + wlen += sizeof(struct ipv6hdr); + if (!pskb_may_pull(skb, wlen)) + goto err; + skb->priority = ipv6_get_dsfield(ipv6_hdr(skb)) >> 2; + break; + } + } if (d->flags & SKBEDIT_F_QUEUE_MAPPING && skb->dev->real_num_tx_queues > d->queue_mapping) skb_set_queue_mapping(skb, d->queue_mapping);@@ -53,6 +75,10 @@ static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a, spin_unlock(&d->tcf_lock); return d->tcf_action; + +err: + spin_unlock(&d->tcf_lock); + return TC_ACT_SHOT; }
sorry for asking this when the patch is a v4... I spotted this, as I'm rebasing a small series that removes the tcf_lock from the data plane of skbedit to gain some speed, and it converts the stats to be per-cpu counters. in the code above, you are catching failures of pskb_may_pull(skb) and then you return TC_ACT_SHOT. That's OK, but I think you should update the drop counter, like other TC actions do. If you (author / reviewers) think this is a minor issue, like I do think, then I can add the missing update in my series and post it when net-next reopens. WDYT? thank you in advance! regards, -- davide