From be91a95998f68b10f6eabcaa941a3328d7b19ef1 Mon Sep 17 00:00:00 2001 From: Preetham Singh Date: Tue, 14 Sep 2021 23:12:41 -0700 Subject: [PATCH 1/3] Routed subinterface enhancements Support for long name and short name routed subinterfaces This swss common library provides APIS for: - Performs Subinterface validation checks - Get parent interface corresponding to subinterface(short/long name) - Get subinterface index --- common/Makefile.am | 3 +- common/ifcommon.cpp | 120 ++++++++++++++++++++++++++++++++++++++++++++ common/ifcommon.h | 26 ++++++++++ 3 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 common/ifcommon.cpp create mode 100644 common/ifcommon.h diff --git a/common/Makefile.am b/common/Makefile.am index 0ae31d1c9..0ac9273df 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -64,7 +64,8 @@ libswsscommon_la_SOURCES = \ subscriberstatetable.cpp \ timestamp.cpp \ warm_restart.cpp \ - redisutility.cpp + redisutility.cpp \ + ifcommon.cpp libswsscommon_la_CXXFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CFLAGS) $(CODE_COVERAGE_CXXFLAGS) libswsscommon_la_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CPPFLAGS) $(CODE_COVERAGE_CPPFLAGS) diff --git a/common/ifcommon.cpp b/common/ifcommon.cpp new file mode 100644 index 000000000..ae2fe2ce4 --- /dev/null +++ b/common/ifcommon.cpp @@ -0,0 +1,120 @@ +#include +#include +#include +#include +#include "ifcommon.h" + +using namespace swss; + +subIntf::subIntf(const std::string &ifName) +{ + alias = ifName; + size_t found = alias.find(VLAN_SUB_INTERFACE_SEPARATOR); + if (found != std::string::npos) + { + parentIf = alias.substr(0, found); + if (!parentIf.compare(0, strlen("Eth"), "Eth")) + { + std::string parentIfName; + if (!parentIf.compare(0, strlen("Ethernet"), "Ethernet")) + { + parentIfShortName = "Eth" + parentIf.substr(strlen("Ethernet")); + isCompressed = false; + parentIfName = "Ethernet" + parentIf.substr(strlen("Ethernet")); + } + else + { + parentIfShortName = "Eth" + parentIf.substr(strlen("Eth")); + isCompressed = true; + parentIfName = "Ethernet" + parentIf.substr(strlen("Eth")); + } + parentIf = parentIfName; + subIfIdx = alias.substr(found + 1); + } + else if (!parentIf.compare(0, strlen("Po"), "Po")) + { + if (!parentIf.compare(0, strlen("PortChannel"), "PortChannel")) + { + isCompressed = false; + parentIfShortName = "Po" + parentIf.substr(strlen("PortChannel")); + parentIf = "PortChannel" + parentIf.substr(strlen("PortChannel")); + } + else + { + isCompressed = true; + parentIfShortName = "Po" + parentIf.substr(strlen("Po")); + parentIf = "PortChannel" + parentIf.substr(strlen("Po")); + } + subIfIdx = alias.substr(found + 1); + } + else + { + subIfIdx = ""; + } + } +} + +bool subIntf::isValid() +{ + if (subIfIdx == "") + { + return false; + } + else if (alias.length() >= IFNAMSIZ) + { + return false; + } + + return true; +} + +std::string subIntf::parentIntf() +{ + return parentIf; +} + +int subIntf::subIntfIdx() +{ + uint16_t id; + try + { + id = static_cast(stoul(subIfIdx)); + } + catch (const std::invalid_argument &e) + { + return -1; + } + catch (const std::out_of_range &e) + { + return -1; + } + return id; +} + +std::string subIntf::longName() +{ + if (isValid() == false) + return ""; + + return (parentIf + VLAN_SUB_INTERFACE_SEPARATOR + subIfIdx); +} + +std::string subIntf::shortName() +{ + if (isValid() == false) + return ""; + + return (parentIfShortName + VLAN_SUB_INTERFACE_SEPARATOR + subIfIdx); +} + +bool subIntf::isShortName() +{ + if (isCompressed == true) + { + return true; + } + else + { + return false; + } +} diff --git a/common/ifcommon.h b/common/ifcommon.h new file mode 100644 index 000000000..a489bcfeb --- /dev/null +++ b/common/ifcommon.h @@ -0,0 +1,26 @@ +#ifndef __IFCOMMON_H__ +#define __IFCOMMON_H__ + +namespace swss { + class subIntf + { + public: + subIntf(const std::string &ifName); + bool isValid(); + std::string parentIntf(); + int subIntfIdx(); + std::string longName(); + std::string shortName(); + bool isShortName(); + + private: +#define VLAN_SUB_INTERFACE_SEPARATOR "." + std::string alias = ""; + std::string subIfIdx = ""; + std::string parentIf = ""; + std::string parentIfShortName = ""; + bool isCompressed = false; + }; +} + +#endif From e34fe85f20b733e50846dda5445e775bbe1a52b5 Mon Sep 17 00:00:00 2001 From: Preetham Singh Date: Tue, 14 Sep 2021 23:23:36 -0700 Subject: [PATCH 2/3] Indentation fix --- common/Makefile.am | 2 +- common/ifcommon.cpp | 104 ++++++++++++++++++++++---------------------- common/ifcommon.h | 31 +++++++------ 3 files changed, 68 insertions(+), 69 deletions(-) diff --git a/common/Makefile.am b/common/Makefile.am index 0ac9273df..ae3600c33 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -65,7 +65,7 @@ libswsscommon_la_SOURCES = \ timestamp.cpp \ warm_restart.cpp \ redisutility.cpp \ - ifcommon.cpp + ifcommon.cpp libswsscommon_la_CXXFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CFLAGS) $(CODE_COVERAGE_CXXFLAGS) libswsscommon_la_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CPPFLAGS) $(CODE_COVERAGE_CPPFLAGS) diff --git a/common/ifcommon.cpp b/common/ifcommon.cpp index ae2fe2ce4..499ef99c7 100644 --- a/common/ifcommon.cpp +++ b/common/ifcommon.cpp @@ -9,68 +9,68 @@ using namespace swss; subIntf::subIntf(const std::string &ifName) { alias = ifName; - size_t found = alias.find(VLAN_SUB_INTERFACE_SEPARATOR); - if (found != std::string::npos) - { - parentIf = alias.substr(0, found); - if (!parentIf.compare(0, strlen("Eth"), "Eth")) - { + size_t found = alias.find(VLAN_SUB_INTERFACE_SEPARATOR); + if (found != std::string::npos) + { + parentIf = alias.substr(0, found); + if (!parentIf.compare(0, strlen("Eth"), "Eth")) + { std::string parentIfName; - if (!parentIf.compare(0, strlen("Ethernet"), "Ethernet")) - { - parentIfShortName = "Eth" + parentIf.substr(strlen("Ethernet")); - isCompressed = false; - parentIfName = "Ethernet" + parentIf.substr(strlen("Ethernet")); - } - else - { - parentIfShortName = "Eth" + parentIf.substr(strlen("Eth")); - isCompressed = true; - parentIfName = "Ethernet" + parentIf.substr(strlen("Eth")); - } + if (!parentIf.compare(0, strlen("Ethernet"), "Ethernet")) + { + parentIfShortName = "Eth" + parentIf.substr(strlen("Ethernet")); + isCompressed = false; + parentIfName = "Ethernet" + parentIf.substr(strlen("Ethernet")); + } + else + { + parentIfShortName = "Eth" + parentIf.substr(strlen("Eth")); + isCompressed = true; + parentIfName = "Ethernet" + parentIf.substr(strlen("Eth")); + } parentIf = parentIfName; - subIfIdx = alias.substr(found + 1); - } - else if (!parentIf.compare(0, strlen("Po"), "Po")) - { - if (!parentIf.compare(0, strlen("PortChannel"), "PortChannel")) - { - isCompressed = false; - parentIfShortName = "Po" + parentIf.substr(strlen("PortChannel")); - parentIf = "PortChannel" + parentIf.substr(strlen("PortChannel")); - } - else - { - isCompressed = true; - parentIfShortName = "Po" + parentIf.substr(strlen("Po")); - parentIf = "PortChannel" + parentIf.substr(strlen("Po")); - } - subIfIdx = alias.substr(found + 1); - } - else - { - subIfIdx = ""; - } - } + subIfIdx = alias.substr(found + 1); + } + else if (!parentIf.compare(0, strlen("Po"), "Po")) + { + if (!parentIf.compare(0, strlen("PortChannel"), "PortChannel")) + { + isCompressed = false; + parentIfShortName = "Po" + parentIf.substr(strlen("PortChannel")); + parentIf = "PortChannel" + parentIf.substr(strlen("PortChannel")); + } + else + { + isCompressed = true; + parentIfShortName = "Po" + parentIf.substr(strlen("Po")); + parentIf = "PortChannel" + parentIf.substr(strlen("Po")); + } + subIfIdx = alias.substr(found + 1); + } + else + { + subIfIdx = ""; + } + } } bool subIntf::isValid() { - if (subIfIdx == "") + if (subIfIdx == "") { - return false; + return false; } else if (alias.length() >= IFNAMSIZ) { return false; } - return true; + return true; } std::string subIntf::parentIntf() { - return parentIf; + return parentIf; } int subIntf::subIntfIdx() @@ -88,23 +88,23 @@ int subIntf::subIntfIdx() { return -1; } - return id; + return id; } std::string subIntf::longName() { - if (isValid() == false) - return ""; + if (isValid() == false) + return ""; - return (parentIf + VLAN_SUB_INTERFACE_SEPARATOR + subIfIdx); + return (parentIf + VLAN_SUB_INTERFACE_SEPARATOR + subIfIdx); } std::string subIntf::shortName() { - if (isValid() == false) - return ""; + if (isValid() == false) + return ""; - return (parentIfShortName + VLAN_SUB_INTERFACE_SEPARATOR + subIfIdx); + return (parentIfShortName + VLAN_SUB_INTERFACE_SEPARATOR + subIfIdx); } bool subIntf::isShortName() diff --git a/common/ifcommon.h b/common/ifcommon.h index a489bcfeb..e5ae61e64 100644 --- a/common/ifcommon.h +++ b/common/ifcommon.h @@ -2,25 +2,24 @@ #define __IFCOMMON_H__ namespace swss { - class subIntf - { - public: - subIntf(const std::string &ifName); - bool isValid(); - std::string parentIntf(); - int subIntfIdx(); - std::string longName(); - std::string shortName(); - bool isShortName(); - + class subIntf + { + public: + subIntf(const std::string &ifName); + bool isValid(); + std::string parentIntf(); + int subIntfIdx(); + std::string longName(); + std::string shortName(); + bool isShortName(); private: #define VLAN_SUB_INTERFACE_SEPARATOR "." std::string alias = ""; - std::string subIfIdx = ""; - std::string parentIf = ""; - std::string parentIfShortName = ""; - bool isCompressed = false; - }; + std::string subIfIdx = ""; + std::string parentIf = ""; + std::string parentIfShortName = ""; + bool isCompressed = false; + }; } #endif From ee1b1f63bdb032477917a4c55771bbf368c3d0e9 Mon Sep 17 00:00:00 2001 From: Preetham Singh Date: Wed, 15 Sep 2021 07:58:01 -0700 Subject: [PATCH 3/3] Addressing review comments --- common/Makefile.am | 2 +- common/ifcommon.h | 25 ------------------------- common/{ifcommon.cpp => subintf.cpp} | 23 ++++++++--------------- common/subintf.h | 22 ++++++++++++++++++++++ 4 files changed, 31 insertions(+), 41 deletions(-) delete mode 100644 common/ifcommon.h rename common/{ifcommon.cpp => subintf.cpp} (88%) create mode 100644 common/subintf.h diff --git a/common/Makefile.am b/common/Makefile.am index ae3600c33..6ab5139be 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -65,7 +65,7 @@ libswsscommon_la_SOURCES = \ timestamp.cpp \ warm_restart.cpp \ redisutility.cpp \ - ifcommon.cpp + subintf.cpp libswsscommon_la_CXXFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CFLAGS) $(CODE_COVERAGE_CXXFLAGS) libswsscommon_la_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CPPFLAGS) $(CODE_COVERAGE_CPPFLAGS) diff --git a/common/ifcommon.h b/common/ifcommon.h deleted file mode 100644 index e5ae61e64..000000000 --- a/common/ifcommon.h +++ /dev/null @@ -1,25 +0,0 @@ -#ifndef __IFCOMMON_H__ -#define __IFCOMMON_H__ - -namespace swss { - class subIntf - { - public: - subIntf(const std::string &ifName); - bool isValid(); - std::string parentIntf(); - int subIntfIdx(); - std::string longName(); - std::string shortName(); - bool isShortName(); - private: -#define VLAN_SUB_INTERFACE_SEPARATOR "." - std::string alias = ""; - std::string subIfIdx = ""; - std::string parentIf = ""; - std::string parentIfShortName = ""; - bool isCompressed = false; - }; -} - -#endif diff --git a/common/ifcommon.cpp b/common/subintf.cpp similarity index 88% rename from common/ifcommon.cpp rename to common/subintf.cpp index 499ef99c7..ab26ee62d 100644 --- a/common/ifcommon.cpp +++ b/common/subintf.cpp @@ -2,7 +2,7 @@ #include #include #include -#include "ifcommon.h" +#include "subintf.h" using namespace swss; @@ -54,7 +54,7 @@ subIntf::subIntf(const std::string &ifName) } } -bool subIntf::isValid() +bool subIntf::isValid() const { if (subIfIdx == "") { @@ -68,12 +68,12 @@ bool subIntf::isValid() return true; } -std::string subIntf::parentIntf() +std::string subIntf::parentIntf() const { return parentIf; } -int subIntf::subIntfIdx() +int subIntf::subIntfIdx() const { uint16_t id; try @@ -91,7 +91,7 @@ int subIntf::subIntfIdx() return id; } -std::string subIntf::longName() +std::string subIntf::longName() const { if (isValid() == false) return ""; @@ -99,7 +99,7 @@ std::string subIntf::longName() return (parentIf + VLAN_SUB_INTERFACE_SEPARATOR + subIfIdx); } -std::string subIntf::shortName() +std::string subIntf::shortName() const { if (isValid() == false) return ""; @@ -107,14 +107,7 @@ std::string subIntf::shortName() return (parentIfShortName + VLAN_SUB_INTERFACE_SEPARATOR + subIfIdx); } -bool subIntf::isShortName() +bool subIntf::isShortName() const { - if (isCompressed == true) - { - return true; - } - else - { - return false; - } + return ((isCompressed == true) ? true : false); } diff --git a/common/subintf.h b/common/subintf.h new file mode 100644 index 000000000..e16324f1f --- /dev/null +++ b/common/subintf.h @@ -0,0 +1,22 @@ +#pragma once + +#define VLAN_SUB_INTERFACE_SEPARATOR "." +namespace swss { + class subIntf + { + public: + subIntf(const std::string &ifName); + bool isValid() const; + std::string parentIntf() const; + int subIntfIdx() const; + std::string longName() const; + std::string shortName() const; + bool isShortName() const; + private: + std::string alias; + std::string subIfIdx; + std::string parentIf; + std::string parentIfShortName; + bool isCompressed = false; + }; +}