Re: [PATCH] bpf: Unshare cloned skb before devmap egress XDP program

["Emil Tsalapatis" <emil@etsalapatis.com>]

On Tue Jun 9, 2026 at 7:06 AM EDT, Menglong Dong wrote:
> On 2026/6/9 18:02 Sun Jian <sun.jian.kdev@gmail.com> write:
>> dev_map_redirect_clone() uses skb_clone() when redirecting a generic XDP
>> skb to multiple devmap destinations. The cloned skb can share packet data
>> with other clones.
>> 
>> If the destination devmap entry has an egress XDP program, that program
>> can modify packet data. Such modifications can then be observed by other
>> clones sharing the same packet data.
>> 
>> This can be reproduced by strengthening xdp_veth_egress to configure a
>> different source MAC for each egress device and checking that store_mac_1/2
>> observe the MAC configured for their own egress devices. Without the fix,
>> the SKB_MODE subtest observes store_mac_1 receiving the MAC configured for
>> the next egress device.
>> 
>> Fix this by unsharing the cloned skb before running the devmap egress XDP
>> program. Limit the extra copy to destinations with an attached egress
>> program.
>
> Hi, Jian.
>
> This sounds like a good idea in this case. When I have a look at bpf_clone_redirect(),
> I found that it use skb_clone() too, which means it has the same problem. The
> data can be modified by other xdp prog in the destination NIC if we use
> bpf_clone_redirect().
>
> So maybe this is the default logic, and I'm not sure if this patch can break the
> existing users :/

I think for use cases where we are using bpf_clone_redirect() to use
one clone for inspection this would add an unnecessary copy. Maybe
adding *_copy() variants instead of changing the *_clone() would be
better? That way we wouldn't be changing the behavior for existing
consumers and the naming would be consistent with the skb_* methods.

But more importantly, is there an actual use case for the kind of API
that the modified selftest requires? Nobody until now has considered
the existing behavior to be a problem.

>
> Thanks!
> Menglong Dong
>
>> 
>> Tested with:
>>   ./test_progs -t xdp_veth_egress
>>   ./test_progs -t xdp_veth
>>   ./test_progs -t xdp
> [...]
>>  
>>  destroy_xdp_redirect_map:
>> -- 
>> 2.43.0
>> 
>> 
>>