On Thu, 2016-10-06 at 10:30 -0700, Cong Wang wrote: > On Thu, Oct 6, 2016 at 6:30 AM, Shmulik Ladkani > <shmulik.ladk...@gmail.com> wrote: > > Hi, > > > > On Mon, Oct 3, 2016 at 12:45 PM, Cong Wang <xiyou.wangc...@gmail.com> wrote: > >> On Thu, Sep 29, 2016 at 4:03 AM, Shmulik Ladkani > >> <shmulik.ladk...@gmail.com> wrote: > >>> skb2->skb_iif = skb->dev->ifindex; > >>> skb2->dev = dev; > >>> - err = dev_queue_xmit(skb2); > >>> + if (tcf_mirred_act_direction(m_eaction) & AT_EGRESS) > >>> + err = dev_queue_xmit(skb2); > >>> + else > >>> + netif_receive_skb(skb2); > >> > >> Any reason why not check the return value here? > > > > Rationale: netif_receive_skb returns err if there was no protocol > > handler to deliver the skb to. > > If skb is not caught by any protocol handler, this should not be > > considered an "ingress redirect" error. The redirect action should be > > considered successful. > > A quick grep shows there are many places returning NET_RX_DROP: > E.g.
And another quick grep shows that out of 142 drivers, only one [1] of them (incorrectly) checks netif_receive_skb() return value. Real question is more like : what is the impact of propagating an error at this point ? [1] drivers/net/caif/caif_virtio.c This is incorrect because at the driver layer, the packet was received and the rx_packets/rx_bytes counters _should_ be incremented regardless of packet being dropped or not by upper layers.
