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

configure Vxlan veth interface mtu based on configured minimum host … #1135

Merged
merged 1 commit into from
May 29, 2018

Conversation

g1rana
Copy link
Contributor

@g1rana g1rana commented May 22, 2018

…interface mtu

Description of the changes:

Currently container mtu in contiv network set as static MTU value as 1450. With this PR, contiv/netplugin
detect minimum configured mtu of host and cofingured Veth pair container side MTU to read MTU - 50 if container is part of Vxlan Bridge in OVS

Type of fix: Bug Fix

Fixes #1132

Please describe:

  • Contiv/netplugin bringup 2 types of OVS switch in every node and all container interface are plugged into these OVS switch based on OVS bridge mode type( Vlan or Vxlan). Incase of Vxlan , Container should not send packets more configured host Interface , to fix this issue, netplugin read lowest configured mtu in set of host interface and set container interface mtu 50 byte less if container interface is part OVS vxlan bridge

  • type of testing done (both manual and automated)
    Bring up Contiv network with Vxlan encap as default switch.
    Bringup container which is part of above contiv network .
    check the configured interface MTU which should be 50 byte less than host minimum MTU

TODO

  • Tests
  • Documentation

Copy link
Contributor

@dseevr dseevr left a comment

Choose a reason for hiding this comment

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

looks fine to me, just a few nitpicks

@@ -100,6 +101,10 @@ func NewOvsSwitch(bridgeName, netType, localIP, fwdMode string,
sw.netType = netType
sw.uplinkDb = cmap.New()
sw.hostPvtNW = hostPvtNW
sw.vxlanEncapMtu, err = netutils.GetHostLowestLinkMtu()
if err != nil {
log.Fatalf("Failed to get Host Node MTU. Err:%v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: add a space between : and %v

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok;

@@ -359,7 +364,12 @@ func (sw *OvsSwitch) CreatePort(intfName string, cfgEp *mastercfg.CfgEndpointSta

// Set the link mtu to 1450 to allow for 50 bytes vxlan encap
// (inner eth header(14) + outer IP(20) outer UDP(8) + vxlan header(8))
err = setLinkMtu(intfName, vxlanEndpointMtu)
if sw.netType == "vxlan" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a constant you can use here instead of a string literal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no constant available ; same string literal has been used other part of code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

see this section of code:
sw.ovsdbDriver.ovsSwitch = sw

    if netType == "vxlan" {
            ofnetPort = vxlanOfnetPort
            ctrlrPort = vxlanCtrlerPort
            switch fwdMode {
            case "bridge":
                    datapath = "vxlan"
            case "routing":
                    datapath = "vrouter"
            default:

@g1rana g1rana merged commit 823fada into contiv:master May 29, 2018
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.

None yet

2 participants