Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gh-96534: socketmodule: support FreeBSD divert(4) socket #96536

Merged
merged 1 commit into from
May 4, 2023
Merged

gh-96534: socketmodule: support FreeBSD divert(4) socket #96536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions Doc/library/socket.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,17 @@ Constants
.. versionadded:: 3.9


.. data:: AF_DIVERT
PF_DIVERT

These two constants, documented in the FreeBSD divert(4) manual page, are
also defined in the socket module.

.. availability:: FreeBSD >= 14.0.

.. versionadded:: 3.12


.. data:: AF_PACKET
PF_PACKET
PACKET_*
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Library/2022-09-03-09-24-02.gh-issue-96534.EU4Oxv.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Support divert(4) added in FreeBSD 14.
13 changes: 13 additions & 0 deletions Modules/socketmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1850,6 +1850,11 @@ getsockaddrarg(PySocketSockObject *s, PyObject *args,
/* RDS sockets use sockaddr_in: fall-through */
#endif /* AF_RDS */

#ifdef AF_DIVERT
case AF_DIVERT:
/* FreeBSD divert(4) sockets use sockaddr_in: fall-through */
#endif /* AF_DIVERT */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why we don't need to check for PF_DIVERT here? Do we need a comment for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neither PF_DIVERT of AF_DIVERT are provided on old versions of FreeBSD. Both are provided on the new versions. I think this is stylistically more correct to put ifdefs as I did. For longer explanation on the difference between them, see my other comment.


case AF_INET:
{
struct maybe_idna host = {NULL, NULL};
Expand Down Expand Up @@ -7628,6 +7633,14 @@ PyInit__socket(void)
PyModule_AddIntMacro(m, AF_SYSTEM);
#endif

/* FreeBSD divert(4) */
#ifdef PF_DIVERT
PyModule_AddIntMacro(m, PF_DIVERT);
#endif
#ifdef AF_DIVERT
PyModule_AddIntMacro(m, AF_DIVERT);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to add a comment why we need to check for both PF_DIVERT and
AF_DIVERT and mention the related freebsd versions.

You explained this in the discussion but having this in the code will make it much
easier to maintain 5 years from now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The distinction between protocol families (PF_* constants) and address families (AF_* constants) is what we inherited from the original BSD sockets API. The PF_FOO is supposed to be an argument for socket(2), while AF_FOO for any socket address manipulation syscalls or functions, e.g. bind(), getsockname(), etc. This distinction makes some sense, since existence of a protocol doesn't dictate existence of address type and vice versa. The divert(4) is actually nice example, as it uses AF_INET addresses. Or look at AF_RDS a couple lines above in the socketmodule.c - the same.

However, all modern implementations of sockets always declare both AF_FOO and PF_FOO. As far as I know, they always are equal. POSIX doesn't mention PF_* families. Linux notes their existence in the NOTES of the manual page. Many software developers would use AF_FOO when calling socket(). This is why AF_DIVERT is also provided by FreeBSD and this is why I provided PyModule_AddIntMacro for it, too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, looking at the rest of the code it is expected to have both constants.

#endif

#ifdef AF_PACKET
PyModule_AddIntMacro(m, AF_PACKET);
#endif
Expand Down