Skip to content

bpo-42653: Add constants to socket module for IsoTp CAN protocol #26174

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

Closed
wants to merge 9 commits into from
Closed

bpo-42653: Add constants to socket module for IsoTp CAN protocol #26174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0cf7cd7
Add import for linux/can/isotp.h
menschel May 16, 2021
17535b5
Add IsoTp constants in socketmodule.c
menschel May 19, 2021
1f7c2c5
Add test for isotp constants into unit test.
menschel May 19, 2021
bc9118c
Add entry to Doc/whatsnew/3.10.rst
menschel May 19, 2021
523bf9f
Fix a whitespace issue in test_socket.py
menschel May 19, 2021
eb283c2
Add issue number 42653 and blurb.
menschel May 19, 2021
2f19691
Fix: Setting of HAVE_LINUX_CAN_ISOTP_H by configure
menschel May 22, 2021
5860ad0
Fix: Change order of headers in configure
menschel May 22, 2021
8132cb1
Fix: Optionally include CAN_ISOTP_SF_BROADCAST
menschel May 23, 2021
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
3 changes: 3 additions & 0 deletions Doc/whatsnew/3.10.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1175,6 +1175,9 @@ The exception :exc:`socket.timeout` is now an alias of :exc:`TimeoutError`.
Add option to create MPTCP sockets with ``IPPROTO_MPTCP``
(Contributed by Rui Cunha in :issue:`43571`.)

Add constants for IsoTp CAN protocol.
(Contributed by Patrick Menschel in :issue:`42653`.)

ssl
---

Expand Down
43 changes: 43 additions & 0 deletions Lib/test/test_socket.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2166,6 +2166,49 @@ def testCrucialConstants(self):
socket.CAN_ISOTP
socket.SOCK_DGRAM

@unittest.skipUnless(hasattr(socket, "SOL_CAN_ISOTP"),
'Constants from isotp.h required for this test.')
def testIsoTpConstants(self):
socket.SOL_CAN_ISOTP

# for socket options affecting the socket (not the global system)
socket.CAN_ISOTP_OPTS
socket.CAN_ISOTP_RECV_FC

# sockopts to force stmin timer values for protocol regression tests
socket.CAN_ISOTP_TX_STMIN
socket.CAN_ISOTP_RX_STMIN
socket.CAN_ISOTP_LL_OPTS

# flags for isotp behaviour
socket.CAN_ISOTP_LISTEN_MODE
socket.CAN_ISOTP_EXTEND_ADDR
socket.CAN_ISOTP_TX_PADDING
socket.CAN_ISOTP_RX_PADDING
socket.CAN_ISOTP_CHK_PAD_LEN
socket.CAN_ISOTP_CHK_PAD_DATA
socket.CAN_ISOTP_HALF_DUPLEX
socket.CAN_ISOTP_FORCE_TXSTMIN
socket.CAN_ISOTP_FORCE_RXSTMIN
socket.CAN_ISOTP_RX_EXT_ADDR
socket.CAN_ISOTP_WAIT_TX_DONE
# This constant is new and not always available
# socket.CAN_ISOTP_SF_BROADCAST

# default values
socket.CAN_ISOTP_DEFAULT_FLAGS
socket.CAN_ISOTP_DEFAULT_EXT_ADDRESS
socket.CAN_ISOTP_DEFAULT_PAD_CONTENT
socket.CAN_ISOTP_DEFAULT_FRAME_TXTIME
socket.CAN_ISOTP_DEFAULT_RECV_BS
socket.CAN_ISOTP_DEFAULT_EXT_ADDRESS
socket.CAN_ISOTP_DEFAULT_RECV_STMIN
socket.CAN_ISOTP_DEFAULT_RECV_WFTMAX

socket.CAN_ISOTP_DEFAULT_LL_MTU
socket.CAN_ISOTP_DEFAULT_LL_TX_DL
socket.CAN_ISOTP_DEFAULT_LL_TX_FLAGS

def testCreateSocket(self):
with socket.socket(socket.PF_CAN, socket.SOCK_RAW, socket.CAN_RAW) as s:
pass
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Core and Builtins/2021-05-19-19-41-15.bpo-42653.qvfUpA.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add constants for IsoTp CAN protocol.
42 changes: 42 additions & 0 deletions Modules/socketmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7785,6 +7785,48 @@ PyInit__socket(void)

PyModule_AddIntMacro(m, J1939_FILTER_MAX);
#endif
#ifdef HAVE_LINUX_CAN_ISOTP_H
PyModule_AddIntMacro(m, SOL_CAN_ISOTP);

/* for socket options affecting the socket (not the global system) */
PyModule_AddIntMacro(m, CAN_ISOTP_OPTS);
PyModule_AddIntMacro(m, CAN_ISOTP_RECV_FC);

/* sockopts to force stmin timer values for protocol regression tests */
PyModule_AddIntMacro(m, CAN_ISOTP_TX_STMIN);
PyModule_AddIntMacro(m, CAN_ISOTP_RX_STMIN);
PyModule_AddIntMacro(m, CAN_ISOTP_LL_OPTS);

/* flags for isotp behaviour */
PyModule_AddIntMacro(m, CAN_ISOTP_LISTEN_MODE);
PyModule_AddIntMacro(m, CAN_ISOTP_EXTEND_ADDR);
PyModule_AddIntMacro(m, CAN_ISOTP_TX_PADDING);
PyModule_AddIntMacro(m, CAN_ISOTP_RX_PADDING);
PyModule_AddIntMacro(m, CAN_ISOTP_CHK_PAD_LEN);
PyModule_AddIntMacro(m, CAN_ISOTP_CHK_PAD_DATA);
PyModule_AddIntMacro(m, CAN_ISOTP_HALF_DUPLEX);
PyModule_AddIntMacro(m, CAN_ISOTP_FORCE_TXSTMIN);
PyModule_AddIntMacro(m, CAN_ISOTP_FORCE_RXSTMIN);
PyModule_AddIntMacro(m, CAN_ISOTP_RX_EXT_ADDR);
PyModule_AddIntMacro(m, CAN_ISOTP_WAIT_TX_DONE);
#ifdef CAN_ISOTP_SF_BROADCAST
/* This constant is new and not always available */
PyModule_AddIntMacro(m, CAN_ISOTP_SF_BROADCAST);
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume the version of the kernel could be checked on buildtime, couldn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically yes, but there is no guarantee that this symbol really is there when the kernel version is above a specific version.
If you install the headers yourself with linux-headers-$(uname -r), the assumption works.

But if the installation of headers has been obscured, like with Raspberry Pi 's package raspberrypi-kernel-headers, this assumption no longer works. It just adds another assumption that the packages
raspberrypi-kernel-headers and raspberrypi-kernel are in sync.

Copy link
Contributor

@rumpelsepp rumpelsepp May 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Personally, I would argue that the mainline upstream kernel is the source of truth and this must be fixed downstream. No idea how the CPython project deals with these kinds of issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, lets wait for the Reviewer to comment on this. This should theoretically be @tiran.

Copy link
Contributor

Choose a reason for hiding this comment

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

aye.

#endif

/* default values */
PyModule_AddIntMacro(m, CAN_ISOTP_DEFAULT_FLAGS);
PyModule_AddIntMacro(m, CAN_ISOTP_DEFAULT_EXT_ADDRESS);
PyModule_AddIntMacro(m, CAN_ISOTP_DEFAULT_PAD_CONTENT);
PyModule_AddIntMacro(m, CAN_ISOTP_DEFAULT_FRAME_TXTIME);
PyModule_AddIntMacro(m, CAN_ISOTP_DEFAULT_RECV_BS);
PyModule_AddIntMacro(m, CAN_ISOTP_DEFAULT_RECV_STMIN);
PyModule_AddIntMacro(m, CAN_ISOTP_DEFAULT_RECV_WFTMAX);

PyModule_AddIntMacro(m, CAN_ISOTP_DEFAULT_LL_MTU);
PyModule_AddIntMacro(m, CAN_ISOTP_DEFAULT_LL_TX_DL);
PyModule_AddIntMacro(m, CAN_ISOTP_DEFAULT_LL_TX_FLAGS);
#endif
#ifdef SOL_RDS
PyModule_AddIntMacro(m, SOL_RDS);
#endif
Expand Down
4 changes: 4 additions & 0 deletions Modules/socketmodule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@ typedef int socklen_t;
#include <linux/can/bcm.h>
#endif

#ifdef HAVE_LINUX_CAN_ISOTP_H
#include <linux/can/isotp.h>
#endif

#ifdef HAVE_LINUX_CAN_J1939_H
#include <linux/can/j1939.h>
#endif
Expand Down
4 changes: 2 additions & 2 deletions configure
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8405,8 +8405,8 @@ fi
done


# On Linux, can.h, can/bcm.h, can/j1939.h, can/raw.h require sys/socket.h
for ac_header in linux/can.h linux/can/bcm.h linux/can/j1939.h linux/can/raw.h
# On Linux, can.h, can/bcm.h, can/isotp.h, can/j1939.h, can/raw.h require sys/socket.h
for ac_header in linux/can.h linux/can/bcm.h linux/can/isotp.h linux/can/j1939.h linux/can/raw.h
do :
as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh`
ac_fn_c_check_header_compile "$LINENO" "$ac_header" "$as_ac_Header" "
Expand Down
4 changes: 2 additions & 2 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2269,8 +2269,8 @@ AC_CHECK_HEADERS(linux/vm_sockets.h,,,[
#endif
])

# On Linux, can.h, can/bcm.h, can/j1939.h, can/raw.h require sys/socket.h
AC_CHECK_HEADERS(linux/can.h linux/can/bcm.h linux/can/j1939.h linux/can/raw.h,,,[
# On Linux, can.h, can/bcm.h, can/isotp.h, can/j1939.h, can/raw.h require sys/socket.h
AC_CHECK_HEADERS(linux/can.h linux/can/bcm.h linux/can/isotp.h linux/can/j1939.h linux/can/raw.h,,,[
#ifdef HAVE_SYS_SOCKET_H
#include <sys/socket.h>
#endif
Expand Down
3 changes: 3 additions & 0 deletions pyconfig.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,9 @@
/* Define to 1 if you have the <linux/can/j1939.h> header file. */
#undef HAVE_LINUX_CAN_J1939_H

/* Define to 1 if you have the <linux/can/isotp.h> header file. */
#undef HAVE_LINUX_CAN_ISOTP_H

/* Define if compiling using Linux 3.6 or later. */
#undef HAVE_LINUX_CAN_RAW_FD_FRAMES

Expand Down