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 <j...@mojatatu.com> > > Signed-off-by: Qiaobin Fu <qiaob...@bu.edu> > Reviewed-by: Michel Machado <mic...@digirati.com.br> > --- >
[...] > @@ -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