-
Notifications
You must be signed in to change notification settings - Fork 266
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
[vslib]Add MACsec Filters #713
Changes from 1 commit
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 |
---|---|---|
@@ -0,0 +1,19 @@ | ||
#pragma once | ||
|
||
#include "MACsecFilter.h" | ||
|
||
namespace saivs | ||
{ | ||
class MACsecEgressFilter : public MACsecFilter | ||
{ | ||
public: | ||
MACsecEgressFilter(_In_ const std::string &macsecInterfaceName); | ||
|
||
virtual ~MACsecEgressFilter() = default; | ||
|
||
protected: | ||
virtual FilterStatus forward( | ||
_In_ const void *buffer, | ||
_In_ ssize_t length) override; | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
#pragma once | ||
|
||
#include "TrafficFilter.h" | ||
|
||
#include <string> | ||
|
||
namespace saivs | ||
{ | ||
class MACsecFilter : public TrafficFilter | ||
{ | ||
public: | ||
MACsecFilter( | ||
_In_ const std::string &macsec_interface_name); | ||
|
||
virtual ~MACsecFilter() = default; | ||
|
||
virtual FilterStatus execute( | ||
_Inout_ void *buffer, | ||
_Inout_ ssize_t &length) override; | ||
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. why signed size ? and not size_t ? 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. This function will be call after 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. ok, this is fine, i though this size is buffer length, since then it make no sense for it to allow negative numbers 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. I agree your point. |
||
|
||
void enable_macsec_device(bool enable); | ||
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. each param needs prefix In and 1 per line 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. Done, please review it. |
||
|
||
void set_macsec_fd(int macsecfd); | ||
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. same here 1 param per line 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. Done, please review it. |
||
|
||
protected: | ||
virtual FilterStatus forward( | ||
_In_ const void *buffer, | ||
_In_ ssize_t length) = 0; | ||
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. why ssize_t ? 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. Same reason as previous comment. |
||
|
||
bool m_macsec_device_enable; | ||
|
||
int m_macsecfd; | ||
|
||
const std::string m_macsec_interface_name; | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
#pragma once | ||
|
||
#include "MACsecFilter.h" | ||
|
||
namespace saivs | ||
{ | ||
class MACsecIngressFilter : public MACsecFilter | ||
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. base class to new line 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. Done, please review it. |
||
{ | ||
public: | ||
MACsecIngressFilter(_In_ const std::string &macsec_interface_name); | ||
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. 1 param per line 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. Done, please review it. |
||
|
||
virtual ~MACsecIngressFilter() = default; | ||
|
||
protected: | ||
virtual FilterStatus forward( | ||
_In_ const void *buffer, | ||
_In_ ssize_t length) override; | ||
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. why ssize 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. Same reason as previous comment. |
||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
#pragma once | ||
|
||
#include "swss/sal.h" | ||
|
||
#include <sys/types.h> | ||
|
||
namespace saivs | ||
{ | ||
enum FilterPriority | ||
{ | ||
MACSEC_FILTER, | ||
}; | ||
|
||
class TrafficFilter | ||
{ | ||
public: | ||
enum FilterStatus | ||
{ | ||
CONTINUE, | ||
TERMINATE, | ||
ERROR, | ||
}; | ||
|
||
TrafficFilter() = default; | ||
|
||
virtual ~TrafficFilter() = default; | ||
|
||
virtual FilterStatus execute( | ||
_Inout_ void *buffer, | ||
_Inout_ ssize_t &length) = 0; | ||
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. why ssize 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. Same reason as previous comment. |
||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
#pragma once | ||
|
||
#include "TrafficFilter.h" | ||
|
||
#include <memory> | ||
#include <map> | ||
#include <mutex> | ||
|
||
namespace saivs | ||
{ | ||
class TrafficFilterPipes | ||
{ | ||
public: | ||
TrafficFilterPipes() = default; | ||
|
||
virtual ~TrafficFilterPipes() = default; | ||
|
||
bool installFilter( | ||
_In_ int priority, | ||
_In_ std::shared_ptr<TrafficFilter> filter); | ||
|
||
bool uninstallFilter( | ||
_In_ std::shared_ptr<TrafficFilter> filter); | ||
|
||
TrafficFilter::FilterStatus execute( | ||
_Inout_ void *buffer, | ||
_Inout_ ssize_t &length); | ||
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. why ssize 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. Same reason as previous comment. |
||
|
||
private: | ||
typedef std::map<int, std::shared_ptr<TrafficFilter> > FilterPriorityQueue; | ||
|
||
std::mutex m_mutex; | ||
FilterPriorityQueue m_filters; | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
#include "MACsecEgressFilter.h" | ||
|
||
#include <swss/logger.h> | ||
|
||
#include <unistd.h> | ||
#include <string.h> | ||
|
||
using namespace saivs; | ||
|
||
MACsecEgressFilter::MACsecEgressFilter(_In_ const std::string &macsecInterfaceName): | ||
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. 1 param per line 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. Done, please review it. |
||
MACsecFilter(macsecInterfaceName) | ||
{ | ||
SWSS_LOG_ENTER(); | ||
|
||
// empty intentionally | ||
} | ||
|
||
TrafficFilter::FilterStatus MACsecEgressFilter::forward( | ||
_In_ const void *buffer, | ||
_In_ ssize_t length) | ||
{ | ||
SWSS_LOG_ENTER(); | ||
|
||
if (write(m_macsecfd, buffer, length) < 0) | ||
{ | ||
if (errno != ENETDOWN && errno != EIO) | ||
{ | ||
SWSS_LOG_ERROR( | ||
"failed to write to macsec device %s fd %d, errno(%d): %s", | ||
m_macsec_interface_name.c_str(), | ||
m_macsecfd, | ||
errno, | ||
strerror(errno)); | ||
} | ||
|
||
if (errno == EBADF) | ||
{ | ||
SWSS_LOG_ERROR( | ||
"ending thread for macsec device %s fd %d", | ||
m_macsec_interface_name.c_str(), | ||
m_macsecfd); | ||
|
||
return TrafficFilter::ERROR; | ||
} | ||
} | ||
|
||
return TrafficFilter::TERMINATE; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
#include "MACsecFilter.h" | ||
|
||
#include "swss/logger.h" | ||
#include "swss/select.h" | ||
|
||
#include <sys/socket.h> | ||
#include <linux/if_packet.h> | ||
#include <linux/if_ether.h> | ||
#include <arpa/inet.h> | ||
#include <net/if.h> | ||
|
||
using namespace saivs; | ||
|
||
#define EAPOL_ETHER_TYPE (0x888e) | ||
|
||
MACsecFilter::MACsecFilter( | ||
_In_ const std::string &macsec_interface_name): | ||
m_macsec_device_enable(false), | ||
m_macsecfd(0), | ||
m_macsec_interface_name(macsec_interface_name) | ||
{ | ||
SWSS_LOG_ENTER(); | ||
|
||
// empty intentionally | ||
} | ||
|
||
void MACsecFilter::enable_macsec_device(bool enable) | ||
{ | ||
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. missing SWSS_LOG_ENTER() 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. Done, please review it. |
||
m_macsec_device_enable = enable; | ||
} | ||
|
||
void MACsecFilter::set_macsec_fd(int macsecfd) | ||
{ | ||
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. missing swss_log_enter 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. Done, please review it. |
||
m_macsecfd = macsecfd; | ||
} | ||
|
||
TrafficFilter::FilterStatus MACsecFilter::execute( | ||
_Inout_ void *buffer, | ||
_Inout_ ssize_t &length) | ||
{ | ||
SWSS_LOG_ENTER(); | ||
|
||
auto mac_hdr = static_cast<const ethhdr *>(buffer); | ||
|
||
if (ntohs(mac_hdr->h_proto) == EAPOL_ETHER_TYPE) | ||
{ | ||
// EAPOL traffic will never be delivered to MACsec device | ||
return TrafficFilter::CONTINUE; | ||
} | ||
|
||
if (m_macsec_device_enable) | ||
{ | ||
return forward(buffer, length); | ||
} | ||
|
||
// Drop all non-EAPOL packets if macsec device haven't been enable. | ||
return TrafficFilter::TERMINATE; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
#include "MACsecIngressFilter.h" | ||
|
||
#include "swss/logger.h" | ||
|
||
#include <unistd.h> | ||
#include <string.h> | ||
|
||
using namespace saivs; | ||
|
||
MACsecIngressFilter::MACsecIngressFilter( | ||
_In_ const std::string &macsec_interface_name) : | ||
MACsecFilter(macsec_interface_name) | ||
{ | ||
SWSS_LOG_ENTER(); | ||
|
||
// empty intentionally | ||
} | ||
|
||
TrafficFilter::FilterStatus MACsecIngressFilter::forward( | ||
_In_ const void *buffer, | ||
_In_ ssize_t length) | ||
{ | ||
SWSS_LOG_ENTER(); | ||
|
||
// MACsec interface will automatically forward ingress MACsec traffic | ||
// by Linux Kernel. | ||
// So this filter just need to drop all ingress MACsec traffic directly | ||
|
||
return TrafficFilter::TERMINATE; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
#include "TrafficFilterPipes.h" | ||
|
||
#include "swss/logger.h" | ||
|
||
using namespace saivs; | ||
|
||
bool TrafficFilterPipes::installFilter( | ||
_In_ int priority, | ||
_In_ std::shared_ptr<TrafficFilter> filter) | ||
{ | ||
SWSS_LOG_ENTER(); | ||
|
||
std::unique_lock<std::mutex> guard(m_mutex); | ||
|
||
return m_filters.emplace(priority, filter).second; | ||
} | ||
|
||
bool TrafficFilterPipes::uninstallFilter( | ||
_In_ std::shared_ptr<TrafficFilter> filter) | ||
{ | ||
SWSS_LOG_ENTER(); | ||
|
||
std::unique_lock<std::mutex> guard(m_mutex); | ||
|
||
for (auto itr = m_filters.begin(); | ||
itr != m_filters.end(); | ||
itr ++) | ||
{ | ||
if (itr->second == filter) | ||
{ | ||
m_filters.erase(itr); | ||
|
||
return true; | ||
} | ||
} | ||
|
||
return false; | ||
} | ||
|
||
TrafficFilter::FilterStatus TrafficFilterPipes::execute( | ||
_Inout_ void *buffer, | ||
_Inout_ ssize_t &length) | ||
{ | ||
SWSS_LOG_ENTER(); | ||
|
||
std::unique_lock<std::mutex> guard(m_mutex); | ||
TrafficFilter::FilterStatus ret = TrafficFilter::CONTINUE; | ||
|
||
for (auto itr = m_filters.begin(); itr != m_filters.end();) | ||
{ | ||
auto filter = itr->second; | ||
|
||
if (filter) | ||
{ | ||
ret = filter->execute( | ||
buffer, | ||
length); | ||
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. join 3 lines to one line ? this is small enough 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. Done, please review it. |
||
|
||
if (ret == TrafficFilter::CONTINUE) | ||
{ | ||
itr ++; | ||
} | ||
else | ||
{ | ||
break; | ||
} | ||
} | ||
else | ||
{ | ||
itr = m_filters.erase(itr); | ||
} | ||
|
||
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. unnecessary empty line 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. Done, please review it. |
||
} | ||
|
||
return ret; | ||
} |
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.
base class to new line
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.
Done, please review it.