On 10/7/21 10:57 PM, Joanne Koong wrote:
On 10/7/21 7:41 AM, Toke Høiland-Jørgensen wrote:
Joanne Koong <joannekoong@xxxxxx> writes:
Currently, bpf_sockops programs have been using bpf_load_hdr_opt() to
parse the tcp header option. It will be useful to allow other bpf prog
types to have a similar way of handling tcp hdr options.
This series adds XDP support for bpf_load_hdr_opt(). At a high level,
these patches are:
Why is this needed? Why not just parse the header directly in XDP?
Parsing a variable number of TCP options is challenging for the verifier.
Some programs are using #pragma unroll as a temporary workaround
(https://github.com/xdp-project/bpf-examples/blob/master/pping/pping_kern.c#L95)
I believe Christian Deacon also recently posted about this on the xdp mailing list
with a link to his bpf fail logs in https://github.com/gamemann/XDP-TCP-Header-Options
which showcases some of the difficulties involved
Seems
a bit arbitrary to add a helper for this particular type of packet
payload parsing to this particular program type. I.e., what about other
headers (IP options?)?
The current use case needs so far have been for parsing tcp headers, but
in the future, when there are needs for parsing other types, they
can be supported as well through bpf_load_hdr_opt.
Are we going to have a whole bunch of
special-purpose parsing helpers to pick out protocol data from packets?
I think bpf_load_hdr_opt is generic enough to support parsing
any kind of protocol data (as specified through flags) in the packets
I tend to agree with Toke here that this is not generic. What has been tried
to improve the verifier instead before submitting the series? It would be much
more preferable to improve the developer experience with regards to a generic
solution, so that other/similar problems can be tackled in one go as well such
as IP options, extension headers, etc.
Also, why only enable this for XDP (and not, say the TC hook as well)?
The plan is to also support this in tc as well (this will be in a separate
patchset)
