bridge: Log MAC address port flapping
ClosedPublic
Actions

Authored by zlei on Mar 31 2023, 6:08 PM.

Details

Reviewers

kp
melifaro

Group Reviewers

network

Commits

rG303d9a593bdd: bridge: Log MAC address port flapping
rGde53ebb0f20e: bridge: Log MAC address port flapping
rG2d3614fb132b: bridge: Log MAC address port flapping

Summary

MAC flapping occurs when a bridge receives packets from different member member interfaces. The common reasons are:

user roams from one bridge port to another
user has wrong network setup, bridge loops e.g.
someone set duplicated ethernet address on his/her nic
some bad guy / virus / trojan send spoofed packets

The current implementation updates bridge routing entry silently hence it is hard to diagnose when any of them happen.

Emit logs when MAC address port flapping occurs so that it will be easy to diagnose.

MFC after: 2 weeks

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

zlei created this revision.Mar 31 2023, 6:08 PM

Herald added subscribers: glebius, ae, imp. · View Herald TranscriptMar 31 2023, 6:08 PM

zlei requested review of this revision.Mar 31 2023, 6:08 PM

zlei added inline comments.

sys/net/if_bridge.c
2868	TODO: need rate limit ?? In case of DoS, system will have huge amount of logs.

kp added inline comments.Apr 1 2023, 12:05 AM

sys/net/if_bridge.c
2868	The usual approach is to use ppsratecheck(9).

Overall that seems like a good idea.

Perhaps phrase it as "bridge0: MAC address A vlan 42 moved from em0 to em2"?

In D39375#896269, @kp wrote:

Overall that seems like a good idea.

Perhaps phrase it as "bridge0: MAC address A vlan 42 moved from em0 to em2"?

Agreed.
"flap from .. to .." sounds bad grammar. But only "is flapping" is less useful.

sys/net/if_bridge.c
2868	I'll try ppsratecheck(9) and test.

Added ppsratecheck() to suppress logs

kp accepted this revision as: kp.Apr 4 2023, 11:00 AM

kp added inline comments.

sys/net/if_bridge.c
470	They're static, so this is probably fine.

This revision is now accepted and ready to land.Apr 4 2023, 11:00 AM

zlei added inline comments.Apr 4 2023, 4:44 PM

sys/net/if_bridge.c
2875	I'll re-check `obif`. It is not lock protected when referenced by `log()`.

kp added inline comments.Apr 4 2023, 11:55 PM

sys/net/if_bridge.c
2875	It's safe to dereference `obif` here. It's managed by epoch, so will remain valid while we're in NET_EPOCH. It will only get deleted by `bridge_rtnode_destroy_cb()` once it can no longer be in use.

zlei marked 2 inline comments as done.Apr 5 2023, 2:33 PM

zlei added inline comments.

sys/net/if_bridge.c
470	That's OK.
2875	I checked all callers, `bridge_rtupdate()` is indeed in `NET_EPOCH`. The assert `BRIDGE_LOCK_OR_NET_EPOCH_ASSERT` in `bridge_rtupdate()` is misleading.

zlei marked an inline comment as done.Apr 5 2023, 2:54 PM

zlei added inline comments.Apr 5 2023, 3:01 PM

sys/net/if_bridge.c
464	`log_mac_flap` is actually `boolean`.

zlei added inline comments.Apr 5 2023, 3:09 PM

sys/net/if_bridge.c
467	No need for `CTLFLAG_TUN` .

zlei edited the summary of this revision. (Show Details)Apr 7 2023, 3:26 AM

I updated the SUMMARY to have a better explaining.

Do you have any suggestions ? @kp

In D39375#898104, @zlei wrote:

Do you have any suggestions ? @kp

I might rephrase "MAC flapping occurs when a bridge receives packets from different member member interfaces. " as "MAC flapping occurs when a bridge receives packets with the same source MAC address on different member member interfaces.", but it's ready to land as-is too.

In D39375#898123, @kp wrote:

In D39375#898104, @zlei wrote:

Do you have any suggestions ? @kp

I might rephrase "MAC flapping occurs when a bridge receives packets from different member member interfaces. " as "MAC flapping occurs when a bridge receives packets with the same source MAC address on different member member interfaces.", but it's ready to land as-is too.