On Tue, Jan 31, 2023 at 03:17:08PM -0800, Joanne Koong wrote:
> >
> > It's not clear how to deal with BPF_F_RECOMPUTE_CSUM though.
> > Expose __skb_postpull_rcsum/__skb_postpush_rcsum as kfuncs?
> > But that defeats Andrii's goal to use dynptr as a generic wrapper.
> > skb is quite special.
>
> If it's the common case that skbs use the same flag across writes in
> their bpf prog, then we can have bpf_dynptr_from_skb take in
> BPF_F_RECOMPUTE_CSUM/BPF_F_INVALIDATE_HASH in its flags arg and then
> always apply this when the skb does a write to packet data.
Remembering these flags at creation of dynptr is an interesting idea,
but it doesn't help with direct write into ptr returned from bpf_dynptr_slice.
The __skb_postpull_rcsum needs to be done before the write and
__skb_postpush_rcsum after the write.
> >
> > Maybe something like:
> > void *bpf_dynptr_slice(const struct bpf_dynptr *ptr, u32 offset, u32 len,
> > void *buffer, u32 buffer__sz)
> > {
> > if (skb_cloned()) {
> > skb_copy_bits(skb, offset, buffer, len);
> > return buffer;
> > }
> > return skb_header_pointer(...);
> > }
> >
> > When prog is just parsing the packet it doesn't need to finalize with bpf_dynptr_write.
> > The prog can always write into the pointer followed by if (p == buf) bpf_dynptr_write.
> > No need for rdonly flag, but extra copy is there in case of cloned which
> > could have been avoided with extra rd_only flag.
>
> We're able to track in the verifier whether the slice gets written to
> or not, so if it does get written to in the skb case, can't we just
> add in a call to bpf_try_make_writable() as a post-processing fixup
> that gets called before bpf_dynptr_slice? Then bpf_dynptr_slice() can
> just return a directly writable ptr and avoid the extra memcpy
It's doable, but bpf_try_make_writable can fail and it's much slower than memcpy.
I'm not sure what you're optimizing here.
