Re: [PATCH net v4 2/3] net: skbuff: don't skb_tx_error() the source skb in skb_zerocopy()

[Willem de Bruijn]

Norbert Szetei wrote:
> skb_zerocopy() copies frags from @from into @to. On an
> skb_orphan_frags() failure it calls skb_tx_error(@from), a destructive
> operation on the source skb the copy helper does not own. That completes
> @from's zerocopy uarg and clears SKBFL_ALL_ZEROCOPY, including the
> SKBFL_SHARED_FRAG page-ownership marker.
> 
> Both callers already report the failure on their own drop path.
> nfnetlink_queue does it at nla_put_failure, and Open vSwitch does it in
> the flow-miss drop arm of ovs_dp_process_packet(), so nothing is lost by
> dropping it here.
> 
> On Open vSwitch's OVS_ACTION_ATTR_USERSPACE path the skb is not freed on
> this error: do_execute_actions() ignores output_userspace()'s return
> value and, unless the upcall was the last action, keeps forwarding the
> same skb through the flow's remaining actions. The uarg is completed
> while that skb is still in flight, telling the producer its buffers are
> free, and SKBFL_SHARED_FRAG is cleared on an skb the rest of the stack
> still handles. That flag is what makes esp_input() call skb_cow_data()
> instead of decrypting in place, so a later local ESP delivery can
> decrypt over frags the skb does not own privately.
> 
> Leave error reporting to the callers.
> 
> Fixes: 36d5fe6a0007 ("core, nfqueue, openvswitch: Orphan frags in skb_zerocopy and handle errors")
> Cc: stable@xxxxxxxxxxxxxxx
> Suggested-by: Ilya Maximets <i.maximets@xxxxxxx>
> Signed-off-by: Norbert Szetei <norbert@xxxxxxxxxxxx>
> Reviewed-by: Ilya Maximets <i.maximets@xxxxxxx>

Reviewed-by: Willem de Bruijn <willemb@xxxxxxxxxx>