Page MenuHomeFreeBSD

sysctl_setsockopt: enable search by connection tuple
Needs ReviewPublic

Authored by glebius on Jul 4 2022, 7:42 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Dec 11, 11:50 PM
Unknown Object (File)
Oct 4 2024, 7:49 AM
Unknown Object (File)
Oct 3 2024, 5:03 PM
Unknown Object (File)
Oct 2 2024, 10:26 AM
Unknown Object (File)
Oct 1 2024, 4:16 PM
Unknown Object (File)
Sep 4 2024, 4:06 AM
Unknown Object (File)
Aug 31 2024, 7:19 AM
Unknown Object (File)
Aug 7 2024, 2:17 AM
Subscribers

Details

Reviewers
tuexen
Summary

The function already had embyrionic support for that feature, as it
was selecting the hash slot if a tuple was supplied. However, the
API still required to specify inp_gencnt. The problem with gencnt
is that to obtain it from the userland you need to dump full pcb
list and in there locate the pcb of interest. With this change you
don't need to do that and can just supply tuple to be looked up.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 46261
Build 43150: arc lint + arc unit

Event Timeline

sys/netinet/in_pcb.c
2845

Wouldn't specific be true for a listening socket? Wouldn't it make more sense to set specific to true, if the both port numbers are not zero?

sys/netinet/in_pcb.c
2845

I don't understand the first question, as at this point we aren't talking about a socket, we are talking about params. For the second question, if we require both ports, then indeed we won't be able to supply a params, that would match a listening socket. Basically the next line hints that, doesn't it?

sys/netinet/in_pcb.c
2845

Sorry for not using a precise language.

The function is called for a single inp, which is specified by the inp_gencnt.
To find it as fast as possible, the local and remote addresses, if specified, are used to find the hash bucket. Up to now, this is just an optimization.

The code looks like:

if (params->sop_inc.inc_lport != htons(0)) {
    /* local address specified */
    if (params->sop_inc.inc_fport == htons(0)) {
        /* remote address not specified */
    } else {
        /* remote address specified */
    }
}

Now you want to also allow the local and remote addresses to be used to identify the inp. Isn't this only possible if both the local and remote addresses are known? If the end point is not bound at all, you can have multiple of them (just call socket() multiple times and not call bind(), connect() or listen()). The same applies for end points which are locally bound, but don't have a remote address yet. You can have, for example, multiple listening end points bound to the same local address.

If I'm not wrong, you can only specify an end point, if both addresses are known, which means in the above case that both port number are not zero.
I think the code should read:

if (params->sop_inc.inc_lport != htons(0)) {
    /* local address specified */
    if (params->sop_inc.inc_fport == htons(0)) {
        /* remote address not specified */
        specific = false;
    } else {
        /* remote address specified */
        specific = true;
    }
} else {
    specific = false;
}

or some equivalent way of expressing the above.

sys/netinet/in_pcb.c
2845

Did I get it correct, that you don't like the fact that we can match multiple sockets, e.g. multiple listeners with the same local address? I'd say it is an intentional behavior. The user specifies a matching criteria. For a connected socket that criteria must match only one, but for listening it may match multiple, which is fine.

sys/netinet/in_pcb.c
2845

I forgot that we exit the search cycle when we find a single pcb... That complicates things. Needs to think more about it.

sys/netinet/in_pcb.c
2845

The basic idea was that the userland specifies a single end-point and all kind of looping is done in the userland. That is why I used the inp_gencnt.

The reason, struct sockopt_parameters contains struct in_conninfo sop_inc is just for performance optimization that allows inp_next() to find the entry faster by using the correct hash bucket, if possible. lstewart@ was concerned about performance, therefore I added that.

Please note that people might or might not want to apply the same socket option to all listers with the same local address. But I don't think that this is true for all sockets being unbound.