diff mbox series

Fix ifinfo_link_with_port race condition with newlink

Message ID 1583952979-583-1-git-send-email-pavel.contrib@gmail.com (mailing list archive)
State New
Headers show
Series Fix ifinfo_link_with_port race condition with newlink | expand

Commit Message

Pavel Shirshov March 11, 2020, 6:56 p.m. UTC
The race condition could happen like this:
When an interface is enslaved into the port channel
immediately after it is created, the order of creating
the ifinfo and linking the ifinfo to the port is not
guaranteed.

The team handler will listen to both netlink message
to track new links get created to allocate the ifinfo
and add the ifinfo into its linked list, and the team
port change message to link the new port with ifinfo
found in its linkedin list. However, when the ifinfo
is not yet created, the error message "Failed to link
port with ifinfo" is thrown with member port failed
to be added into the team handler's port list.

This fix adds a condition to check if
ifinfo_link_with_port is linking ifinfo to a port or
to the team interface itself. If it is a port,
ifinfo_find_create function is used to fix the race
condition.

Signed-off-by: Shu0T1an ChenG <shuche@microsoft.com>
---
 libteam/ifinfo.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Jiri Pirko March 12, 2020, 5:40 p.m. UTC | #1
Wed, Mar 11, 2020 at 07:56:19PM CET, pavel.contrib@gmail.com wrote:
>The race condition could happen like this:
>When an interface is enslaved into the port channel
>immediately after it is created, the order of creating
>the ifinfo and linking the ifinfo to the port is not
>guaranteed.
>
>The team handler will listen to both netlink message
>to track new links get created to allocate the ifinfo
>and add the ifinfo into its linked list, and the team
>port change message to link the new port with ifinfo
>found in its linkedin list. However, when the ifinfo
>is not yet created, the error message "Failed to link
>port with ifinfo" is thrown with member port failed
>to be added into the team handler's port list.
>
>This fix adds a condition to check if

Again, please tell the codebase what to do:
"Fix this by ...." "Do this" "Fix that" etc.


>ifinfo_link_with_port is linking ifinfo to a port or
>to the team interface itself. If it is a port,
>ifinfo_find_create function is used to fix the race
>condition.
>
>Signed-off-by: Shu0T1an ChenG <shuche@microsoft.com>

Please fix the name not to contain capital letters in the middle and 0
and 1.

Also, is this the author of the patch? If yes, the From: additional
header should be filled (auto by git send-email when author is different
from the sender).

Also, add your signed off by line.



>---
> libteam/ifinfo.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
>diff --git a/libteam/ifinfo.c b/libteam/ifinfo.c
>index 46d56a2..a15788b 100644
>--- a/libteam/ifinfo.c
>+++ b/libteam/ifinfo.c
>@@ -453,7 +453,10 @@ int ifinfo_link_with_port(struct team_handle *th, uint32_t ifindex,
> {
> 	struct team_ifinfo *ifinfo;
> 
>-	ifinfo = ifinfo_find(th, ifindex);
>+	if (port)
>+		ifinfo = ifinfo_find_create(th, ifindex);
>+	else
>+		ifinfo = ifinfo_find(th, ifindex);
> 	if (!ifinfo)
> 		return -ENOENT;
> 	if (ifinfo->linked)
>-- 
>2.7.4
>_______________________________________________
>libteam mailing list -- libteam@lists.fedorahosted.org
>To unsubscribe send an email to libteam-leave@lists.fedorahosted.org
>Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
>List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
>List Archives: https://lists.fedorahosted.org/archives/list/libteam@lists.fedorahosted.org
diff mbox series

Patch

diff --git a/libteam/ifinfo.c b/libteam/ifinfo.c
index 46d56a2..a15788b 100644
--- a/libteam/ifinfo.c
+++ b/libteam/ifinfo.c
@@ -453,7 +453,10 @@  int ifinfo_link_with_port(struct team_handle *th, uint32_t ifindex,
 {
 	struct team_ifinfo *ifinfo;
 
-	ifinfo = ifinfo_find(th, ifindex);
+	if (port)
+		ifinfo = ifinfo_find_create(th, ifindex);
+	else
+		ifinfo = ifinfo_find(th, ifindex);
 	if (!ifinfo)
 		return -ENOENT;
 	if (ifinfo->linked)