Port: review the netlink commands

[glimchb]

see

opi-evpn-bridge/pkg/evpn/port.go

Lines 54 to 117 in 6b0df33

	// not found, so create a new one
	bridgeName := "br-tenant"
	// get tenant bridge device by name
	bridge, err := netlink.LinkByName(bridgeName)
	if err != nil {
	err := status.Errorf(codes.NotFound, "unable to find key %s", bridgeName)
	log.Printf("error: %v", err)
	return nil, err
	}
	// get base interface (e.g.: eth2)
	iface, err := netlink.LinkByName(resourceID)
	if err != nil {
	err := status.Errorf(codes.NotFound, "unable to find key %s", resourceID)
	log.Printf("error: %v", err)
	return nil, err
	}
	// Example: ip link set eth2 addr aa:bb:cc:00:00:41
	if len(in.BridgePort.Spec.MacAddress) > 0 {
	if err := netlink.LinkSetHardwareAddr(iface, in.BridgePort.Spec.MacAddress); err != nil {
	fmt.Printf("Failed to set MAC on link: %v", err)
	return nil, err
	}
	}
	// Example: ip link set eth2 master br-tenant
	if err := netlink.LinkSetMaster(iface, bridge); err != nil {
	fmt.Printf("Failed to add iface to bridge: %v", err)
	return nil, err
	}
	// add port to specified logical bridges
	for _, bridgeRefName := range in.BridgePort.Spec.LogicalBridges {
	fmt.Printf("add iface to logical bridge %s", bridgeRefName)
	// get object from DB
	bridgeObject, ok := s.Bridges[bridgeRefName]
	if !ok {
	err := status.Errorf(codes.NotFound, "unable to find key %s", bridgeRefName)
	log.Printf("error: %v", err)
	return nil, err
	}
	vid := uint16(bridgeObject.Spec.VlanId)
	switch in.BridgePort.Spec.Ptype {
	case pb.BridgePortType_ACCESS:
	// Example: bridge vlan add dev eth2 vid 20 pvid untagged
	if err := netlink.BridgeVlanAdd(iface, vid, true, true, false, false); err != nil {
	fmt.Printf("Failed to add vlan to bridge: %v", err)
	return nil, err
	}
	case pb.BridgePortType_TRUNK:
	// Example: bridge vlan add dev eth2 vid 20
	if err := netlink.BridgeVlanAdd(iface, vid, false, false, false, false); err != nil {
	fmt.Printf("Failed to add vlan to bridge: %v", err)
	return nil, err
	}
	default:
	msg := fmt.Sprintf("Only ACCESS or TRUNK supported and not (%d)", in.BridgePort.Spec.Ptype)
	log.Print(msg)
	return nil, status.Errorf(codes.InvalidArgument, msg)
	}
	}
	// Example: ip link set eth2 up
	if err := netlink.LinkSetUp(iface); err != nil {
	fmt.Printf("Failed to up iface link: %v", err)
	return nil, err
	}
	response := proto.Clone(in.BridgePort).(*pb.BridgePort)

need another look and review

[glimchb]

from @mardim91

In the below code I would have done it a bit differently. I would have first checked for the type of port and the do a for loop for the logical bridges. That is because you would have always one type of BridgePort for all the logical Birdges in the list


for _, bridgeRefName := range in.BridgePort.Spec.LogicalBridges {
		fmt.Printf("add iface to logical bridge %s", bridgeRefName)
		// get object from DB
		bridgeObject, ok := s.Bridges[bridgeRefName]
		if !ok {
			err := status.Errorf(codes.NotFound, "unable to find key %s", bridgeRefName)
			log.Printf("error: %v", err)
			return nil, err
		}
		vid := uint16(bridgeObject.Spec.VlanId)
		switch in.BridgePort.Spec.Ptype {
		case pb.BridgePortType_ACCESS:
			// Example: bridge vlan add dev eth2 vid 20 pvid untagged
			if err := netlink.BridgeVlanAdd(iface, vid, true, true, false, false); err != nil {
				fmt.Printf("Failed to add vlan to bridge: %v", err)
				return nil, err
			}
		case pb.BridgePortType_TRUNK:
			// Example: bridge vlan add dev eth2 vid 20
			if err := netlink.BridgeVlanAdd(iface, vid, false, false, false, false); err != nil {
				fmt.Printf("Failed to add vlan to bridge: %v", err)
				return nil, err
			}
		default:
			msg := fmt.Sprintf("Only ACCESS or TRUNK supported and not (%d)", in.BridgePort.Spec.Ptype)
			log.Print(msg)
			return nil, status.Errorf(codes.InvalidArgument, msg)
		}
	}

[mardim91]

The only comment that I have is the above one. The rest looks ok