cpu/stm32_common/periph: Fix addr filtering #13383

JannesVolkens · 2020-02-14T12:13:08Z

Contribution description

This PR fixes the eth reception of stm32_common. So far, every frame was being received, regardless of the destination MAC. To accomplish this the MACFFR was set to unicast filtering and the byte-order of the own MAC was corrected.

This PR was tested using the nucleo-f207zg and the example under examples/default

Testing procedure

None.

Issues/PRs references

None.

kb2ma · 2020-02-14T13:44:41Z

@benpicco, am I missing something? I don't have the background to make a meaningful contribution here.

benpicco · 2020-02-14T14:19:45Z

@kb2ma sorry I thought you were using ethernet on stm32 recently - @wosym might be interested then.

wosym · 2020-02-14T14:41:46Z

@benpicco : I was working with eth on stm32 a while ago, true. Still have the hardware, so I can test it.
However, in order to do so, I need to know what exactly has to be tested. @JannesVolkens could you elaborate a bit more on what was wrong before, and how you fixed it? And how can we test it?

JannesVolkens · 2020-02-14T20:40:59Z

@wosym I am currently using a Nucleo-f207zg and noticed that every eth frame was passed on to the application regardless of the destination MAC. So the device always behaved as if it were in promiscuous mode. At first I thought that this behavior occurs only in my application, but the same thing happened when running the example under examples/default, which allows you to send a simple string directly over the link layer using unicast or broadcast.
I checked the driver and found that the MACFFR (Ethernet MAC frame filter register) was set to pass all received frames to the application and not perfect address filtering. Also the setter function for setting the devices own MAC address was setting the address in the wrong byte-order.
So I changed the MACFFR to perfect filtering and the byte-order of the address. I tested this with the example under examples/default and was now able to receive eth frames only with the correct destination MAC.

I hope this helps.

wosym · 2020-02-18T10:31:10Z

@JannesVolkens: I tested both the master branch and this PR today, but could not reproduce the issue you describe. master and this PR both showed the same behavior.

Testing procedure used:

Connect nucleo-f207zg directly to ethernet port on laptop
open wireshark on ethernet iface
send raw ethernet frames

sendp(Ether(dst="52:10:4B:1B:03:45"), iface="enp0s31f6")
sendp(Ether(dst="52:10:4B:1B:03:46"), iface="enp0s31f6")
sendp(Ether(dst="12:34:56:78:9a:bc"), iface="enp0s31f6")

Wireshark shows all three. However, the stm only reacts when sending the first one (which has the correct MAC-address for my board).

Did I misunderstand the issue you're trying to fix? Or does it only occur under specific circumstances?

JannesVolkens · 2020-02-18T12:56:53Z

@wosym Thank you for taking a look at it.
I forgot to mention, that I connected two nucleo-f207zg boards directly to each other. For this case I get a different behavior than you describe. I’ve tried sending eth frames from one device to the other using different MAC-addresses:

36:10:5F:1B:0D:45 (Which is the correct destination MAC-address for this board)
36:10:5F:1B:0D:46
12:34:56:78:9A:BC

In case of the master every frame gets passed on to the application, where this PR only passes on the first received frame with the correct MAC-address.

Do you happen to have the hardware to check this case?

wosym · 2020-02-18T13:00:51Z

@JannesVolkens : Sadly, I only have one stm32 board with Ethernet at hand.
But I'm curious why it would make a difference who is sending the frame? What is different in the Ethernet frame?

JannesVolkens · 2020-02-20T10:24:48Z

@wosym I rebased on the current master and tested once again. I tried to reproduce your setup and connected my stm32 board directly to my laptop from where I sent raw ethernet frames to it. I’m still getting the same behavior as last time, meaning that every frame is being acted on by the application, regardless of the destination MAC-address. Whereas I get the correct behavior with this PR. Even a different board (Nucleo-f767zi) gave me the same results.

wosym · 2020-02-20T10:27:02Z

@JannesVolkens : that's strange. Maybe someone else can try to reproduce the behavior?
There might be something about my setup that automatically filters the mac-addresses, or something in your setup that causes the filtering to fail.

benpicco · 2020-02-20T10:32:24Z

@wosym but your board still receives packets with this patch and the mac address displayed in ifconfig matches with the destination mac of your packets?

wosym · 2020-02-20T10:33:27Z

@benpicco : Yes, this patch does not break anything for me (as far as I could tell at least).
The only thing I couldn't do, was reproduce the issue this PR is supposed to solve.

PeterKietzmann · 2020-02-20T13:36:35Z

I've tested this with a nucleo-f767z1 and confirm this branch fixes the described bug. The board is directly connected to my laptop. With scapy I send two ethernet frames consecutively

    sendp(Ether(dst="2A:10:2F:1B:03:45"), iface="< if >")
    sendp(Ether(dst="56:13:3C:10:16:42"), iface="< if >")

Board output on master:


ifconfig
Iface  4  HWaddr: 56:13:3C:10:16:42 
          L2-PDU:1500 Source address length: 6
          
 PKTDUMP: data received:
~~ SNIP  0 - size: 213 byte, type: NETTYPE_UNDEF (0)
00000000  60  00  4B  C7  00  AD  11  FF  FE  80  00  00  00  00  00  00
00000010  DA  35  4D  BF  2A  96  AA  5A  FF  02  00  00  00  00  00  00
00000020  00  00  00  00  00  00  00  FB  14  E9  14  E9  00  AD  70  EB
00000030  00  00  00  00  00  09  00  01  00  00  00  00  05  5F  69  70
00000040  70  73  04  5F  74  63  70  05  6C  6F  63  61  6C  00  00  0C
00000050  00  01  04  5F  66  74  70  C0  12  00  0C  00  01  07  5F  77
00000060  65  62  64  61  76  C0  12  00  0C  00  01  08  5F  77  65  62
00000070  64  61  76  73  C0  12  00  0C  00  01  09  5F  73  66  74  70
00000080  2D  73  73  68  C0  12  00  0C  00  01  04  5F  73  6D  62  C0
00000090  12  00  0C  00  01  0B  5F  61  66  70  6F  76  65  72  74  63
000000A0  70  C0  12  00  0C  00  01  04  5F  6E  66  73  C0  12  00  0C
000000B0  00  01  04  5F  69  70  70  C0  12  00  0C  00  01  C0  5A  00
000000C0  0C  00  01  00  00  11  94  00  0C  09  4B  49  45  54  5A  4D
000000D0  41  4E  4E  C0  5A
~~ SNIP  1 - size:  20 byte, type: NETTYPE_NETIF (-1)
if_pid: 4  rssi: 0  lqi: 0
flags: 0x0
src_l2addr: 00:50:B6:8C:B5:19
dst_l2addr: 33:33:00:00:00:FB
~~ PKT    -  2 snips, total size: 233 byte
PKTDUMP: data received:
~~ SNIP  0 - size:  46 byte, type: NETTYPE_UNDEF (0)
00000000  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00
00000010  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00
00000020  00  00  00  00  00  00  00  00  00  00  00  00  00  00
~~ SNIP  1 - size:  20 byte, type: NETTYPE_NETIF (-1)
if_pid: 4  rssi: 0  lqi: 0
flags: 0x0
src_l2addr: 88:B1:11:D1:E3:EA
dst_l2addr: 2A:10:2F:1B:03:45
~~ PKT    -  2 snips, total size:  66 byte
PKTDUMP: data received:
~~ SNIP  0 - size:  46 byte, type: NETTYPE_UNDEF (0)
00000000  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00
00000010  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00
00000020  00  00  00  00  00  00  00  00  00  00  00  00  00  00
~~ SNIP  1 - size:  20 byte, type: NETTYPE_NETIF (-1)
if_pid: 4  rssi: 0  lqi: 0
flags: 0x0
src_l2addr: 88:B1:11:D1:E3:EA
dst_l2addr: 56:13:3C:10:16:42
~~ PKT    -  2 snips, total size:  66 byte

-> board receives frames that contain a different dst address.

Board output on this branch:

ifconfig
Iface  4  HWaddr: 56:13:3C:10:16:42 
          L2-PDU:1500 Source address length: 6
          
 PKTDUMP: data received:
~~ SNIP  0 - size:  46 byte, type: NETTYPE_UNDEF (0)
00000000  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00
00000010  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00
00000020  00  00  00  00  00  00  00  00  00  00  00  00  00  00
~~ SNIP  1 - size:  20 byte, type: NETTYPE_NETIF (-1)
if_pid: 4  rssi: 0  lqi: 0
flags: 0x0
src_l2addr: 88:B1:11:D1:E3:EA
dst_l2addr: 56:13:3C:10:16:42
~~ PKT    -  2 snips, total size:  66 byte

-> board only receives frames that match its dst address.

PeterKietzmann · 2020-02-20T13:37:19Z

I didn't read the manual to confirm the code change, but ACK for the fix in general

benpicco · 2020-02-20T13:38:09Z

Let's consult the data sheet, page 911:

HPF: Hash or perfect filter
When this bit is set and if the HM or HU bit is set, the address filter passes frames that
match either the perfect filtering or the hash filtering.
When this bit is cleared and if the HU or HM bit is set, only frames that match the Hash filter
are passed.

PAM: Pass all multicast
When set, this bit indicates that all received frames with a multicast destination address (first
bit in the destination address field is '1') are passed.
When reset, filtering of multicast frame depends on the HM bit.

DAIF: Destination address inverse filtering
When this bit is set, the address check block operates in inverse filtering mode for the DA
address comparison for both unicast and multicast frames.
When reset, normal filtering of frames is performed.

HM: Hash multicast
When set, MAC performs destination address filtering of received multicast frames
according to the hash table.
When reset, the MAC performs a perfect destination address filtering for multicast frames,
that is, it compares the DA field with the values programmed in DA registers

and Table 144, page 869:

From my understanding HPF is correct, but we should also set the HM bit. (as I understand it otherwise the multicast address is not considered a multicast address and just compared directly with the device address - but might be misunderstanding the data sheet at that point)
Did you also test reception of multicast frames?

JannesVolkens · 2020-02-21T09:56:27Z

I think I misinterpreted the reference manual and from my understanding now the only bit that is needed is the PAM bit. According to table 144, section unicast in the second line, no filter bit is required to receive unicast frames. In addition, according to the section multicast, only the PAM bit has to be set to receive multicast frames. I was able to confirm both and was now able to receive unicast and multicast frames. Broadcast too, of course.

benpicco · 2020-02-21T10:05:29Z

Hm but do you really want to pass all multicast frames?
If you don't set anything at all, you'd only get frames that pass the group filter, which sounds more right. (That is, if the group filter is set up correctly)

- Set MACFFR to unicast filtering - Change byte-order of the MAC

JannesVolkens · 2020-02-21T10:45:14Z

Your’re right.
Can someone test again with these changes?

PeterKietzmann

I've re-tested this PR as described above.

    sendp(Ether(dst="2A:10:2F:1B:03:45"), iface="< if >")
    sendp(Ether(dst="56:13:3C:10:16:42"), iface="< if >")
    sendp(Ether(dst="ff:ff:ff:ff:ff:ff"), iface="< if >")

Multicast frames are now completely filtered whereas broadcast and unicast frames pass. I also sent L2 packets from the Nucleo board to my laptop and src/dst addresses look as expected.

The MACFFR register stays with its Reset value: 0x0000 0000 after initialization. Reading the description of all settings this seems to be a very basic but IMO reasonable setup for RIOT currently.

However, this PR makes me wonder how we handle Ethernet multicast addresses in general but answering this question is out of scope for this PR. ACK

kfessel · 2020-02-26T12:38:00Z

somehow this broke ipv6 for me (just doing pings to to a nucleo-f767z1 runing the gcoap example)

PeterKietzmann · 2020-02-26T12:52:04Z

I give it a look. Linux -> RIOT ping? Which IPv6 address did you try to ping?

kfessel · 2020-02-26T13:09:09Z

i tryed to ping from linux to RIOT used the IPv6 address (no short form)
(for me it is fe80::2813:2fff:fe10:1542)

kfessel · 2020-02-26T13:15:54Z

Why does this patch leave MACFFR unconfigured? (started to check the documentation and it seems to me this would pass all frames (no filter at all))

PeterKietzmann · 2020-02-26T13:47:00Z

@kfessel can you elaborate on you test setup, I cannot reproduce the issue. I connected the nucleo-f767z1 directly to my laptop with examples/gcoap running on 2020.01 release as well as current master ad7c19d5842e7f9dcf80968fd1ffd993cf7f88a5. On both versions I can successfully ping the ling local IPv6 address from Linux to RIOT (ping6 <ll IPv6>%<eth if>) and vice versa. However, on the release branch it seemed as if the interface on the nucleo didn't respond initially and I had to press reset to make it work (didn't investigate further).

Why does this patch leave MACFFR unconfigured? (started to check the documentation and it seems to me like it shold pass all frames (no filter at all))

It remains with its reset value which enables basic filtering that only passes unicast addresses and broadcast. As indicated above, IMO a proper assignment of multicast next to the other addresses is currently missing. But blindly passing all frames is not reasonable at all in my opinion.

PeterKietzmann · 2020-02-26T14:18:31Z

Yet no idea why it worked between Linux<->RIOT (for me). Between two RIOT nodes we face the same issue. NDP maps to a multicast MAC address which is now blocked. So the neighbor table stays empty and IPv6 packets cannot be sent. We'll provide a fix

kfessel · 2020-02-26T14:27:20Z

i got a switch between my laptop and the board, but i am not sure what is happening, seems like in this neighbor/router sociation/advertisement some times the informations do not take the right path.

sometimes it seems to work but other times not. When i wrote my complaint the thing did not work within mutiple resets and power cycles flashing multiple times (yes i know doing the same thing multible time and expecting differnt results is kind o stupid)

then checked the history and went back before your patch and it worked (most the time (there is definetly an reset issue))

atm the master seems to work.

i am also not sure if there is some issue due to the LAN8742A (PHY) beeing connected to an RC reset which the manufacturer advises angainst.

kfessel · 2020-02-26T14:31:51Z

one moment group mac ??

i had an issue and wrote a patch it is PR #13273 (merged)

since this patch turns the byte order are you sure about that change?

luid handels addr->uint8[0] as the msb of the MAC (see
eui48_set_local(addr) and eui48_clear_group(addr))

PeterKietzmann · 2020-02-26T14:48:48Z

The address order change works fine (as reported above). @JannesVolkens will provide a quick fix regarding multicast filtering (would be great if you could test it then) and we should discuss multicast address assignment in a separate issue.

kfessel · 2020-02-26T15:05:55Z

ah ok now understand
it woked with the patch if some how the mach of the ip address was still known to my linux ( -ARP- NDP cache) if it took to long to flash then the cach lost its information and asked the network (using ethernet group cast) and this is filtert by the new (default rule of the mac Filter)

( i just did a sudo ip -6 neigh flush dev en.. and the previously working ping stoped working)

PeterKietzmann · 2020-02-26T16:08:41Z

was still known to my linux ( -ARP- NDP cache) if it took to long to flash then the cach

Yep, that must have been the reason here too. After flushing the Linux neighbor cache I couldn't ping anymore

benpicco added Area: drivers Area: Device drivers Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Feb 14, 2020

benpicco requested review from aabadie and kb2ma February 14, 2020 13:16

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 20, 2020

PeterKietzmann added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Feb 20, 2020

JannesVolkens requested review from DipSwitch, fjmolinas and vincent-d as code owners February 21, 2020 09:53

JannesVolkens force-pushed the stm32_eth_mac_filter_fix branch from 573710c to ad888c5 Compare February 21, 2020 09:54

JannesVolkens removed request for DipSwitch, vincent-d and fjmolinas February 21, 2020 09:55

cpu/stm32_common/periph: Fix addr filtering

4bb0d8b

- Set MACFFR to unicast filtering - Change byte-order of the MAC

JannesVolkens force-pushed the stm32_eth_mac_filter_fix branch from c189034 to 4bb0d8b Compare February 21, 2020 10:41

benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 21, 2020

PeterKietzmann approved these changes Feb 25, 2020

View reviewed changes

PeterKietzmann merged commit 00d4d36 into RIOT-OS:master Feb 25, 2020

JannesVolkens mentioned this pull request Feb 26, 2020

cpu/stm32_common/periph: Fix multicast filtering #13487

Merged

PeterKietzmann mentioned this pull request Feb 26, 2020

cpu/stm32/eth, board/nucleo-f767z1: ethernet initialisation fails sometimes #13490

Closed

JannesVolkens mentioned this pull request Feb 27, 2020

ethernet: Missing multicast addr assignment #13493

Open

leandrolanzieri added this to the Release 2020.04 milestone Mar 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cpu/stm32_common/periph: Fix addr filtering #13383

cpu/stm32_common/periph: Fix addr filtering #13383

JannesVolkens commented Feb 14, 2020

kb2ma commented Feb 14, 2020

benpicco commented Feb 14, 2020 •

edited

Loading

wosym commented Feb 14, 2020

JannesVolkens commented Feb 14, 2020

wosym commented Feb 18, 2020

JannesVolkens commented Feb 18, 2020

wosym commented Feb 18, 2020

JannesVolkens commented Feb 20, 2020

wosym commented Feb 20, 2020 •

edited

Loading

benpicco commented Feb 20, 2020

wosym commented Feb 20, 2020

PeterKietzmann commented Feb 20, 2020

PeterKietzmann commented Feb 20, 2020

benpicco commented Feb 20, 2020 •

edited

Loading

JannesVolkens commented Feb 21, 2020

benpicco commented Feb 21, 2020 •

edited

Loading

JannesVolkens commented Feb 21, 2020

PeterKietzmann left a comment

kfessel commented Feb 26, 2020 •

edited

Loading

PeterKietzmann commented Feb 26, 2020

kfessel commented Feb 26, 2020 •

edited

Loading

kfessel commented Feb 26, 2020 •

edited

Loading

PeterKietzmann commented Feb 26, 2020

PeterKietzmann commented Feb 26, 2020

kfessel commented Feb 26, 2020

kfessel commented Feb 26, 2020 •

edited

Loading

PeterKietzmann commented Feb 26, 2020

kfessel commented Feb 26, 2020

PeterKietzmann commented Feb 26, 2020 •

edited

Loading

cpu/stm32_common/periph: Fix addr filtering #13383

cpu/stm32_common/periph: Fix addr filtering #13383

Conversation

JannesVolkens commented Feb 14, 2020

Contribution description

Testing procedure

Issues/PRs references

kb2ma commented Feb 14, 2020

benpicco commented Feb 14, 2020 • edited Loading

wosym commented Feb 14, 2020

JannesVolkens commented Feb 14, 2020

wosym commented Feb 18, 2020

Testing procedure used:

JannesVolkens commented Feb 18, 2020

wosym commented Feb 18, 2020

JannesVolkens commented Feb 20, 2020

wosym commented Feb 20, 2020 • edited Loading

benpicco commented Feb 20, 2020

wosym commented Feb 20, 2020

PeterKietzmann commented Feb 20, 2020

PeterKietzmann commented Feb 20, 2020

benpicco commented Feb 20, 2020 • edited Loading

JannesVolkens commented Feb 21, 2020

benpicco commented Feb 21, 2020 • edited Loading

JannesVolkens commented Feb 21, 2020

PeterKietzmann left a comment

Choose a reason for hiding this comment

kfessel commented Feb 26, 2020 • edited Loading

PeterKietzmann commented Feb 26, 2020

kfessel commented Feb 26, 2020 • edited Loading

kfessel commented Feb 26, 2020 • edited Loading

PeterKietzmann commented Feb 26, 2020

PeterKietzmann commented Feb 26, 2020

kfessel commented Feb 26, 2020

kfessel commented Feb 26, 2020 • edited Loading

PeterKietzmann commented Feb 26, 2020

kfessel commented Feb 26, 2020

PeterKietzmann commented Feb 26, 2020 • edited Loading

benpicco commented Feb 14, 2020 •

edited

Loading

wosym commented Feb 20, 2020 •

edited

Loading

benpicco commented Feb 20, 2020 •

edited

Loading

benpicco commented Feb 21, 2020 •

edited

Loading

kfessel commented Feb 26, 2020 •

edited

Loading

kfessel commented Feb 26, 2020 •

edited

Loading

kfessel commented Feb 26, 2020 •

edited

Loading

kfessel commented Feb 26, 2020 •

edited

Loading

PeterKietzmann commented Feb 26, 2020 •

edited

Loading