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

bugfix: parse ipv4 src/dst error #1015

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Charlie17Li
Copy link

Introduction

When I use tc-pedit module to modify the src_ipv4/dst_ipv4, I check the result but the src_ipv4/dst_ipv4 is always "0".

For example, I want to set the dst_ipv4 to “172.17.0.2”, and I check the result with tc filter show dev eth0:

# actual
action order 1: pedit action ...
   key #1 at ipv4+16 val 00000000 mask 00000000

but the result is expeted to this:

# expect, the val is "172.17.0.2" not "0"
action order 1: pedit action ...
   key #1 at ipv4+16 val ac110002 mask 00000000

@Charlie17Li Charlie17Li force-pushed the bugfix-pedit-srcip branch 2 times, most recently from b7a442d to 1a940e7 Compare September 8, 2024 18:14
@Charlie17Li
Copy link
Author

Charlie17Li commented Sep 8, 2024

I just remove the unit test, because I found that parsing the pedit action is not supported now. so when I get filter and action from FilterList, the content of pedit action is empty.

netlink/filter_linux.go

Lines 800 to 820 in b1ce50c

case nl.TCA_OPTIONS:
adata, err := nl.ParseRouteAttr(aattr.Value)
if err != nil {
return nil, err
}
for _, adatum := range adata {
switch actionType {
case "mirred":
switch adatum.Attr.Type {
case nl.TCA_MIRRED_PARMS:
mirred := *nl.DeserializeTcMirred(adatum.Value)
action.(*MirredAction).ActionAttrs = ActionAttrs{}
toAttrs(&mirred.TcGen, action.Attrs())
action.(*MirredAction).Ifindex = int(mirred.Ifindex)
action.(*MirredAction).MirredAction = MirredAct(mirred.Eaction)
case nl.TCA_MIRRED_TM:
tcTs := nl.DeserializeTcf(adatum.Value)
actionTimestamp = toTimeStamp(tcTs)
}
case "tunnel_key":

@Charlie17Li
Copy link
Author

cc @aboch @chent1996

nl/tc_linux.go Outdated Show resolved Hide resolved
nl/tc_linux.go Outdated Show resolved Hide resolved
@Charlie17Li Charlie17Li force-pushed the bugfix-pedit-srcip branch 2 times, most recently from 55ec793 to baea00e Compare September 11, 2024 16:15
@aboch
Copy link
Collaborator

aboch commented Sep 11, 2024

Thank you @Charlie17Li .
Is it possible to add a new UT or extend an existing one to program and then read back this filter and make sure what is programmed in kernel is what we wanted to program?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants