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

[iccpd] Add nft-based ebtables utilities. #19324

Merged
merged 3 commits into from
Nov 19, 2024

Conversation

puffc
Copy link
Contributor

@puffc puffc commented Jun 17, 2024

Why I did it

Fix a problem (fixes #19323) where ebtables command cannot be executed by iccpd. This cause the trapped BUM packets (ipv6 neighbor discovery) looping back to the MCLAG port channel.

Work item tracking
  • Microsoft ADO (number only):

How I did it

Replacing the legacy ebtables with the nft-based ebtables command suite by adding iptables package into the iccpd docker container.

How to verify it

Verified the ebtables command can be executed from the iccpd docker. When the MCLAG reaches operational state the isolation rules can be added.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

master
202305
202211

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@puffc puffc requested a review from lguohan as a code owner June 17, 2024 01:58
@puffc
Copy link
Contributor Author

puffc commented Jun 17, 2024

@yxieca @Praveen-Brcm Please help to review this PR. Thanks.

@TafkaMax
Copy link

TafkaMax commented Jun 20, 2024

I added the iptables to the Dockerfile.j2 to test the changes.

I seem to get these syslog messages:

 iccpd#supervisord: iccpd ebtables v1.8.9 (nf_tables): Could not fetch rule set generation id: Permission denied (you must be root)

Inside the container.

# enter container
docker exec -it iccpd sh
# iptables -L
iptables v1.8.9 (nf_tables): Could not fetch rule set generation id: Permission denied (you must be root)

@puffc
Copy link
Contributor Author

puffc commented Jun 20, 2024

I added the iptables to the Dockerfile.j2 to test the changes.

I seem to get these syslog messages:


 iccpd#supervisord: iccpd ebtables v1.8.9 (nf_tables): Could not fetch rule set generation id: Permission denied (you must be root)

Inside the container.


# enter container

docker exec -it iccpd sh

# iptables -L

iptables v1.8.9 (nf_tables): Could not fetch rule set generation id: Permission denied (you must be root)

Thanks for your valuable feedback. I'll look into it and it may relate to PR #17835.

@kperumalbfn
Copy link
Contributor

Thanks @puffc Could you update the diff with the fix for permission denied error. We could merge the change after that.

Signed-off-by: Julian Chang - TW <[email protected]>
@puffc puffc requested review from qiluo-msft and xumia as code owners June 21, 2024 01:46
@puffc
Copy link
Contributor Author

puffc commented Jun 21, 2024

Thanks @puffc Could you update the diff with the fix for permission denied error. We could merge the change after that.

Thanks @kperumalbfn. The permission problem had been fixed.

@puffc
Copy link
Contributor Author

puffc commented Jun 21, 2024

/azpw ms_conflict

@yxieca yxieca requested review from saiarcot895 and prsunny June 24, 2024 01:38
@TafkaMax
Copy link

TafkaMax commented Jul 3, 2024

Are people on vacation?

@saiarcot895
Copy link
Contributor

Since the ebtables package just has the legacy version now, can that be removed, and only the iptables package be installed?

@TafkaMax
Copy link

TafkaMax commented Jul 4, 2024

Since the ebtables package just has the legacy version now, can that be removed, and only the iptables package be installed?

ebtables CLI utility does come with the iptables package, but I am not 100% sure if it can be removed without testing it first

Signed-off-by: Julian Chang - TW <[email protected]>
@puffc
Copy link
Contributor Author

puffc commented Jul 4, 2024

Remvoed legacy ebtables packages. Now ebtables only points to ebtables-nft.

root@sonic:/# ebtables -L
Bridge table: filter

Bridge chain: INPUT, entries: 0, policy: ACCEPT

Bridge chain: FORWARD, entries: 4, policy: ACCEPT
-d BGA -j DROP
-p ARP -j DROP
-p 802_1Q --vlan-encap 0806 -j DROP
-d Multicast -j DROP

Bridge chain: OUTPUT, entries: 0, policy: ACCEPT
root@sonic:/# update-alternatives --list ebtables
/usr/sbin/ebtables-nft
root@sonic:/# ebtables -V
ebtables 1.8.7 (nf_tables)
root@sonic:/#

@puffc
Copy link
Contributor Author

puffc commented Jul 7, 2024

@lguohan Please review and merge. Thanks.

@puffc
Copy link
Contributor Author

puffc commented Jul 15, 2024

@xumia @prsunny Please review this PR. Thanks.

@puffc
Copy link
Contributor Author

puffc commented Aug 20, 2024

@lguohan Please review, Thanks.

@puffc
Copy link
Contributor Author

puffc commented Sep 23, 2024

@prsunny Would you please merge this PR? Many thanks!

@lguohan lguohan merged commit 9eeae96 into sonic-net:master Nov 19, 2024
21 checks passed
@radha-danda
Copy link

@lguohan, kindly cherry-pick the patch to 202405 branch

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.

[MCLAG] iccpd cannot execute "ebtables" command.
7 participants