Skip to content
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.

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

u32 ematch for mac address is used incorrectly #1

Open
fblaese opened this issue Jan 8, 2021 · 1 comment · May be fixed by #2
Open

u32 ematch for mac address is used incorrectly #1

fblaese opened this issue Jan 8, 2021 · 1 comment · May be fixed by #2
Assignees
Labels
bug Something isn't working

Comments

@fblaese
Copy link
Contributor

fblaese commented Jan 8, 2021

The u32 ematch has the following syntax:

       u32( ALIGN VALUE MASK at [ nexthdr+ ] OFFSET )
       ALIGN := { u8 | u16 | u32 }

The third argument is used as a mask for matching the value.

The u32 ematch is used by macnocker to match against the mac adress of packets, so value has to contain (parts of) the mac address, while mask has to contain only 1-bits.

Currently however, the mac address is inserted into both VALUE and MASK, so only 1-bits of the mac address are matched correctly, 0-bits are ignored:

macnocker/tc.c

Lines 62 to 65 in d0bdfdb

snprintf(cmd, 2048, "tc filter add dev %s protocol all parent ffff: prio %d "
"basic match \"u32(u32 0x%s 0x%s at -8)\" "
"and \"u32(u16 0x%s 0x%s at -4)\" flowid :1 action pass",
g_interface, prio, mac32, mac32, mac16, mac16);

@fblaese fblaese added the bug Something isn't working label Jan 8, 2021
@fblaese fblaese self-assigned this Jan 8, 2021
@fblaese
Copy link
Contributor Author

fblaese commented Jan 8, 2021

A tc filter inserted by macnocker using the mac address "11:22:33:44:55:66" looks like this:

# tc filter show dev enp6s0 ingress
filter parent ffff: protocol all pref 10 basic chain 0 
filter parent ffff: protocol all pref 10 basic chain 0 handle 0x1 flowid :1 
  u32(11223344/11223344 at -8)
  AND u32(55660000/55660000 at -4)

	action order 1: gact action pass
	 random type none pass val 0
	 index 2 ref 1 bind 1

filter parent ffff: protocol all pref 65535 basic chain 0 
filter parent ffff: protocol all pref 65535 basic chain 0 handle 0x1 flowid :1 
  u32(00004305/0000ffff at -4)

	action order 1: gact action drop
	 random type none pass val 0
	 index 1 ref 1 bind 1

fblaese added a commit to fblaese/macnocker that referenced this issue Jan 8, 2021
Previously, the mac address was inserted into both the VALUE
and MASK arguemnt of the u32 matcher. This is of course incorrect,
because then only 1-bits of the mac address are actually matched by
the kernel.

Therefore the correct mask is used instead. The second u32 matcher
only uses a 16 bit mask, because the other 16 bits of the 32-bit field contain
the ethertype.

Fixes: FreifunkFranken#1

Signed-off-by: Fabian Bläse <[email protected]>
@fblaese fblaese linked a pull request Jan 8, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant