Skip to content

sys/net/link_layer/ieee802154/submac: move ack transmission #21973

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

Open
Lukas-Luger wants to merge 11 commits into
base: master
Choose a base branch
from
+151 −113
Open

sys/net/link_layer/ieee802154/submac: move ack transmission #21973

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
9db609a
drivers/netdev_ieee802154_submac: remove softack
Lukas-Luger Jan 5, 2026
55d81be
sys/net/link_layer/ieee802154/submac: add tx ack logic
Lukas-Luger Jan 5, 2026
a07216b
fixup! sys/net/link_layer/ieee802154/submac: add tx ack logic
Lukas-Luger Jan 6, 2026
ec68e3a
fixup! sys/net/link_layer/ieee802154/submac: add tx ack logic
Lukas-Luger Jan 9, 2026
4a88e62
fixup! sys/net/link_layer/ieee802154/submac: add tx ack logic
Lukas-Luger Jan 9, 2026
87b185e
fixup! sys/net/link_layer/ieee802154/submac: add tx ack logic
Lukas-Luger Jan 9, 2026
39b35fa
fixup! sys/net/link_layer/ieee802154/submac: add tx ack logic
Lukas-Luger Jan 15, 2026
b3bdb3b
fixup! drivers/netdev_ieee802154_submac: remove softack
Lukas-Luger Jan 22, 2026
e0a3316
fixup! sys/net/link_layer/ieee802154/submac: add tx ack logic
Lukas-Luger Jan 22, 2026
8c6dc34
fixup! sys/net/link_layer/ieee802154/submac: add tx ack logic
Lukas-Luger Jan 22, 2026
22683b3
fixup! sys/net/link_layer/ieee802154/submac: add tx ack logic
Lukas-Luger Jan 22, 2026
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
1 change: 0 additions & 1 deletion cpu/native/Makefile.dep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ ifneq (,$(filter socket_zep,$(USEMODULE)))
USEMODULE += ieee802154
ifneq (,$(filter netdev,$(USEMODULE)))
USEMODULE += netdev_ieee802154_submac
USEMODULE += netdev_ieee802154_submac_soft_ack
endif
endif

Expand Down
1 change: 0 additions & 1 deletion cpu/nrf52/Makefile.dep
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ ifneq (,$(filter nrf802154,$(USEMODULE)))
USEMODULE += luid
ifneq (,$(filter netdev,$(USEMODULE)))
USEMODULE += netdev_ieee802154_submac
USEMODULE += netdev_ieee802154_submac_soft_ack
endif
endif

Expand Down
4 changes: 4 additions & 0 deletions drivers/netdev_ieee802154_submac/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,5 @@
ifneq (,$(filter netdev_ieee802154_submac_soft_ack,$(USEMODULE)))
$(warning SubMAC Soft ACK is now on by default and will be deprecated.)
endif

include $(RIOTBASE)/Makefile.base
27 changes: 2 additions & 25 deletions drivers/netdev_ieee802154_submac/netdev_ieee802154_submac.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ static int _recv(netdev_t *netdev, void *buf, size_t len, void *info)
netdev_ieee802154_submac_t,
dev);
ieee802154_submac_t *submac = &netdev_submac->submac;
ieee802154_rx_info_t rx_info;
ieee802154_rx_info_t rx_info = { 0 };

if (buf == NULL && len == 0) {
return ieee802154_get_frame_length(submac);
Expand All @@ -303,30 +303,7 @@ static int _recv(netdev_t *netdev, void *buf, size_t len, void *info)

netdev_rx_info->lqi = rx_info.lqi;
}
#if IS_USED(MODULE_NETDEV_IEEE802154_SUBMAC_SOFT_ACK)
const uint8_t *mhr = buf;
if ((mhr[0] & IEEE802154_FCF_TYPE_MASK) == IEEE802154_FCF_TYPE_DATA &&
(mhr[0] & IEEE802154_FCF_ACK_REQ)) {
ieee802154_filter_mode_t mode;
if (!ieee802154_radio_has_capability(&submac->dev, IEEE802154_CAP_AUTO_ACK) &&
(ieee802154_radio_get_frame_filter_mode(&submac->dev, &mode) < 0
|| mode == IEEE802154_FILTER_ACCEPT)) {
/* send ACK if not handled by the driver and not in promiscuous mode */
uint8_t ack[IEEE802154_ACK_FRAME_LEN - IEEE802154_FCS_LEN]
= { IEEE802154_FCF_TYPE_ACK, 0x00, ieee802154_get_seq(mhr) };
iolist_t io = {
.iol_base = ack,
.iol_len = sizeof(ack),
.iol_next = NULL
};
DEBUG("IEEE802154 submac: Sending ACK\n");
int snd = _send(netdev, &io);
if (snd < 0) {
DEBUG("IEEE802154 submac: failed to send ACK (%d)\n", snd);
}
}
}
#endif

return res;
}

Expand Down
1 change: 1 addition & 0 deletions makefiles/deprecated_modules.inc.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
# Keep this list ALPHABETICALLY SORTED!!!!111elven
DEPRECATED_MODULES += gnrc_nettype_lorawan
DEPRECATED_MODULES += sema_deprecated
DEPRECATED_MODULES += netdev_ieee802154_submac_soft_ack
87 changes: 51 additions & 36 deletions sys/include/net/ieee802154/submac.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,30 +30,36 @@
* | RX | |PREPARE |<--->| TX |
* | | +--->| | | |
* +--------+ | +--------+ +--------+
* ^ | ^ |
* | | | |
* | | | |
* | | +--------+ |
* | | | | v
* | | |WAIT FOR|<--------+
* | | | ACK | |
* | | +--------+ |
* | | | |
* | | | |
* | | v |
* | | +--------+ |
* | +-----| | |
* | | IDLE | |
* +------------->| |<-------+
* +--------+
* | ^ | ^ |
* | | | | |
* | | | | |
* | | | +--------+ |
* | | | | | v
* | | | |WAIT FOR|<--------+
* | | | | ACK | |
* | | | +--------+ |
* | | | | |
* | | | | |
* | | | v |
* | | | +--------+ |
* | | +-----| | |
* | | | IDLE | |
* | +----------->| |<-------+
* | +--------+
* | +--------+ ^
* | | | |
* +--->| TX ACK |-----+
* | |
* +--------+
* ```
*
* - IDLE: The transceiver is off and therefore cannot receive frames. Sending
* frames might be triggered using @ref ieee802154_send. The next SubMAC
* state would be PREPARE.
* - RX: The device is ready to receive frames. In case the SubMAC receives a
* frame it will call @ref ieee802154_submac_cb_t::rx_done and immediately go
* to IDLE. Same as the IDLE state, it's possible
* frame it will transmit an ACK frame if necessary then call
* @ref ieee802154_submac_cb_t::rx_done and immediately go
* to IDLE or TX ACK. Same as the IDLE state, it's possible
* to trigger frames using @ref ieee802154_send.
* - PREPARE: The frame is already in the framebuffer and waiting to be
* transmitted. This state might handle CSMA-CA backoff timer in case the
Expand All @@ -71,22 +77,25 @@
* (either triggered by the radio or a timer), the SubMAC goes to either
* IDLE if there are no more retransmissions left or no more CSMA-CA
* retries or PREPARE otherwise.
* - TX ACK: The received frame requires instantanous acknowledgement. Sending
* further frames in this state is not permitted. After ACK transmission is
* completed, the SubMAC will go IDLE.
*
* The events that trigger state machine changes are defined in
* @ref ieee802154_fsm_state_t
*
* The following events are valid for each state:
*
* Event/State | RX | IDLE | PREPARE | TX | WAIT FOR ACK
* --------------|----|-------|---------|----|-------------
* TX_DONE | - | - | - | X | -
* RX_DONE | X | X* | X* | X* | X
* CRC_ERROR | X | X* | X* | X* | X
* ACK_TIMEOUT | - | - | - | - | X
* BH | - | - | X | - | -
* REQ_TX | X | X | - | - | -
* REQ_SET_RX_ON | - | X | - | - | -
* REQ_SET_IDLE | X | - | - | - | -
* Event/State | RX | IDLE | PREPARE | TX | WAIT FOR ACK | TX ACK |
* --------------|----|-------|---------|----|------------------------
* TX_DONE | - | - | - | X | - |X
* RX_DONE | X | X* | X* | X* | X |-
* CRC_ERROR | X | X* | X* | X* | X |-
* ACK_TIMEOUT | - | - | - | - | X |-
* BH | - | - | X | - | - |-
* REQ_TX | X | X | - | - | - |-
* REQ_SET_RX_ON | - | X | - | - | - |-
* REQ_SET_IDLE | X | - | - | - | - |-
*
* *: RX_DONE and CRC_ERROR during these events might be a race condition
* between the ACK Timer and the radios RX_DONE event. If this happens, the
Expand Down Expand Up @@ -167,6 +176,7 @@ typedef enum {
IEEE802154_FSM_STATE_PREPARE, /**< The SubMAC is preparing the next transmission */
IEEE802154_FSM_STATE_TX, /**< The SubMAC is currently transmitting a frame */
IEEE802154_FSM_STATE_WAIT_FOR_ACK, /**< The SubMAC is waiting for an ACK frame */
IEEE802154_FSM_STATE_TX_ACK, /**< The SubMAC is transmitting an ACK frame */
IEEE802154_FSM_STATE_NUMOF, /**< Number of SubMAC FSM states */
} ieee802154_fsm_state_t;

Expand Down Expand Up @@ -208,6 +218,9 @@ struct ieee802154_submac {
ieee802154_fsm_state_t fsm_state; /**< State of the SubMAC */
ieee802154_phy_mode_t phy_mode; /**< IEEE 802.15.4 PHY mode */
const iolist_t *psdu; /**< stores the current PSDU */
uint8_t rx_buf[IEEE802154_FRAME_LEN_MAX]; /**< stores received frame */
size_t rx_len; /**< stores length of received frame */
ieee802154_rx_info_t rx_info; /**< stores lqi and rssi of received frame */
};

/**
Expand Down Expand Up @@ -410,26 +423,20 @@ static inline int ieee802154_set_tx_power(ieee802154_submac_t *submac,
/**
* @brief Get the received frame length
*
* @pre this function MUST be called either inside @ref ieee802154_submac_cb_t::rx_done
* or in SLEEP state.
Comment on lines -413 to -414
Copy link
Contributor

@mguetschow mguetschow Jan 16, 2026

Choose a reason for hiding this comment

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

Are you sure you can just remove this precondition? What is the initial value of it before receiving any frame? Isn't the buffer cleared at any point in time? (Should it maybe?)

Also, does it make sense to retrieve a frame length in any other state than RX and SLEEP?

(same below, obviously)

Copy link
Contributor Author

@Lukas-Luger Lukas-Luger Jan 22, 2026

Choose a reason for hiding this comment

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

The preconditions were present because these functions accessed the radio buffer directly. Now that we have a separate rx-buffer we can access it anytime. The initial values are set to zero at ieee802154_submac_init(). The buffer only gets overwritten, as soon as a new frame gets received.
We could restrict access in certain states and clear it after the first read, but this would create unnecessary overhead in my opinion.

*
* @param[in] submac pointer to the SubMAC
*
* @return length of the PSDU (excluding FCS length)
*/
static inline int ieee802154_get_frame_length(ieee802154_submac_t *submac)
{
return ieee802154_radio_len(&submac->dev);
return submac->rx_len;
}

/**
* @brief Read the received frame
*
* This functions reads the received PSDU from the device (excluding FCS)
*
* @pre this function MUST be called either inside @ref ieee802154_submac_cb_t::rx_done
* or in SLEEP state.
Copy link
Contributor

@Stopkaa Stopkaa Jan 22, 2026

Choose a reason for hiding this comment

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

I don't think it's a good idea to remove the precondition here, because the
submac isn't thread-safe. Removing the precondition could lead to reading the
frame while the RX interrupt is simultaneously overwriting the buffer, which
creates a race condition

*
* @param[in] submac pointer to the SubMAC descriptor
* @param[out] buf buffer to write into. If NULL, the packet is discarded
* @param[in] len length of the buffer
Expand All @@ -441,7 +448,15 @@ static inline int ieee802154_get_frame_length(ieee802154_submac_t *submac)
static inline int ieee802154_read_frame(ieee802154_submac_t *submac, void *buf,
size_t len, ieee802154_rx_info_t *info)
{
return ieee802154_radio_read(&submac->dev, buf, len, info);
if (submac->rx_len > len) {
return -ENOBUFS;
}
if (info != NULL) {
info->rssi = submac->rx_info.rssi;
info->lqi = submac->rx_info.lqi;
}
memcpy(buf, submac->rx_buf, submac->rx_len);
Copy link
Contributor

@Stopkaa Stopkaa Jan 22, 2026

Choose a reason for hiding this comment

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

If a caller passes buf = NULL, this will likely result in a segmentation fault

return submac->rx_len;
}

/**
Expand Down
Loading
Loading