issue: 4023262 Handle down interface states during table update #215
+29
−7
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BasharRadya
force-pushed
the
handle_down_interface
branch
from
77b5c5e
to
ac3349d
Compare
|
bot:retest |
BasharRadya
force-pushed
the
handle_down_interface
branch
2 times, most recently
from
d7ac05c
to
82e3e5b
Compare
AlexanderGrissik
force-pushed
the
vNext
branch
from
cfce2e8
to
51c2340
Compare
BasharRadya
force-pushed
the
handle_down_interface
branch
2 times, most recently
from
a61d143
to
a17b2b2
Compare
BasharRadya
force-pushed
the
handle_down_interface
branch
from
a17b2b2
to
5e5f92b
Compare
BasharRadya
force-pushed
the
handle_down_interface
branch
2 times, most recently
from
2e15ca1
to
4230b33
Compare
Refactored ip_addr move constructor to use std::forward for improved efficiency and move semantics. Signed-off-by: Bashar Abdelgafer <[email protected]>
BasharRadya
force-pushed
the
handle_down_interface
branch
2 times, most recently
from
aed5039
to
986ac8f
Compare
pasis
reviewed
Aug 30, 2024
| @@ -264,19 +264,32 @@ void net_device_table_mgr::update_tbl() | |||
| if ((int)get_max_mtu() < p_net_device_val->get_mtu()) { | |||
| set_max_mtu(p_net_device_val->get_mtu()); | |||
| } | |||
| auto handle_ip_insertion = [p_net_device_val](const std::unique_ptr<ip_data> &ip, | |||
| auto &net_device_map, | |||
There was a problem hiding this comment.
Please replace auto with real type here. Keep auto for lambda and iterator. Motivation: this will simplify reading (what we do more frequent than writing) and allow to use ctags easier.
Comment on lines
275
to
279
| if (it != net_device_map.end()) { | ||
| vlog_printf(VLOG_WARNING, | ||
| "IP ADDRESS %s ASSIGNED TO MULTIPLE INTERFACES NOT SUPPORTED\n", | ||
| ip->local_addr.to_str(family).c_str()); | ||
| } |
There was a problem hiding this comment.
This approach can print multiple extra messages (per problematic interface). This is OK since this is unlikely scenario. We still can consider choosing among the following options:
- Print a message per problematic interface as is. I'd prefer to move the if block at the top of the lambda not to think about iterator invalidation (the previous block manipulates with the collection).
- Remember the fact that there is at least one duplicate IP and print a single common warning message at the end.
- Create a map on stack with key=IP value=net_device_val and print a single message per IP at the end with mentioning which interface will be used for each IP.
Also rewrite the warning message:
- Don't use upper case
- Write more "light" message, because XLIO will work as expected in multiple scenarios even with duplicate IP addresses, and the current message makes impression that XLIO won't work at all. Use something like "Duplicate IP address %s is detected on multiple interfaces. XLIO will work with a single interface".
BasharRadya
force-pushed
the
handle_down_interface
branch
2 times, most recently
from
bfa6a1d
to
3d0141a
Compare
BasharRadya
force-pushed
the
handle_down_interface
branch
from
3d0141a
to
8860b0c
Compare
pasis
approved these changes
Sep 23, 2024
|
bot:retest |
Detects when the same IP is assigned to more than one interface. Prioritizes the "Up" interface if a conflict occurs. Prints a warning since the current implementation only supports one interface per IP. Signed-off-by: Bashar Abdelgafer <[email protected]>
BasharRadya
force-pushed
the
handle_down_interface
branch
from
8860b0c
to
1a8f82e
Compare
|
bot:retest |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In this commit, we address a scenario where more than one network interface is assigned the same IP address. The current implementation supports creating RX rules for only one interface per IP address, which can lead to issues if multiple interfaces share the same IP.
Key Changes:
Added a mechanism to detect when the same IP address is assigned to multiple interfaces.
If this conflict occurs, a warning message is printed to notify the user that this configuration is not supported.
When faced with this conflict, the implementation now prioritizes the interface that is in the "Up" state, assigning the RX rules to it.
This change helps prevent potential conflicts and ensures that the most relevant (active) interface is used when an IP address conflict occurs.
Change type
What kind of change does this PR introduce?
Check list