Skip to content

Fix for UBLOX_EVK_ODIN_W2 fails in emac test. #11547

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

Jump to bottom
Merged
merged 1 commit into from
Oct 15, 2019
Merged

Fix for UBLOX_EVK_ODIN_W2 fails in emac test. #11547

merged 1 commit into from
Oct 15, 2019

Conversation

mtomczykmobica
Copy link

@mtomczykmobica mtomczykmobica commented Sep 23, 2019 •
edited
Loading

UBLOX_EVK_ODIN_W2 and MTB_UBLOX_ODIN_W2 haven't multicast implementation so emac multicast test is disabled for those tests.
Function without implementation:

  • add_multicast_group
  • remove_multicast_group
  • set_all_multicast

For boards:

  • MTB_MXCHIP_EMW3166
  • MTB_ADV_WISE_1530
  • MTB_USI_WM_BN_BM_22
    Ipv4 multicast filtering is disable. All use WISE1530 module. Looks like multicast filtring adding is correct by calling mcast_list ioctl with pattern (01:00:5e:x:x:x for ipv4 and 33:33:x:x:x:x for ipv6) but for ipv4 doesn't work and it strongly suggest bug in firmware.

Description

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[x] Test update
[ ] Breaking change

Reviewers

@SeppoTakalo

Release Notes

@SeppoTakalo
Copy link
Contributor

CC: @mikaleppanen @kjbracey-arm

How does it then receive all multicast traffic?
Sounds like it should be the requirement for EMAC driver.

Instead of skipping the test, we should report this to Ublox and ask them to implement the multicast filtering.

@SeppoTakalo
Copy link
Contributor

@ARMmbed/team-ublox Hi, are you aware of this EMAC test failure?

Can it be fixed on your EMAC driver?

@ciarmcom ciarmcom requested review from SeppoTakalo and a team September 23, 2019 11:00
@ciarmcom
Copy link
Member

@mtomczykmobica, thank you for your changes.
@SeppoTakalo @ARMmbed/mbed-os-test @ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-maintainers please review.

MarceloSalazar
MarceloSalazar reviewed Sep 30, 2019
View reviewed changes
MarceloSalazar
MarceloSalazar reviewed Sep 30, 2019
View reviewed changes
Copy link

@MarceloSalazar MarceloSalazar left a comment

Choose a reason for hiding this comment

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

Please remove code that involves specific platforms

@MarceloSalazar MarceloSalazar requested a review from a team September 30, 2019 10:53
@mtomczykmobica
Copy link
Author

@ARMmbed/team-ublox Hi, are you aware of this EMAC test failure?

Can it be fixed on your EMAC driver?

I agree should be resolve in EMAC driver. Should be created new task call: "Emac multicast filtering implementation for ublox emac driver."

Second problem is wise1530 firmware. I'm sure we correctly send ioctl command with multicast filter for ipv4, but filtering doesn't work.

@mtomczykmobica mtomczykmobica force-pushed the ONME-4366 branch 3 times, most recently from ec5f2ea to dc419b8 Compare October 3, 2019 08:26
@mtomczykmobica
Copy link
Author

Please remove code that involves specific platforms

Done

@mtomczykmobica
Copy link
Author

Please review pull request.
@SeppoTakalo
@MarceloSalazar
@ARMmbed/team-ublox
@ARMmbed/mbed-os-maintainers
@ARMmbed/mbed-os-test

@AnttiKauppila
Copy link

Please review this

0xc0170
0xc0170 requested changes Oct 14, 2019
View reviewed changes
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

Just one alignment fix requested. LGTM otherwise.

Please also rebase, there's travis fix that should be in if you update PR

TESTS/network/emac/mbed_lib.json Outdated
"name": "network-emac",
"config": {
"NO_SUPPORT_FOR_MULTICAST_FILTER": false,
"NO_SUPPORT_FOR_IPV4_MULTICAST_FILTER": false
Copy link
Contributor

Choose a reason for hiding this comment

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

can you fix alignment here?

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

MarceloSalazar
MarceloSalazar approved these changes Oct 14, 2019
View reviewed changes
Copy link

@MarceloSalazar MarceloSalazar left a comment

Choose a reason for hiding this comment

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

Looks good to me (pending minor formatting issue)

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 14, 2019

CI started

@mbed-ci
Copy link

mbed-ci commented Oct 14, 2019

Test run: SUCCESS

Summary: 5 of 5 test jobs passed
Build number : 1
Build artifacts

@0xc0170 0xc0170 merged commit befc833 into ARMmbed:master Oct 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MarceloSalazar MarceloSalazar MarceloSalazar approved these changes

@0xc0170 0xc0170 0xc0170 approved these changes

@AnttiKauppila AnttiKauppila AnttiKauppila approved these changes

@SeppoTakalo SeppoTakalo Awaiting requested review from SeppoTakalo

Assignees
No one assigned
Labels
release-version: 5.14.2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@mtomczykmobica @SeppoTakalo @ciarmcom @AnttiKauppila @0xc0170 @mbed-ci @MarceloSalazar