OrgZephyr/zephyr
OrgZephyr
/
zephyr
mirror of https://github.com/zephyrproject-rtos/zephyr.git
1
0
Fork 0
Code Issues Releases Wiki Activity

net: l2: ethernet: fix k_work API usage in carrier on/off handling 

net_eth_carrier_on() and net_eth_carrier_off() call k_work_init() on
work item that can be pending or still be processed in another thread.
This results in undefined behavior.

Initialize work item once and use an atomic flag to switch between
up/down carrier state. Submit work to workqueue whenever up/down carrier
state changes, so that last state is always properly propagated to
network interface layer.

While at it, save network interface pointer during ethernet context
initialization, so that is becomes static (and thread-safe) during whole
ethernet context lifetime.

Signed-off-by: Marcin Niestroj <m.niestroj@grinn-global.com>
This commit is contained in:
Marcin Niestroj 2021-03-16 17:18:54 +01:00 committed by Jukka Rissanen
parent e3a41194f7
commit c1f7b9f45a
2 changed files with 51 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
include/net/ethernet.h
View File

@ -347,8 +347,17 @@ struct ethernet_lldp {
};
#endif /* CONFIG_NET_LLDP */
enum ethernet_flags {
ETH_CARRIER_UP,
};
/** Ethernet L2 context that is needed for VLAN */
struct ethernet_context {
/** Flags representing ethernet state, which are accessed from multiple
* threads.
*/
atomic_t flags;
#if defined(CONFIG_NET_VLAN)
struct ethernet_vlan vlan[NET_VLAN_MAX_COUNT];
@ -360,19 +369,16 @@ struct ethernet_context {
ATOMIC_DEFINE(interfaces, NET_VLAN_MAX_COUNT);
#endif
struct {
/** Carrier ON/OFF handler worker. This is used to create
* network interface UP/DOWN event when ethernet L2 driver
* notices carrier ON/OFF situation. We must not create another
* network management event from inside management handler thus
* we use worker thread to trigger the UP/DOWN event.
*/
struct k_work work;
/** Carrier ON/OFF handler worker. This is used to create
* network interface UP/DOWN event when ethernet L2 driver
* notices carrier ON/OFF situation. We must not create another
* network management event from inside management handler thus
* we use worker thread to trigger the UP/DOWN event.
*/
struct k_work carrier_work;
/** Network interface that is detecting carrier ON/OFF event.
*/
struct net_if *iface;
} carrier_mgmt;
/** Network interface. */
struct net_if *iface;
#if defined(CONFIG_NET_LLDP)
struct ethernet_lldp lldp[NET_VLAN_MAX_COUNT];
@ -415,8 +421,11 @@ struct ethernet_context {
int8_t vlan_enabled;
#endif
/** Is network carrier up */
bool is_net_carrier_up : 1;
/** Is this context already initialized */
bool is_init;
bool is_init : 1;
};
/**

59
subsys/net/l2/ethernet/ethernet.c
View File

@ -1002,55 +1002,52 @@ int net_eth_vlan_disable(struct net_if *iface, uint16_t tag)
NET_L2_INIT(ETHERNET_L2, ethernet_recv, ethernet_send, ethernet_enable,
ethernet_flags);
static void carrier_on(struct k_work *work)
static void carrier_on_off(struct k_work *work)
{
struct ethernet_context *ctx = CONTAINER_OF(work,
struct ethernet_context,
carrier_mgmt.work);
struct ethernet_context *ctx = CONTAINER_OF(work, struct ethernet_context,
carrier_work);
bool eth_carrier_up;
NET_DBG("Carrier ON for interface %p", ctx->carrier_mgmt.iface);
if (ctx->iface == NULL) {
return;
}
ethernet_mgmt_raise_carrier_on_event(ctx->carrier_mgmt.iface);
eth_carrier_up = atomic_test_bit(&ctx->flags, ETH_CARRIER_UP);
net_if_up(ctx->carrier_mgmt.iface);
}
if (eth_carrier_up == ctx->is_net_carrier_up) {
return;
}
static void carrier_off(struct k_work *work)
{
struct ethernet_context *ctx = CONTAINER_OF(work,
struct ethernet_context,
carrier_mgmt.work);
ctx->is_net_carrier_up = eth_carrier_up;
NET_DBG("Carrier OFF for interface %p", ctx->carrier_mgmt.iface);
NET_DBG("Carrier %s for interface %p", eth_carrier_up ? "ON" : "OFF",
ctx->iface);
ethernet_mgmt_raise_carrier_off_event(ctx->carrier_mgmt.iface);
net_if_carrier_down(ctx->carrier_mgmt.iface);
}
static void handle_carrier(struct ethernet_context *ctx,
struct net_if *iface,
k_work_handler_t handler)
{
k_work_init(&ctx->carrier_mgmt.work, handler);
ctx->carrier_mgmt.iface = iface;
k_work_submit(&ctx->carrier_mgmt.work);
if (eth_carrier_up) {
ethernet_mgmt_raise_carrier_on_event(ctx->iface);
net_if_up(ctx->iface);
} else {
ethernet_mgmt_raise_carrier_off_event(ctx->iface);
net_if_carrier_down(ctx->iface);
}
}
void net_eth_carrier_on(struct net_if *iface)
{
struct ethernet_context *ctx = net_if_l2_data(iface);
handle_carrier(ctx, iface, carrier_on);
if (!atomic_test_and_set_bit(&ctx->flags, ETH_CARRIER_UP)) {
k_work_submit(&ctx->carrier_work);
}
}
void net_eth_carrier_off(struct net_if *iface)
{
struct ethernet_context *ctx = net_if_l2_data(iface);
handle_carrier(ctx, iface, carrier_off);
if (atomic_test_and_clear_bit(&ctx->flags, ETH_CARRIER_UP)) {
k_work_submit(&ctx->carrier_work);
}
}
#if defined(CONFIG_PTP_CLOCK)
@ -1149,6 +1146,8 @@ void ethernet_init(struct net_if *iface)
NET_DBG("Initializing Ethernet L2 %p for iface %p", ctx, iface);
ctx->ethernet_l2_flags = NET_L2_MULTICAST;
ctx->iface = iface;
k_work_init(&ctx->carrier_work, carrier_on_off);
if (net_eth_get_hw_capabilities(iface) & ETHERNET_PROMISC_MODE) {
ctx->ethernet_l2_flags |= NET_L2_PROMISC_MODE;