Skip to content

Conversation

@GAnthony
Copy link

@GAnthony GAnthony commented Oct 8, 2018

Recently, the wifi net offload driver has been asserting
as init_iface() was checking for api->send != NULL, even in
the case of NET_OFFLOAD

This patch suggests a fix to handle the NET_OFFLOAD case.

Signed-off-by: Gil Pitney [email protected]

@codecov-io
Copy link

codecov-io commented Oct 8, 2018

Codecov Report

Merging #10441 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10441      +/-   ##
==========================================
- Coverage   53.18%   53.18%   -0.01%     
==========================================
  Files         210      210              
  Lines       25821    25825       +4     
  Branches     5684     5686       +2     
==========================================
+ Hits        13733    13735       +2     
  Misses       9781     9781              
- Partials     2307     2309       +2
Impacted Files Coverage Δ
subsys/net/ip/net_if.c 65.38% <0%> (-0.12%) ⬇️
include/net/net_if.h 60% <0%> (+1.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7b82e9f...60d03df. Read the comment docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, I guess this is a perfect case to use IS_ENABLED() to avoid repetition. Or maybe we just should define net_if_is_ip_offloaded() to return the right thing for !CONFIG_NET_OFFLOAD case and have even clearer flow.

Copy link
Member

Choose a reason for hiding this comment

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

As part of #9825 I added some offloading checks in commit 1ca71cf Would that solve this issue too?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, @jukkar, actually that would help implement @pfalcon's second suggestion, simplifying to:

if (!net_if_is_ip_offloaded(iface)) { NET_ASSERT(api->send); }

Reason I didn't do that initially was because it seemed net_core favored code size reduction via Kconfig vs readability. But, I much prefer the readability of the run-time check.

How close is #9825 from being merged? Would you prefer I grab commit 1ca71cf as part of this PR, or wait and rebase?

Copy link
Member

Choose a reason for hiding this comment

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

I just sent this as a separate PR, see #10521

Copy link
Author

Choose a reason for hiding this comment

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

Perfect, thanks!

Recently, the wifi net offload driver has been asserting
as init_iface() was checking for api->send != NULL, even in
the case of NET_OFFLOAD

This patch suggests a fix to handle the NET_OFFLOAD case.

Signed-off-by: Gil Pitney <[email protected]>
@GAnthony GAnthony force-pushed the fix_netif_send_assert branch from 045007c to 60d03df Compare October 11, 2018 17:55
@GAnthony
Copy link
Author

@jukkar, rebased on commit fd25c8b .
Thanks!

@jukkar jukkar merged commit 0089579 into zephyrproject-rtos:master Oct 12, 2018
@GAnthony GAnthony deleted the fix_netif_send_assert branch November 20, 2018 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants