-
Notifications
You must be signed in to change notification settings - Fork 18
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
Fix multiple vlan issue #27
Conversation
syslog(LOG_ERR, "setsockopt: Failed to attach filter\n"); | ||
(void) close(s); | ||
return -1; | ||
} | ||
|
||
int optval = 0; |
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.
Adjust Indent
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.
Thanks for comments, will apply in latest diff.
} | ||
event_add(listen_event, NULL); | ||
syslog(LOG_INFO, "libevent: add filter socket event\n"); | ||
} |
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.
If the sock_open fails for some fatal reason, shouldn't the process just abort/exit? I'd suggest having different return values for different severity. For Eg: if socket create/bind fails, please exit/abort. If the setting buffer fails, then don't exit and continue.
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.
Sure, good suggestion, will apply in next diff.
src/relay.cpp
Outdated
void callback(evutil_socket_t fd, short event, void *arg) { | ||
struct relay_config *config = (struct relay_config *)arg; | ||
void client_callback(evutil_socket_t fd, short event, void *arg) { | ||
auto vlans = *(reinterpret_cast<std::unordered_map<std::string, struct relay_config> *>(arg)); |
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.
This will create a copy of the map. Since the callback is invoked frequently, i think it's better to not copy the structure every time. I'd suggest casting it to an unordered_map <> * and use the pointer throughout the method.
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.
Thanks for the good catch, will apply in next diff.
# This is the 1st commit message: fix multi-vlan relay issue # This is the commit message sonic-net#2: issue fix # This is the commit message sonic-net#3: code refine in UT # This is the commit message sonic-net#4: fix counter rollback issue # This is the commit message sonic-net#5: Use github code scanning instead of LGTM (sonic-net#26) * Use github code scanning instead of LGTM * fi libyang libyang1 * remove libyang1 * add python * add libpython
40182a4
to
c139d52
Compare
listen_event = event_new(base, filter, EV_READ|EV_PERSIST, callback, config); | ||
} | ||
server_listen_event = event_new(base, local_sock, EV_READ|EV_PERSIST, server_callback, config); | ||
server_listen_event = event_new(base, local_sock, EV_READ|EV_PERSIST, server_callback, &(vlan.second)); |
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.
Indentation issue here
This commit could not be cleanly cherry-picked to 202012. Please submit another PR. |
* # This is a combination of 5 commits. # This is the 1st commit message: fix multi-vlan relay issue
* # This is a combination of 5 commits. # This is the 1st commit message: fix multi-vlan relay issue
Description of PR
Fix dhcpv6 relay multiple VLAN issue.
Summary:
Fixes #12805
sonic-net/sonic-buildimage#12805
Type of change
Back port request
Approach
What is the motivation for this PR?
Fix multiple vlan issue introduced by dual-tor support commit.
How did you do it?
Not covered:
dhcp6relay support vlan member interface dynamic changing will be applied in another PR.
How did you verify/test it?
Test on single tor and dual tor DUT.
Any platform specific information?
Supported testbed topology if it's a new test case?
Multiple vlan topo
Documentation