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

Set net.ipv4.conf.antrea-gw0.arp_announce to 1 #5657

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

gran-vmv
Copy link
Contributor

@gran-vmv gran-vmv commented Nov 2, 2023

Fix #5451
Set arp_announce to 1 on Linux platform to make the ARP requests sent on the gateway
interface always use the gateway IP as the source IP, otherwise the ARP requests would be
dropped by ARP SpoofGuard flow.

@gran-vmv gran-vmv self-assigned this Nov 2, 2023
@gran-vmv
Copy link
Contributor Author

gran-vmv commented Nov 2, 2023

/test-flexible-ipam-e2e

Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add commit message to explain the changes.
Unit test failure should be related too.

@@ -730,6 +730,11 @@ func (i *Initializer) setupGatewayInterface() error {
if err := i.setInterfaceMTU(i.hostGateway, i.networkConfig.InterfaceMTU); err != nil {
return err
}
if i.nodeConfig.GatewayConfig.IPv4 != nil {
if err := i.setInterfaceARPAnnounce(gatewayIface.InterfaceName, 1); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add comment why this is needed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comment and PR description about this context.

@luolanzone luolanzone added this to the Antrea v1.15 release milestone Nov 3, 2023
@luolanzone luolanzone added the action/release-note Indicates a PR that should be included in release notes. label Nov 3, 2023
@gran-vmv gran-vmv force-pushed the ipam-arp branch 2 times, most recently from b65af14 to 2c59098 Compare November 3, 2023 06:14
@gran-vmv
Copy link
Contributor Author

gran-vmv commented Nov 3, 2023

/test-flexible-ipam-e2e

@gran-vmv gran-vmv added kind/bug Categorizes issue or PR as related to a bug. action/backport Indicates a PR that requires backports. labels Nov 3, 2023
@@ -730,6 +730,15 @@ func (i *Initializer) setupGatewayInterface() error {
if err := i.setInterfaceMTU(i.hostGateway, i.networkConfig.InterfaceMTU); err != nil {
return err
}
// Fix #5451
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typically no need to link issue in code file when the comment can explain the code. Otherwise the code file would be full of issue links.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed issue link.

Comment on lines 734 to 735
// Set arp_announce to 1 on Linux platform to make the ARP requests from host to gateway
// interface always use gateway IP as source IP. These ARP requests without gateway IP will
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"the ARP requests from host to gateway interface" doesn't sound correct.

Set arp_announce to 1 on Linux platform to make the ARP requests sent on the gateway interface always use the gateway IP as the source IP, otherwise the ARP requests would be dropped by ARP SpoofGuard flow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated code comment, commit message and PR description.

Fix antrea-io#5451
Set arp_announce to 1 on Linux platform to make the ARP requests sent on the gateway
interface always use the gateway IP as the source IP, otherwise the ARP requests would be
dropped by ARP SpoofGuard flow.

Signed-off-by: gran <[email protected]>
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tnqn
Copy link
Member

tnqn commented Nov 7, 2023

/test-flexible-ipam-e2e
/test-all

@tnqn
Copy link
Member

tnqn commented Nov 7, 2023

/test-networkpolicy

@tnqn tnqn merged commit 0196e18 into antrea-io:main Nov 7, 2023
49 of 57 checks passed
@tnqn
Copy link
Member

tnqn commented Nov 7, 2023

@gran-vmv please backport it to 1.12-1.14

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes. kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[flexible-ipam] ARP request leaks cause network issue
3 participants