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.

Sign up for GitHub

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

fix dhcpv6 counters are not cleared on clear command #38

Merged
merged 1 commit into from
Jun 5, 2023
Merged

fix dhcpv6 counters are not cleared on clear command #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion .github/workflows/codeql-analysis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ on:
branches:
- 'master'
- '202[0-9][0-9][0-9]'
pull_request_target:
pull_request:
branches:
- 'master'
- '202[0-9][0-9][0-9]'
Expand Down Expand Up @@ -58,6 +58,13 @@ jobs:
libpython2.7-dev \
python

- name: reset-submodules
run: |
git submodule foreach --recursive 'git clean -xfdf || true'
git submodule foreach --recursive 'git reset --hard || true'
git submodule foreach --recursive 'git remote update || true'
git submodule update --init --recursive

- name: build-swss-common
run: |
set -x
Expand Down
123 changes: 54 additions & 69 deletions src/relay.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,23 +50,24 @@ const struct sock_fprog ether_relay_fprog = {
ether_relay_filter
};

/* DHCPv6 Counter */
uint64_t counters[DHCPv6_MESSAGE_TYPE_COUNT];
std::map<int, std::string> counterMap = {{DHCPv6_MESSAGE_TYPE_UNKNOWN, "Unknown"},
{DHCPv6_MESSAGE_TYPE_SOLICIT, "Solicit"},
{DHCPv6_MESSAGE_TYPE_ADVERTISE, "Advertise"},
{DHCPv6_MESSAGE_TYPE_REQUEST, "Request"},
{DHCPv6_MESSAGE_TYPE_CONFIRM, "Confirm"},
{DHCPv6_MESSAGE_TYPE_RENEW, "Renew"},
{DHCPv6_MESSAGE_TYPE_REBIND, "Rebind"},
{DHCPv6_MESSAGE_TYPE_REPLY, "Reply"},
{DHCPv6_MESSAGE_TYPE_RELEASE, "Release"},
{DHCPv6_MESSAGE_TYPE_DECLINE, "Decline"},
{DHCPv6_MESSAGE_TYPE_RECONFIGURE, "Reconfigure"},
{DHCPv6_MESSAGE_TYPE_INFORMATION_REQUEST, "Information-Request"},
{DHCPv6_MESSAGE_TYPE_RELAY_FORW, "Relay-Forward"},
{DHCPv6_MESSAGE_TYPE_RELAY_REPL, "Relay-Reply"},
{DHCPv6_MESSAGE_TYPE_MALFORMED, "Malformed"}};
/* DHCPv6 counter name map */
std::map<int, std::string> counterMap = {
{DHCPv6_MESSAGE_TYPE_UNKNOWN, "Unknown"},
{DHCPv6_MESSAGE_TYPE_SOLICIT, "Solicit"},
{DHCPv6_MESSAGE_TYPE_ADVERTISE, "Advertise"},
{DHCPv6_MESSAGE_TYPE_REQUEST, "Request"},
{DHCPv6_MESSAGE_TYPE_CONFIRM, "Confirm"},
{DHCPv6_MESSAGE_TYPE_RENEW, "Renew"},
{DHCPv6_MESSAGE_TYPE_REBIND, "Rebind"},
{DHCPv6_MESSAGE_TYPE_REPLY, "Reply"},
{DHCPv6_MESSAGE_TYPE_RELEASE, "Release"},
{DHCPv6_MESSAGE_TYPE_DECLINE, "Decline"},
{DHCPv6_MESSAGE_TYPE_RECONFIGURE, "Reconfigure"},
{DHCPv6_MESSAGE_TYPE_INFORMATION_REQUEST, "Information-Request"},
{DHCPv6_MESSAGE_TYPE_RELAY_FORW, "Relay-Forward"},
{DHCPv6_MESSAGE_TYPE_RELAY_REPL, "Relay-Reply"},
{DHCPv6_MESSAGE_TYPE_MALFORMED, "Malformed"}
};

/* interface to vlan mapping */
std::unordered_map<std::string, std::string> vlan_map;
Expand Down Expand Up @@ -246,47 +247,47 @@ bool DHCPv6Msg::UnmarshalBinary(const uint8_t *packet, uint16_t len) {
}

/**
* @code initialize_counter(std::shared_ptr<swss::DBConnector> state_db, std::string counterVlan);
* @code initialize_counter(std::shared_ptr<swss::DBConnector> state_db, std::string &ifname);
*
* @brief initialize the counter by each Vlan
* @brief initialize the counter for interface
*
* @param std::shared_ptr<swss::DBConnector> state_db state_db connector pointer
* @param counterVlan counter table with interface name
* @param ifname interface name
*
* @return none
*/
void initialize_counter(std::shared_ptr<swss::DBConnector> state_db, std::string counterVlan) {
state_db->hset(counterVlan, "Unknown", toString(counters[DHCPv6_MESSAGE_TYPE_UNKNOWN]));
state_db->hset(counterVlan, "Solicit", toString(counters[DHCPv6_MESSAGE_TYPE_SOLICIT]));
state_db->hset(counterVlan, "Advertise", toString(counters[DHCPv6_MESSAGE_TYPE_ADVERTISE]));
state_db->hset(counterVlan, "Request", toString(counters[DHCPv6_MESSAGE_TYPE_REQUEST]));
state_db->hset(counterVlan, "Confirm", toString(counters[DHCPv6_MESSAGE_TYPE_CONFIRM]));
state_db->hset(counterVlan, "Renew", toString(counters[DHCPv6_MESSAGE_TYPE_RENEW]));
state_db->hset(counterVlan, "Rebind", toString(counters[DHCPv6_MESSAGE_TYPE_REBIND]));
state_db->hset(counterVlan, "Reply", toString(counters[DHCPv6_MESSAGE_TYPE_REPLY]));
state_db->hset(counterVlan, "Release", toString(counters[DHCPv6_MESSAGE_TYPE_RELEASE]));
state_db->hset(counterVlan, "Decline", toString(counters[DHCPv6_MESSAGE_TYPE_DECLINE]));
state_db->hset(counterVlan, "Reconfigure", toString(counters[DHCPv6_MESSAGE_TYPE_RECONFIGURE]));
state_db->hset(counterVlan, "Information-Request", toString(counters[DHCPv6_MESSAGE_TYPE_INFORMATION_REQUEST]));
state_db->hset(counterVlan, "Relay-Forward", toString(counters[DHCPv6_MESSAGE_TYPE_RELAY_FORW]));
state_db->hset(counterVlan, "Relay-Reply", toString(counters[DHCPv6_MESSAGE_TYPE_RELAY_REPL]));
state_db->hset(counterVlan, "Malformed", toString(counters[DHCPv6_MESSAGE_TYPE_MALFORMED]));
void initialize_counter(std::shared_ptr<swss::DBConnector> state_db, std::string &ifname) {
std::string table_name = counter_table + ifname;
for (auto &intr : counterMap) {
state_db->hset(table_name, intr.second, toString(0));
}
}

/**
* @code void increase_counter(std::shared_ptr<swss::DBConnector> state_db, std::string CounterVlan, uint8_t msg_type);
* @code void increase_counter(std::shared_ptr<swss::DBConnector> state_db, std::string &ifname, uint8_t msg_type);
*
* @brief increase the counter in state_db with count of each DHCPv6 message type
* @brief increase the counter in state_db with count of each DHCPv6 message types
*
* @param std::shared_ptr<swss::DBConnector> state_db, state_db connector pointer
* @param counterVlan counter table with interface name
* @param ifname interface name
* @param msg_type dhcpv6 message type to be increased in counter
*
* @return none
*/
void increase_counter(std::shared_ptr<swss::DBConnector> state_db, std::string counterVlan, uint8_t msg_type) {
counters[msg_type]++;
state_db->hset(counterVlan, counterMap.find(msg_type)->second, toString(counters[msg_type]));
void increase_counter(std::shared_ptr<swss::DBConnector> state_db, std::string &ifname, uint8_t msg_type) {
if (counterMap.find(msg_type) == counterMap.end()) {
syslog(LOG_WARNING, "Unexpected message type %d(0x%x)\n", msg_type, msg_type);
return;
}
std::string table_name = counter_table + ifname;
std::string type = counterMap.find(msg_type)->second;
auto count_str = state_db->hget(table_name, type);
if (count_str == nullptr) {
state_db->hset(table_name, type, toString(1));
} else {
auto count = atoll(count_str.get()->c_str());
state_db->hset(table_name, type, toString(count + 1));
}
}

/**
Expand Down Expand Up @@ -400,15 +401,6 @@ const struct dhcpv6_option *parse_dhcpv6_opt(const uint8_t *buffer, const uint8_
return option;
}

void sent_msg_increase_counter(relay_config *config, uint8_t msg_type) {
std::string counterVlan = counter_table;
if (counterMap.find(msg_type) != counterMap.end()) {
increase_counter(config->state_db, counterVlan.append(config->interface), msg_type);
} else {
syslog(LOG_WARNING, "Unexpected message type %d(0x%x)\n", msg_type, msg_type);
}
}

/**
* @code sock_open(const struct sock_fprog *fprog);
*
Expand Down Expand Up @@ -610,19 +602,17 @@ void prepare_socket(int &local_sock, int &server_sock, relay_config &config) {
* @return none
*/
void relay_client(int sock, const uint8_t *msg, uint16_t len, const ip6_hdr *ip_hdr, const ether_header *ether_hdr, relay_config *config) {
std::string counterVlan = counter_table;

/* unmarshal dhcpv6 message to detect malformed message */
DHCPv6Msg dhcpv6;
auto result = dhcpv6.UnmarshalBinary(msg, len);
if (!result) {
char addr_str[INET6_ADDRSTRLEN];
inet_ntop(AF_INET6, &ip_hdr->ip6_src, addr_str, INET6_ADDRSTRLEN);
increase_counter(config->state_db, counterVlan.append(config->interface), DHCPv6_MESSAGE_TYPE_MALFORMED);
increase_counter(config->state_db, config->interface, DHCPv6_MESSAGE_TYPE_MALFORMED);
syslog(LOG_WARNING, "DHCPv6 option is invalid or contains malformed payload from %s\n", addr_str);
return;
}
increase_counter(config->state_db, counterVlan.append(config->interface), dhcpv6.m_msg_hdr.msg_type);
increase_counter(config->state_db, config->interface, dhcpv6.m_msg_hdr.msg_type);

/* generate relay packet */
class RelayMsg relay;
Expand Down Expand Up @@ -658,7 +648,7 @@ void relay_client(int sock, const uint8_t *msg, uint16_t len, const ip6_hdr *ip_

for(auto server: config->servers_sock) {
if(send_udp(sock, relay_pkt, server, relay_pkt_len)) {
sent_msg_increase_counter(config, DHCPv6_MESSAGE_TYPE_RELAY_FORW);
increase_counter(config->state_db, config->interface, DHCPv6_MESSAGE_TYPE_RELAY_FORW);
}
}
}
Expand Down Expand Up @@ -707,7 +697,7 @@ void relay_relay_forw(int sock, const uint8_t *msg, int32_t len, const ip6_hdr *

for(auto server: config->servers_sock) {
if(send_udp(sock, send_buffer, server, send_buffer_len)) {
sent_msg_increase_counter(config, DHCPv6_MESSAGE_TYPE_RELAY_FORW);
increase_counter(config->state_db, config->interface, DHCPv6_MESSAGE_TYPE_RELAY_FORW);
}
}
}
Expand All @@ -725,19 +715,17 @@ void relay_relay_forw(int sock, const uint8_t *msg, int32_t len, const ip6_hdr *
* @return none
*/
void relay_relay_reply(int sock, const uint8_t *msg, int32_t len, relay_config *config) {
std::string counterVlan = counter_table;

class RelayMsg relay;
auto result = relay.UnmarshalBinary(msg, len);
if (!result) {
increase_counter(config->state_db, counterVlan.append(config->interface), DHCPv6_MESSAGE_TYPE_MALFORMED);
increase_counter(config->state_db, config->interface, DHCPv6_MESSAGE_TYPE_MALFORMED);
syslog(LOG_WARNING, "Relay-reply option is invalid or contains malformed payload\n");
return;
}

auto opt_value = relay.m_option_list.Get(OPTION_RELAY_MSG);
if (opt_value.empty()) {
increase_counter(config->state_db, counterVlan.append(config->interface), DHCPv6_MESSAGE_TYPE_UNKNOWN);
increase_counter(config->state_db, config->interface, DHCPv6_MESSAGE_TYPE_UNKNOWN);
syslog(LOG_WARNING, "Option relay-msg not found");
return;
}
Expand All @@ -753,7 +741,7 @@ void relay_relay_forw(int sock, const uint8_t *msg, int32_t len, const ip6_hdr *
target_addr.sin6_scope_id = if_nametoindex(config->interface.c_str());

if(send_udp(sock, dhcpv6, target_addr, length)) {
sent_msg_increase_counter(config, msg_type);
increase_counter(config->state_db, config->interface, msg_type);
}
}

Expand Down Expand Up @@ -851,7 +839,6 @@ void client_packet_handler(uint8_t *buffer, ssize_t length, struct relay_config
auto buffer_end = buffer + length;
const uint8_t *current_position = buffer;
const uint8_t *prev = NULL;
std::string counterVlan = counter_table;

auto ether_header = parse_ether_frame(current_position, &current_position);
auto ip6_header = parse_ip6_hdr(current_position, &current_position);
Expand Down Expand Up @@ -881,7 +868,7 @@ void client_packet_handler(uint8_t *buffer, ssize_t length, struct relay_config
auto msg = parse_dhcpv6_hdr(current_position);
// RFC3315 only
if (msg->msg_type < DHCPv6_MESSAGE_TYPE_SOLICIT || msg->msg_type > DHCPv6_MESSAGE_TYPE_RELAY_REPL) {
increase_counter(config->state_db, counterVlan.append(config->interface), DHCPv6_MESSAGE_TYPE_UNKNOWN);
increase_counter(config->state_db, config->interface, DHCPv6_MESSAGE_TYPE_UNKNOWN);
syslog(LOG_WARNING, "Unknown DHCPv6 message type %d from %s\n", msg->msg_type, ifname.c_str());
return;
}
Expand Down Expand Up @@ -931,7 +918,6 @@ void server_callback(evutil_socket_t fd, short event, void *arg) {
int32_t pkts_num = 0;

while (pkts_num++ < BATCH_SIZE) {
std::string counterVlan = counter_table;
auto buffer_sz = recvfrom(config->local_sock, server_recv_buffer, BUFFER_SIZE, 0, (sockaddr *)&from, &len);
if (buffer_sz <= 0) {
if (errno != EAGAIN) {
Expand All @@ -948,12 +934,12 @@ void server_callback(evutil_socket_t fd, short event, void *arg) {
auto msg_type = parse_dhcpv6_hdr(server_recv_buffer)->msg_type;
// RFC3315 only
if (msg_type < DHCPv6_MESSAGE_TYPE_SOLICIT || msg_type > DHCPv6_MESSAGE_TYPE_RELAY_REPL) {
increase_counter(config->state_db, counterVlan.append(config->interface), DHCPv6_MESSAGE_TYPE_UNKNOWN);
increase_counter(config->state_db, config->interface, DHCPv6_MESSAGE_TYPE_UNKNOWN);
syslog(LOG_WARNING, "Unknown DHCPv6 message type %d\n", msg_type);
continue;
}

increase_counter(config->state_db, counterVlan.append(config->interface), msg_type);
increase_counter(config->state_db, config->interface, msg_type);
if (msg_type == DHCPv6_MESSAGE_TYPE_RELAY_REPL) {
relay_relay_reply(config->server_sock, server_recv_buffer, buffer_sz, config);
}
Expand Down Expand Up @@ -1089,8 +1075,7 @@ void loop_relay(std::unordered_map<std::string, relay_config> &vlans) {

update_vlan_mapping(vlan.first, config_db);

std::string counterVlan = counter_table;
initialize_counter(vlan.second.state_db, counterVlan.append(vlan.second.interface));
initialize_counter(vlan.second.state_db, vlan.second.interface);
Copy link
Contributor

Choose a reason for hiding this comment

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

Changes look good, i also suggest clearing out the entire COUNTERS Table, during init. That'll fix this issue as well https://github.com/sonic-net/sonic-buildimage/issues/15047

Copy link
Contributor

Choose a reason for hiding this comment

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

sonic-net/sonic-buildimage#15047

Looks like the issue is handled by sonic-net/sonic-utilities#2852, not required to clear DHCPv6_COUNTERS table here. #Resolved

prepare_socket(local_sock, server_sock, vlan.second);

vlan.second.local_sock = local_sock;
Expand Down
14 changes: 7 additions & 7 deletions src/relay.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,29 +288,29 @@ void signal_callback(evutil_socket_t fd, short event, void *arg);
void shutdown_relay();

/**
* @code void initialize_counter(std::shared_ptr<swss::Table> state_db, std::string counterVlan);
* @code void initialize_counter(std::shared_ptr<swss::Table> state_db, std::string &ifname);
*
* @brief initialize the counter by each Vlan
* @brief initialize the counter for interface
*
* @param std::shared_ptr<swss::Table> state_db state_db connector
* @param counterVlan counter table with interface name
* @param ifname interface name
*
* @return none
*/
void initialize_counter(std::shared_ptr<swss::DBConnector> state_db, std::string counterVlan);
void initialize_counter(std::shared_ptr<swss::DBConnector> state_db, std::string &ifname);

/**
* @code void increase_counter(shared_ptr<swss::DBConnector>, std::string CounterVlan, uint8_t msg_type);
* @code void increase_counter(shared_ptr<swss::DBConnector>, std::string ifname, uint8_t msg_type);
*
* @brief increase the counter in state_db with count of each DHCPv6 message type
*
* @param shared_ptr<swss::DBConnector> state_db state_db connector
* @param counterVlan counter table with interface name
* @param ifname interface name
* @param msg_type dhcpv6 message type to be increased in counter
*
* @return none
*/
void increase_counter(std::shared_ptr<swss::DBConnector> state_db, std::string counterVlan, uint8_t msg_type);
void increase_counter(std::shared_ptr<swss::DBConnector> state_db, std::string &ifname, uint8_t msg_type);

/* Helper functions */

Expand Down
15 changes: 10 additions & 5 deletions test/mock_relay.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,8 @@ TEST(prepareConfig, prepare_socket)
TEST(counter, initialize_counter)
{
std::shared_ptr<swss::DBConnector> state_db = std::make_shared<swss::DBConnector> ("STATE_DB", 0);
initialize_counter(state_db, "DHCPv6_COUNTER_TABLE|Vlan1000");
std::string ifname = "Vlan1000";
initialize_counter(state_db, ifname);
EXPECT_TRUE(state_db->hexists("DHCPv6_COUNTER_TABLE|Vlan1000", "Unknown"));
EXPECT_TRUE(state_db->hexists("DHCPv6_COUNTER_TABLE|Vlan1000", "Solicit"));
EXPECT_TRUE(state_db->hexists("DHCPv6_COUNTER_TABLE|Vlan1000", "Advertise"));
Expand All @@ -315,7 +316,8 @@ TEST(counter, increase_counter)
{
std::shared_ptr<swss::DBConnector> state_db = std::make_shared<swss::DBConnector> ("STATE_DB", 0);
state_db->hset("DHCPv6_COUNTER_TABLE|Vlan1000", "Solicit", "0");
increase_counter(state_db, "DHCPv6_COUNTER_TABLE|Vlan1000", 1);
std::string ifname = "Vlan1000";
increase_counter(state_db, ifname, 1);
std::shared_ptr<std::string> output = state_db->hget("DHCPv6_COUNTER_TABLE|Vlan1000", "Solicit");
std::string *ptr = output.get();
EXPECT_EQ(*ptr, "1");
Expand Down Expand Up @@ -632,7 +634,8 @@ TEST(relay, update_vlan_mapping) {

TEST(relay, client_packet_handler) {
std::shared_ptr<swss::DBConnector> state_db = std::make_shared<swss::DBConnector> ("STATE_DB", 0);
initialize_counter(state_db, "DHCPv6_COUNTER_TABLE|Vlan1000");
std::string vlan_name = "Vlan1000";
initialize_counter(state_db, vlan_name);

struct relay_config config{};
config.is_option_79 = true;
Expand Down Expand Up @@ -707,7 +710,8 @@ MOCK_GLOBAL_FUNC6(recvfrom, ssize_t(int, void *, size_t, int, struct sockaddr *,

TEST(relay, server_callback) {
std::shared_ptr<swss::DBConnector> state_db = std::make_shared<swss::DBConnector> ("STATE_DB", 0);
initialize_counter(state_db, "DHCPv6_COUNTER_TABLE|Vlan1000");
std::string ifname = "Vlan1000";
initialize_counter(state_db, ifname);

struct relay_config config{};
config.is_option_79 = true;
Expand Down Expand Up @@ -738,7 +742,8 @@ TEST(relay, client_callback) {
std::shared_ptr<swss::Table> mux_table = std::make_shared<swss::Table> (
state_db.get(), "HW_MUX_CABLE_TABLE"
);
initialize_counter(state_db, "DHCPv6_COUNTER_TABLE|Vlan1000");
std::string ifname = "Vlan1000";
initialize_counter(state_db, ifname);

struct relay_config config{};
config.is_option_79 = true;
Expand Down