Skip to content

bugfix: parse ipv4 src/dst error#1015

Merged
aboch merged 1 commit intovishvananda:mainfrom
Charlie17Li:bugfix-pedit-srcip
May 9, 2025
Merged

bugfix: parse ipv4 src/dst error#1015
aboch merged 1 commit intovishvananda:mainfrom
Charlie17Li:bugfix-pedit-srcip

Conversation

@Charlie17Li
Copy link
Copy Markdown
Contributor

@Charlie17Li Charlie17Li commented Sep 8, 2024

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
Copy Markdown
Contributor Author

Charlie17Li commented Sep 8, 2024 •
edited
Loading

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
Copy Markdown
Contributor Author

Charlie17Li commented Sep 8, 2024

cc @aboch @chent1996

aboch
aboch reviewed Sep 8, 2024
View reviewed changes
aboch
aboch reviewed Sep 9, 2024
View reviewed changes
@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
Copy Markdown
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?

@aboch
Copy link
Copy Markdown
Collaborator

aboch commented May 9, 2025

LGTM

@aboch aboch merged commit e9f11f7 into vishvananda:main May 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aboch aboch aboch left review comments

+1 more reviewer

@chent1996 chent1996 chent1996 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Charlie17Li @aboch @chent1996