socket module add SO_ATTACH_REUSEPORT_CPBF for Linux. #8062
Conversation
devnexen
force-pushed
the
reuseport_cpbf_linux
branch
3 times, most recently
from
612c075 to
f78674b
Compare
ext/sockets/sockets.c
Outdated
| REGISTER_LONG_CONSTANT("SO_DETACH_FILTER", SO_DETACH_FILTER, CONST_CS | CONST_PERSISTENT); | ||
| REGISTER_LONG_CONSTANT("SO_DETACH_BPF", SO_DETACH_BPF, CONST_CS | CONST_PERSISTENT); |
There was a problem hiding this comment.
would be probably good to have a test for those.
ext/sockets/sockets.c
Outdated
| BPF_STMT((BPF_LD|BPF_W|BPF_ABS), (uint32_t)(SKF_AD_OFF+SKF_AD_CPU)), | ||
| BPF_STMT((BPF_RET|BPF_A), 0), |
There was a problem hiding this comment.
This kind support just one use case so it's sort of limited. I think it's useful but we are sort of introducing a single behavior that cannot be changed. Maybe we should use arg4 as a constant that will identify type of the program that will be used. Then if someone comes with another classic BPF program use case it could be simply added. It would also allow us to overload arg4 in the future if we ever decide to provide some sort of interface to BPF.
There was a problem hiding this comment.
Yes I planned to pick it up at some point but too late for 8.2
devnexen
force-pushed
the
reuseport_cpbf_linux
branch
from
f78674b to
7ee377f
Compare
ext/sockets/sockets.c
Outdated
| switch (k) { | ||
| case SKF_AD_CPU: | ||
| cbpf[0].code = (BPF_LD|BPF_W|BPF_ABS); | ||
| cbpf[0].k = (uint32_t)(SKF_AD_OFF+k); |
There was a problem hiding this comment.
It took me quite a bit of time to realise that this is for getting to current cpu id because I have to realise that k is actually SKF_AD_CPU. I know it is obvious as it is a switch case but it could be a bit clearer just by using SKF_AD_CPU
There was a problem hiding this comment.
also NIT that we usually write a space around +...
ext/sockets/sockets.c
Outdated
| static struct sock_fprog bpfprog; | ||
|
|
||
| switch (k) { | ||
| case SKF_AD_CPU: |
There was a problem hiding this comment.
I was thinking if this is the best name as it's a part of actual CBPF. Although it's the part that actual matters so it makes some sense. Basically I couldn't come up with anything better so guess it's fine :)
devnexen
force-pushed
the
reuseport_cpbf_linux
branch
from
aec6bba to
37cba3a
Compare
to be used in conjunction with SO_REUSPORT, giving a greater control over how we bind a socket instead of the round robin workflow, we do instead attach to the processor id as : - we assign the processor_id to A in the BPF filter. - then returns A. in other words, a more modern version of SO_INCOMING_CPU (ie can have a per worker notion we do not use here).
devnexen
force-pushed
the
reuseport_cpbf_linux
branch
from
37cba3a to
d5c1acc
Compare
|
Just checking - if I look in the UPGRADING guide, I can see the new |
|
Yeah I think those should be added as well. @devnexen are you fine to do that? |
|
no problem for me, will do today. |
From the PHP 8.3 changelog: > . SO_ATTACH_REUSEPORT_CBPF (Linux only). The other two constants are visible in the commit, but not mentioned in the changelog. I've left [a comment asking if this needs fixing up](php/php-src#8062 (comment)). Ref: * https://github.com/php/php-src/blob/655f116be57b46efe32221d7adfec6d6b81eeece/UPGRADING#L462 * php/php-src 8062 * php/php-src@615b800 Related to 1589
to be used in conjunction with SO_REUSPORT, giving a greater control
over how we bind a socket instead of the round robin workflow, we do
instead attach to the processor id as :
in other words, a more modern version of SO_INCOMING_CPU (ie can have a per
worker notion we do not use here).