On Thu, Jun 13, 2019 at 10:44 AM John Hurley <john.hur...@netronome.com> wrote:
> +static inline void tcf_mpls_set_eth_type(struct sk_buff *skb, __be16
> ethertype)
> +{
> + struct ethhdr *hdr = eth_hdr(skb);
> +
> + skb_postpull_rcsum(skb, &hdr->h_proto, ETH_TLEN);
> + hdr->h_proto = ethertype;
> + skb_postpush_rcsum(skb, &hdr->h_proto, ETH_TLEN);
So you just want to adjust the checksum with the new ->h_proto
value. please use a right csum API, rather than skb_post*_rcsum().
> +}
> +
> +static int tcf_mpls_act(struct sk_buff *skb, const struct tc_action *a,
> + struct tcf_result *res)
> +{
> + struct tcf_mpls *m = to_mpls(a);
> + struct mpls_shim_hdr *lse;
> + struct tcf_mpls_params *p;
> + u32 temp_lse;
> + int ret;
> + u8 ttl;
> +
> + tcf_lastuse_update(&m->tcf_tm);
> + bstats_cpu_update(this_cpu_ptr(m->common.cpu_bstats), skb);
> +
> + /* Ensure 'data' points at mac_header prior calling mpls manipulating
> + * functions.
> + */
> + if (skb_at_tc_ingress(skb))
> + skb_push_rcsum(skb, skb->mac_len);
> +
> + ret = READ_ONCE(m->tcf_action);
> +
> + p = rcu_dereference_bh(m->mpls_p);
> +
> + switch (p->tcfm_action) {
> + case TCA_MPLS_ACT_POP:
> + if (unlikely(!eth_p_mpls(skb->protocol)))
> + goto out;
> +
> + if (unlikely(skb_ensure_writable(skb, ETH_HLEN + MPLS_HLEN)))
> + goto drop;
> +
> + skb_postpull_rcsum(skb, mpls_hdr(skb), MPLS_HLEN);
> + memmove(skb->data + MPLS_HLEN, skb->data, ETH_HLEN);
> +
> + __skb_pull(skb, MPLS_HLEN);
> + skb_reset_mac_header(skb);
> + skb_set_network_header(skb, ETH_HLEN);
> +
> + tcf_mpls_set_eth_type(skb, p->tcfm_proto);
> + skb->protocol = p->tcfm_proto;
> + break;
> + case TCA_MPLS_ACT_PUSH:
> + if (unlikely(skb_cow_head(skb, MPLS_HLEN)))
> + goto drop;
> +
> + skb_push(skb, MPLS_HLEN);
> + memmove(skb->data, skb->data + MPLS_HLEN, ETH_HLEN);
> + skb_reset_mac_header(skb);
> + skb_set_network_header(skb, ETH_HLEN);
> +
> + lse = mpls_hdr(skb);
> + lse->label_stack_entry = 0;
> + tcf_mpls_mod_lse(lse, p, !eth_p_mpls(skb->protocol));
> + skb_postpush_rcsum(skb, lse, MPLS_HLEN);
> +
> + tcf_mpls_set_eth_type(skb, p->tcfm_proto);
> + skb->protocol = p->tcfm_proto;
> + break;
Is it possible to refactor and reuse the similar code in
net/openvswitch/actions.c::pop_mpls()/push_mpls()?
> + case TCA_MPLS_ACT_MODIFY:
> + if (unlikely(!eth_p_mpls(skb->protocol)))
> + goto out;
> +
> + if (unlikely(skb_ensure_writable(skb, ETH_HLEN + MPLS_HLEN)))
> + goto drop;
> +
> + lse = mpls_hdr(skb);
> + skb_postpull_rcsum(skb, lse, MPLS_HLEN);
> + tcf_mpls_mod_lse(lse, p, false);
> + skb_postpush_rcsum(skb, lse, MPLS_HLEN);
> + break;
> + case TCA_MPLS_ACT_DEC_TTL:
> + if (unlikely(!eth_p_mpls(skb->protocol)))
> + goto out;
> +
> + if (unlikely(skb_ensure_writable(skb, ETH_HLEN + MPLS_HLEN)))
> + goto drop;
> +
> + lse = mpls_hdr(skb);
> + temp_lse = be32_to_cpu(lse->label_stack_entry);
> + ttl = (temp_lse & MPLS_LS_TTL_MASK) >> MPLS_LS_TTL_SHIFT;
> + if (!--ttl)
> + goto drop;
> +
> + temp_lse &= ~MPLS_LS_TTL_MASK;
> + temp_lse |= ttl << MPLS_LS_TTL_SHIFT;
> + skb_postpull_rcsum(skb, lse, MPLS_HLEN);
> + lse->label_stack_entry = cpu_to_be32(temp_lse);
> + skb_postpush_rcsum(skb, lse, MPLS_HLEN);
> + break;
> + default:
> + WARN_ONCE(1, "Invalid MPLS action\n");
This warning is not necessary, it must be validated in ->init().
> + }
> +
> +out:
> + if (skb_at_tc_ingress(skb))
> + skb_pull_rcsum(skb, skb->mac_len);
> +
> + return ret;
> +
> +drop:
> + qstats_drop_inc(this_cpu_ptr(m->common.cpu_qstats));
> + return TC_ACT_SHOT;
> +}
Thanks.
