[ISSUE] Failed to find "enabled" option.
by Xin Long
I found this err in "loadbalance" and "lacp" runner when adding ports.
It's caused by trying to set "enabled" option in .port_link_changed()
or .port_changed().
When a new port is added, the first 'port changed event' process is
earlier than CMD TEAM_CMD_OPTIONS_GET, in this CMD, all
the options are synchronized from kernel.
It means there's no 'enabled' option yet when calling port_link_changed
in the first 'port changed event' process. In lb_event_watch_port_link_changed
and lacp_event_watch_port_changed, they call teamd_port_check_enable
to set 'enabled' option. this err is triggered.
I'm not sure why teamd_port_check_enable needs to check if
'enabled' option exists. I checked the ab's .port_link_changed(),
it just sets it by calling team_set_port_enabled(), instead of
checking 'enabled' option first.
can we just use team_set_port_enabled to set it directly in
.port(_link)_changed OR improve teamd_port_check_enable
to avoid this err ?
Thanks.
4 years, 2 months
[PATCH] teamd: add port_master_ifindex_changed for teamd_event_watch_ops
by Hangbin Liu
When multi interfaces up, there is a possibility that the link states
notification came later after interface up, which cause the master_ifindex
not update timely. Then the teamd_port_present() checking in
ab_link_watch_handler() -> teamd_for_each_tdport() -> teamd_get_next_tdport()
will failed and we will not set the active port.
Fix it by adding a new teamd_event_watch_ops port_master_ifindex_changed for
ab mode.
Reproducer:
\#/bin/bash
WAIT=2
COUNT=0
start_team()
{
num=$1
teamd -o -n -U -d -t team$num -c '{"runner": {"name": "activebackup"},"link_watch": {"name": "ethtool"}}' -gg
teamdctl team$num port add eth$num
}
while :; do
echo "-----------------------------------------------------------"
let "COUNT++"
echo "Loop $COUNT"
if teamdctl team1 state | grep -q "active port: eth1" && \
teamdctl team2 state | grep -q "active port: eth2"; then
echo "Pass"
else
echo "FAIL"
exit 1
fi
teamd -k -t team1
teamd -k -t team2
sleep "$WAIT"
start_team 1 &
start_team 2 &
sleep "$WAIT"
done
Result: Usually in 10 rounds, we will get the failure,
\# teamdctl team1 state
setup:
runner: activebackup
ports:
eth1
link watches:
link summary: up
instance[link_watch_0]:
name: ethtool
link: up
down count: 0
runner:
active port:
Signed-off-by: Hangbin Liu <liuhangbin(a)gmail.com>
---
teamd/teamd.h | 5 +++++
teamd/teamd_events.c | 19 +++++++++++++++++++
teamd/teamd_ifinfo_watch.c | 5 +++++
teamd/teamd_runner_activebackup.c | 8 ++++++++
4 files changed, 37 insertions(+)
diff --git a/teamd/teamd.h b/teamd/teamd.h
index 5dbfb9b..3934fc2 100644
--- a/teamd/teamd.h
+++ b/teamd/teamd.h
@@ -189,6 +189,9 @@ struct teamd_event_watch_ops {
struct teamd_port *tdport, void *priv);
int (*port_ifname_changed)(struct teamd_context *ctx,
struct teamd_port *tdport, void *priv);
+ int (*port_master_ifindex_changed)(struct teamd_context *ctx,
+ struct teamd_port *tdport,
+ void *priv);
int (*option_changed)(struct teamd_context *ctx,
struct team_option *option, void *priv);
char *option_changed_match_name;
@@ -208,6 +211,8 @@ int teamd_event_ifinfo_hwaddr_changed(struct teamd_context *ctx,
struct team_ifinfo *ifinfo);
int teamd_event_ifinfo_ifname_changed(struct teamd_context *ctx,
struct team_ifinfo *ifinfo);
+int teamd_event_ifinfo_master_ifindex_changed(struct teamd_context *ctx,
+ struct team_ifinfo *ifinfo);
int teamd_event_ifinfo_admin_state_changed(struct teamd_context *ctx,
struct team_ifinfo *ifinfo);
int teamd_events_init(struct teamd_context *ctx);
diff --git a/teamd/teamd_events.c b/teamd/teamd_events.c
index 1a95974..65aa46a 100644
--- a/teamd/teamd_events.c
+++ b/teamd/teamd_events.c
@@ -167,6 +167,25 @@ int teamd_event_ifinfo_ifname_changed(struct teamd_context *ctx,
return 0;
}
+int teamd_event_ifinfo_master_ifindex_changed(struct teamd_context *ctx,
+ struct team_ifinfo *ifinfo)
+{
+ struct event_watch_item *watch;
+ uint32_t ifindex = team_get_ifinfo_ifindex(ifinfo);
+ struct teamd_port *tdport = teamd_get_port(ctx, ifindex);
+ int err;
+
+ list_for_each_node_entry(watch, &ctx->event_watch_list, list) {
+ if (watch->ops->port_master_ifindex_changed && tdport) {
+ err = watch->ops->port_master_ifindex_changed(ctx, tdport,
+ watch->priv);
+ if (err)
+ return err;
+ }
+ }
+ return 0;
+}
+
int teamd_event_ifinfo_admin_state_changed(struct teamd_context *ctx,
struct team_ifinfo *ifinfo)
{
diff --git a/teamd/teamd_ifinfo_watch.c b/teamd/teamd_ifinfo_watch.c
index f334ff6..6a19532 100644
--- a/teamd/teamd_ifinfo_watch.c
+++ b/teamd/teamd_ifinfo_watch.c
@@ -59,6 +59,11 @@ static int ifinfo_change_handler_func(struct team_handle *th, void *priv,
if (err)
return err;
}
+ if (team_is_ifinfo_master_ifindex_changed(ifinfo)) {
+ err = teamd_event_ifinfo_master_ifindex_changed(ctx, ifinfo);
+ if (err)
+ return err;
+ }
}
return 0;
}
diff --git a/teamd/teamd_runner_activebackup.c b/teamd/teamd_runner_activebackup.c
index 8a3447f..f92d341 100644
--- a/teamd/teamd_runner_activebackup.c
+++ b/teamd/teamd_runner_activebackup.c
@@ -520,6 +520,13 @@ static int ab_event_watch_port_link_changed(struct teamd_context *ctx,
return ab_link_watch_handler(ctx, priv);
}
+static int ab_event_watch_port_master_ifindex_changed(struct teamd_context *ctx,
+ struct teamd_port *tdport,
+ void *priv)
+{
+ return ab_link_watch_handler(ctx, priv);
+}
+
static int ab_event_watch_prio_option_changed(struct teamd_context *ctx,
struct team_option *option,
void *priv)
@@ -532,6 +539,7 @@ static const struct teamd_event_watch_ops ab_event_watch_ops = {
.port_hwaddr_changed = ab_event_watch_port_hwaddr_changed,
.port_added = ab_event_watch_port_added,
.port_link_changed = ab_event_watch_port_link_changed,
+ .port_master_ifindex_changed = ab_event_watch_port_master_ifindex_changed,
.option_changed = ab_event_watch_prio_option_changed,
.option_changed_match_name = "priority",
};
--
2.5.5
5 years, 3 months
[PATCHv2] utils: check to_stdout return correctly in bond2team
by Xin Long
to_stdout is a function, not a string, so fix the check on
its return in bond2team.
v1->v2:
improve the coding style as Flavio suggested.
Fixes: d5a1c8ee9e36 ("utils: add bond2team conversion tool")
Signed-off-by: Xin Long <lucien.xin(a)gmail.com>
---
utils/bond2team | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/utils/bond2team b/utils/bond2team
index f8d46ef..fc81c4b 100755
--- a/utils/bond2team
+++ b/utils/bond2team
@@ -623,7 +623,7 @@ team_ifcfg_write()
team_ifcfg_deliver()
{
pr_dbg "${FUNCNAME} $*"
- if [ ! to_stdout ]; then
+ if ! to_stdout; then
return 0
fi
--
2.1.0
5 years, 4 months
[PATCH] binding/python: use SWIG_FromCharPtrAndSize for Python3 support
by Xin Long
PyString_FromStringAndSize is replaced with PyUnicode_FromStringAndSize
in Python3, and SWIG_FromCharPtrAndSize will choose the right one with
the check "#if PY_VERSION_HEX >= 0x0300000".
Fixes: 4cb7829debd7 ("add support for hw address manipulation")
Signed-off-by: Xin Long <lucien.xin(a)gmail.com>
---
binding/python/team/capi.i.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/binding/python/team/capi.i.in b/binding/python/team/capi.i.in
index 5ce4659..c7a5842 100644
--- a/binding/python/team/capi.i.in
+++ b/binding/python/team/capi.i.in
@@ -40,7 +40,7 @@ int team_hwaddr_set(struct team_handle *th, uint32_t ifindex, const char *addr,
if ($1) free($1);
}
%typemap(argout) (char *addr, unsigned int addr_len) {
- $result = SWIG_Python_AppendOutput($result, PyString_FromStringAndSize($1,$2));
+ $result = SWIG_Python_AppendOutput($result, SWIG_FromCharPtrAndSize($1,$2));
}
%apply char *OUTPUT {char *addr};
int team_hwaddr_get(struct team_handle *th, uint32_t ifindex, char *addr, unsigned int addr_len);
--
2.1.0
5 years, 4 months
Re: [PATCH] utils: check to_stdout return correctly in bond2team
by Xin Long
On Wed, Jul 25, 2018 at 9:51 PM, Flavio Leitner <fbl(a)redhat.com> wrote:
> On Wed, Jul 25, 2018 at 04:24:40PM +0800, Xin Long wrote:
>> to_stdout is a function, not a string, so fix the check on
>> its return in bond2team.
>>
>> Fixes: d5a1c8ee9e36 ("utils: add bond2team conversion tool")
>> Signed-off-by: Xin Long <lucien.xin(a)gmail.com>
>> ---
>> utils/bond2team | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/utils/bond2team b/utils/bond2team
>> index f8d46ef..1a3d7d6 100755
>> --- a/utils/bond2team
>> +++ b/utils/bond2team
>> @@ -623,9 +623,7 @@ team_ifcfg_write()
>> team_ifcfg_deliver()
>> {
>> pr_dbg "${FUNCNAME} $*"
>> - if [ ! to_stdout ]; then
>> - return 0
>> - fi
>> + to_stdout || return 0
>>
>> if [ -z "${OUTPUT_DIR}" ]; then
>> show_output_files
>
> Although that fixes the problem, it's not the coding style.
> Would this work for you?
Sure, will post v2.
>
> diff --git a/utils/bond2team b/utils/bond2team
> index f8d46ef..fc81c4b 100755
> --- a/utils/bond2team
> +++ b/utils/bond2team
> @@ -623,7 +623,7 @@ team_ifcfg_write()
> team_ifcfg_deliver()
> {
> pr_dbg "${FUNCNAME} $*"
> - if [ ! to_stdout ]; then
> + if ! to_stdout; then
> return 0
> fi
>
>
> --
> Flavio
5 years, 4 months
[PATCH] utils: check to_stdout return correctly in bond2team
by Xin Long
to_stdout is a function, not a string, so fix the check on
its return in bond2team.
Fixes: d5a1c8ee9e36 ("utils: add bond2team conversion tool")
Signed-off-by: Xin Long <lucien.xin(a)gmail.com>
---
utils/bond2team | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/utils/bond2team b/utils/bond2team
index f8d46ef..1a3d7d6 100755
--- a/utils/bond2team
+++ b/utils/bond2team
@@ -623,9 +623,7 @@ team_ifcfg_write()
team_ifcfg_deliver()
{
pr_dbg "${FUNCNAME} $*"
- if [ ! to_stdout ]; then
- return 0
- fi
+ to_stdout || return 0
if [ -z "${OUTPUT_DIR}" ]; then
show_output_files
--
2.1.0
5 years, 4 months