On Tue, 2016-09-27 at 23:59 +0300, Shmulik Ladkani wrote:
> Up until now, 'action mirred' supported only egress actions (either
> TCA_EGRESS_REDIR or TCA_EGRESS_MIRROR).
>
> This patch implements the corresponding ingress actions
> TCA_INGRESS_REDIR and TCA_INGRESS_MIRROR.
> - if (m->tcfm_mac_header_xmit)
> + /* If action's target direction differs than filter's direction,
> + * and devices expect a mac header on xmit, then mac push/pull is
> + * needed.
> + */
> + if (at != tcf_mirred_act_direction(m->tcfm_eaction) &&
Note that m->tcfm_eaction is read here.
> + m->tcfm_mac_header_xmit) {
> + if (at & AT_EGRESS) {
> + /* caught at egress, act ingress: pull mac */
> + mac_len = skb_network_header(skb) - skb_mac_header(skb);
> + skb_pull_rcsum(skb2, mac_len);
> + } else {
> + /* caught at ingress, act egress: push mac */
> skb_push_rcsum(skb2, skb->mac_len);
> + }
> }
>
> /* mirror is always swallowed */
> - if (m->tcfm_eaction != TCA_EGRESS_MIRROR)
> + if (tcf_mirred_is_act_redirect(m->tcfm_eaction))
> skb2->tc_verd = SET_TC_FROM(skb2->tc_verd, at);
>
> skb2->skb_iif = skb->dev->ifindex;
> skb2->dev = dev;
> - err = dev_queue_xmit(skb2);
Note that m->tcfm_eaction is read another time here.
> + if (tcf_mirred_act_direction(m->tcfm_eaction) & AT_EGRESS)
> + err = dev_queue_xmit(skb2);
> + else
> + netif_receive_skb(skb2);
>
Since this runs lockless, another cpu might change m->tcfm_eaction in
the middle, and you could call dev_queue_xmit(skb2) while the skb2 was
prepared for the opposite action.
I guess some drivers could crash, because they expect to find a MAC
header.
If not, a comment would be nice.
Thanks.
