net: vlan: Allow VLAN count to be set to 0 #84873
Conversation
zephyrbot
requested review from
decsny,
lmajewski,
maass-hamburg,
pdgendt,
rlubos,
ssharks and
tbursztyka
| /* Dummy functions if VLAN is not really used. This is only needed | ||
| * if priority tagged frames (tag 0) are supported. | ||
| */ | ||
| bool net_eth_is_vlan_enabled(struct ethernet_context *ctx, |
There was a problem hiding this comment.
What if an additional vlan-interface (count > 0) is enabled at compile time, but it is not (yet) enabled at runtime? priority-tagged frames should still be received. I expect changes to ethernet_recv are needed?
There was a problem hiding this comment.
No changes are needed here as this use case was already handled by #83734, I also just verified that it works as expected.
There was a problem hiding this comment.
I expect, we would not enter this if:
if (IS_ENABLED(CONFIG_NET_VLAN) && type == NET_ETH_PTYPE_VLAN) {
if (net_eth_is_vlan_enabled(ctx, iface) &&
!eth_is_vlan_tag_stripped(iface)) {under those circumstances, but we should?
There was a problem hiding this comment.
We enter this branch in this case so that the Ethernet header size get set properly so that the Ethernet header is removed properly. The VLAN is enabled in this scenario, it is just that all the other tags than 0 are not in use.
There was a problem hiding this comment.
I am not sure, that I understand. I mean in the case, that CONFIG_VLAN>0 but not yet net_eth_is_vlan_enabled?
Maybe it is and edge case, that is not relevant. Just wanted to point it out.
There was a problem hiding this comment.
This works because by default the tag is unspecified (we have not yet parsed the VLAN header at this point) when we enter this branch. The naming of net_eth_is_vlan_enabled() is slightly misleading, it checks in this case that the Ethernet interface has VLAN enabled bit on. It is only after a call to
net_pkt_set_vlan_tci(pkt, ntohs(hdr_vlan->vlan.tci)); the tag is set to 0.
There was a problem hiding this comment.
Thank you for the explanation.
So net_eth_is_vlan_enabled() will return true, even if it has not yet been enabled at runtime using net_eth_vlan_enable?
If so, all is clear. Thanks.
There was a problem hiding this comment.
Yes, that is the case here. Without this, the code would not work as you noticed.
|
This works as intended for |
If VLAN count is set to 0, then only priority tagged VLAN frames that have tag value 0 can be received. Also this means that VLAN interfaces are not created which can save memory if you do not need to receive any other VLAN frames than those tagged with value 0. Fixes zephyrproject-rtos#84023 Signed-off-by: Jukka Rissanen <[email protected]>
Make sure the tests work if VLAN count is set to 0. This means that only priority tagged (tag 0) VLAN frames can be received. Signed-off-by: Jukka Rissanen <[email protected]>
No point trying to initialize VLAN interfaces if the VLAN interface count is zero. Signed-off-by: Jukka Rissanen <[email protected]>
jukkar
force-pushed
the
devel/priority-tagging-with-zero-vlan
branch
from
462c6ea to
593e1e9
Compare
|
| @@ -988,7 +993,7 @@ int net_eth_get_hw_config(struct net_if *iface, enum ethernet_config_type type, | |||
| * | |||
| * @return 0 if ok, <0 if error | |||
| */ | |||
| #if defined(CONFIG_NET_VLAN) | |||
| #if defined(CONFIG_NET_VLAN) && NET_VLAN_MAX_COUNT > 0 | |||
There was a problem hiding this comment.
I wonder if this should be reduced to simply:
| #if defined(CONFIG_NET_VLAN) && NET_VLAN_MAX_COUNT > 0 | |
| #if NET_VLAN_MAX_COUNT > 0 |
There was a problem hiding this comment.
I suppose that could be done too. The values are linked together although if not careful by the developer, the NET_VLAN_MAX_COUNT could be set to a value >0 even if CONFIG_NET_VLAN is not set (yes, this is an error and would probably not happen in practice).
I kind of prefer this as it is now easy to understand what the check means, so VLAN needs to be enabled and the VLAN count > 0.
If VLAN count is set to 0, then only priority tagged VLAN frames that have tag value 0 can be received. Also this means that VLAN interfaces are not created which can save memory if you do not need to receive any other VLAN frames than those tagged with value 0.
Fixes #84023
Signed-off-by: Jukka Rissanen [email protected]