> -----Original Message----- > From: Daniel Borkmann <daniel@xxxxxxxxxxxxx> > Sent: Thursday, January 6, 2022 3:35 PM > To: Tyler Wear (QUIC) <quic_twear@xxxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; bpf@xxxxxxxxxxxxxxx > Cc: maze@xxxxxxxxxx; yhs@xxxxxx; kafai@xxxxxx; toke@xxxxxxxxxx; Tyler Wear <quic_twear@xxxxxxxxxxx> > Subject: Re: [PATCH bpf-next v3] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP_SKB > > WARNING: This email originated from outside of Qualcomm. Please be wary of any links or attachments, and do not enable macros. > > On 1/6/22 1:43 AM, Tyler Wear wrote: > > From: Tyler Wear <quic_twear@xxxxxxxxxxx> > > > > Need to modify the ds field to support upcoming Wifi QoS Alliance spec. > > Instead of adding generic function for just modifying the ds field, > > add skb_store_bytes for BPF_PROG_TYPE_CGROUP_SKB. > > This allows other fields in the network and transport header to be > > modified in the future. > > > > Checksum API's also need to be added for completeness. > > > > It is not possible to use CGROUP_(SET|GET)SOCKOPT since the policy may > > change during runtime and would result in a large number of entries > > with wildcards. > > > > Signed-off-by: Tyler Wear <quic_twear@xxxxxxxxxxx> > > --- > > net/core/filter.c | 10 ++ > > .../bpf/prog_tests/cgroup_store_bytes.c | 97 +++++++++++++++++++ > > .../selftests/bpf/progs/cgroup_store_bytes.c | 64 ++++++++++++ > > 3 files changed, 171 insertions(+) > > create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_store_bytes.c > > create mode 100644 > > tools/testing/selftests/bpf/progs/cgroup_store_bytes.c > > > > diff --git a/net/core/filter.c b/net/core/filter.c index > > 6102f093d59a..ce01a8036361 100644 > > --- a/net/core/filter.c > > +++ b/net/core/filter.c > > @@ -7299,6 +7299,16 @@ cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) > > return &bpf_sk_storage_delete_proto; > > case BPF_FUNC_perf_event_output: > > return &bpf_skb_event_output_proto; > > + case BPF_FUNC_skb_store_bytes: > > + return &bpf_skb_store_bytes_proto; > > + case BPF_FUNC_csum_update: > > + return &bpf_csum_update_proto; > > + case BPF_FUNC_csum_level: > > + return &bpf_csum_level_proto; > > + case BPF_FUNC_l3_csum_replace: > > + return &bpf_l3_csum_replace_proto; > > + case BPF_FUNC_l4_csum_replace: > > + return &bpf_l4_csum_replace_proto; > > #ifdef CONFIG_SOCK_CGROUP_DATA > > case BPF_FUNC_skb_cgroup_id: > > return &bpf_skb_cgroup_id_proto; > > Do we need skb_share_check in the write helpers at these hook points when this goes beyond just reading? > > Thanks, > Daniel Is there a difference here between the cg_skb and other function proto's that would require skb_share_check? Since these function proto's already exist for other attach types it should be fine right?
