Page MenuHomeFreeBSD

unix/dgram: use minimal possible socket buffer for PF_UNIX/SOCK_DGRAM
ClosedPublic

Authored by glebius on May 23 2022, 9:02 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 5, 10:46 AM
Unknown Object (File)
Fri, Apr 5, 10:46 AM
Unknown Object (File)
Fri, Apr 5, 10:46 AM
Unknown Object (File)
Fri, Apr 5, 10:46 AM
Unknown Object (File)
Fri, Apr 5, 10:46 AM
Unknown Object (File)
Fri, Apr 5, 10:46 AM
Unknown Object (File)
Fri, Apr 5, 10:46 AM
Unknown Object (File)
Fri, Apr 5, 10:46 AM
Subscribers

Details

Summary

This change fully splits away PF_UNIX/SOCK_DGRAM from other socket
buffer implementations, without any behavior changes.

Generic socket implementation is reduced down to one STAILQ and very
little code.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Note: this patch is a step towards D35303.

sys/kern/uipc_usrreq.c
1328

I'd prefer an explicit KASSERT which hints at why we expect m != NULL. Or restructure the code so that m == NULL will result in a page fault.

1419

I would make each of these cases set error and then break, then after the loop:

if (m != NULL) {
    SOCK_RECVBUF_UNLOCK(so);
    SOCK_IO_RECV_UNLOCK(so);
    return (error);
}
glebius added inline comments.
sys/kern/uipc_usrreq.c
1419

Can you please look at the final code in D35303? With it the handling would be more comlex:

if (m == NULL && sb == NULL) {
        SOCK_RECVBUF_UNLOCK(so);
        SOCK_IO_RECV_UNLOCK(so);
        return (error)
}

If you agree with that, I can go forward and change.

markj added inline comments.
sys/kern/uipc_usrreq.c
1419

Hmm, maybe just ignore my suggestion then. We can do this kind of cleanup after the functional changes are done.

1435

I wish this code (and sbfree()) would check for wraparound with INVARIANTS on...

This revision is now accepted and ready to land.May 25 2022, 8:02 PM
This revision now requires review to proceed.May 31 2022, 3:52 AM

Add pru_sopoll method. Fixes select(2).

Add pru_sopoll method. Fixes select(2).

Is there an existing test case which catches this?

sys/kern/uipc_usrreq.c
1526

Does so->so_rerror need to be checked also?

1527

You also checked for SBS_CNTRCVMORE below, when POLLINIGNEOF is not set. Comparing to sopoll_generic(), the check here looks wrong.

Add pru_sopoll method. Fixes select(2).

Is there an existing test case which catches this?

I'm working on the event tests now. It seems I need to do substantial changes to this branch, cause there is also kevent and aio, who also just use bare sbavail().

There are two directions to go:

  1. make protocol specific sbavail() & sbused() or soreadable() & sowriteable() and that would probably allow to leave socket poll/kevent/aio unmodified.
  2. Make protocol specific so_poll, kevent filterops and aio ops.

I'm researching these two options. Any input much appreciated.

Abandon protocol specific select/poll for now, and instead just maintain
sb_acc + sb_ccc that generic socket code requires.

Now passes tests/kern/unix_dgram:event

markj added inline comments.
sys/kern/uipc_usrreq.c
1147

BTW, you might consider asserting on valid values for flags here. For example, I forgot that PRUS_NOTREADY cannot appear here since it's only used with stream sockets.

1271
This revision is now accepted and ready to land.Jun 6 2022, 1:29 PM
sys/kern/uipc_usrreq.c
1147

I can do that in the changeset that adds uipc_sosend_dgram().

glebius added inline comments.
sys/kern/uipc_usrreq.c
1147

Nope. Those flags are not in the family of PRUS_* flags. Those are MSG_* flags. This is pru_sosend method, not pru_send. We already return error for MSG_OOB and ignore the rest. This is what original implementation does.

This revision now requires review to proceed.Jun 7 2022, 1:12 AM
This revision was not accepted when it landed; it landed in state Needs Review.Jun 24 2022, 4:11 PM
This revision was automatically updated to reflect the committed changes.