> -----Original Message----- > From: Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> > Sent: Tuesday, January 18, 2022 12:38 PM > To: Maciej Żenczykowski <maze@xxxxxxxxxx> > Cc: Tyler Wear (QUIC) <quic_twear@xxxxxxxxxxx>; Network Development > <netdev@xxxxxxxxxxxxxxx>; bpf <bpf@xxxxxxxxxxxxxxx>; Yonghong Song > <yhs@xxxxxx>; Martin KaFai Lau <kafai@xxxxxx>; Toke Høiland-Jørgensen > <toke@xxxxxxxxxx>; Daniel Borkmann <daniel@xxxxxxxxxxxxx>; Song Liu > <song@xxxxxxxxxx> > Subject: Re: [PATCH bpf-next v6 1/2] 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 Fri, Jan 14, 2022 at 1:18 PM Maciej Żenczykowski <maze@xxxxxxxxxx> > wrote: > > > > > > > > This is wrong. > > > > > > CGROUP_INET_EGRESS bpf prog cannot arbitrary change packet > data. > > > > I agree with this sentiment, which is why the original proposal was > > simply to add a helper which is only capable of modifying the > > tos/tclass/dscp field, and not any arbitrary bytes. (note: there > > already is such a helper to set the ECN congestion notification bits, > > so there's somewhat of a precedent) > > True. bpf_skb_ecn_set_ce() is available to cg_skb progs. > An arbitrary tos rewriting helper would screw it up. Patch 1 was for a ds_field helper to modify the top 6 bits of TOS, not an arbitrary rewriting. This should suffice since it doesn't interfere with CE.