-
Notifications
You must be signed in to change notification settings - Fork 257
enable dual NIC support in transparent VLAN #4057
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
base: master
Are you sure you want to change the base?
Changes from all commits
2ddbf89
2f33a42
776430c
7cc5010
c31176e
4441787
6fde3f8
3ddc19e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -292,7 +292,6 @@ func (client *TransparentVlanEndpointClient) PopulateVM(epInfo *EndpointInfo) er | |
_, err = client.netioshim.GetNetworkInterfaceByName(client.vlanIfName) | ||
return errors.Wrap(err, "failed to get vlan interface") | ||
}, numRetries, sleepInMs) | ||
|
||
if err != nil { | ||
deleteNSIfNotNilErr = errors.Wrapf(err, "failed to get vlan interface: %s", client.vlanIfName) | ||
return deleteNSIfNotNilErr | ||
|
@@ -400,14 +399,32 @@ func (client *TransparentVlanEndpointClient) PopulateVnet(epInfo *EndpointInfo) | |
return nil | ||
} | ||
|
||
// Set ARP proxy on the vlan interface to respond to ARP requests for the gateway IP | ||
func (client *TransparentVlanEndpointClient) setArpProxy(ifName string) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: for the comment, the function itself is not necessarily on the vlan interface, just whatever interface is passed in |
||
cmd := fmt.Sprintf("echo 1 > /proc/sys/net/ipv4/conf/%v/proxy_arp", ifName) | ||
_, err := client.plClient.ExecuteRawCommand(cmd) | ||
Comment on lines
+404
to
+405
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using shell command execution with string formatting could be vulnerable to command injection if ifName contains malicious characters. Consider using a safer approach or validating the interface name against a whitelist of allowed characters. Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||
if err != nil { | ||
logger.Error("Failed to set ARP proxy", zap.String("interface", ifName), zap.Error(err)) | ||
return errors.Wrap(err, "failed to set arp proxy") | ||
} | ||
return nil | ||
} | ||
|
||
func (client *TransparentVlanEndpointClient) AddEndpointRules(epInfo *EndpointInfo) error { | ||
if err := client.AddSnatEndpointRules(); err != nil { | ||
return errors.Wrap(err, "failed to add snat endpoint rules") | ||
} | ||
logger.Info("[transparent-vlan] Adding tunneling rules in vnet namespace") | ||
err := ExecuteInNS(client.nsClient, client.vnetNSName, func() error { | ||
return client.AddVnetRules(epInfo) | ||
if err := client.AddVnetRules(epInfo); err != nil { | ||
return err | ||
} | ||
|
||
// Set ARP proxy on vnet veth (inside vnet namespace) | ||
logger.Info("calling setArpProxy for", zap.String("vnetVethName", client.vnetVethName)) | ||
return client.setArpProxy(client.vnetVethName) | ||
}) | ||
|
||
return err | ||
} | ||
|
||
|
@@ -519,9 +536,15 @@ func (client *TransparentVlanEndpointClient) ConfigureContainerInterfacesAndRout | |
} | ||
} | ||
|
||
if epInfo.SkipDefaultRoutes { | ||
logger.Info("Skipping adding routes in container ns as requested") | ||
return nil | ||
} | ||
logger.Info("Adding default routes in container ns") | ||
if err := client.addDefaultRoutes(client.containerVethName, 0); err != nil { | ||
return errors.Wrap(err, "failed container ns add default routes") | ||
} | ||
|
||
if err := client.AddDefaultArp(client.containerVethName, client.vnetMac.String()); err != nil { | ||
return errors.Wrap(err, "failed container ns add default arp") | ||
} | ||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: could you also update the comment here if linux now supports dual nic?