Skip to contentSkip to conversation

cman: Fix udpb handling in cman-preconfig

PATCH New Lon Hohberger 3 participants 5 messages Jan 3, 2011 – Jan 4, 2011

lore
Share & tools
lore

Lon Hohberger

author
#0 lore raw

PATCH 1/1 cman: Fix udpb handling in cman-preconfig

Signed-off-by Lon Hohberger <[email protected]>
---
 cman/daemon/cman-preconfig.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
cman/daemon/cman-preconfig.c +1 -1
index 5239d89..5d608a7 100644
@@ -286,7 +286,7 @@ static int add_ifaddr(struct objdb_iface_ver0 *objdb, char *mcast, char *ifaddr,
 	int ret = 0;
 	const char *tx_mech_to_str[] = {
 		[TX_MECH_UDP] = "udp",
-		[TX_MECH_UDPB] = "udp",
+		[TX_MECH_UDPB] = "udpb",
 		[TX_MECH_UDPU] = "udpu",
 		[TX_MECH_RDMA] = "iba",
 	};
Signature · 1 line
1.7.2.3
#1 lore raw

PATCH 1/1 cman: Fix udpb handling in cman-preconfig

Lon,
even it may "logically" seemed that original code is incorrect and your patch make sense, it's not the case.

Actually, corosync has 3 transports:
udp, udpu and iba.

Broadcast is same transport as udp but only has broadcast flag set on.

In other words, this patch is not correct.

Regards,
Honza

Quoted 18 lines · Lon Hohberger
Signed-off-by Lon Hohberger <[email protected]>
---
 cman/daemon/cman-preconfig.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
cman/daemon/cman-preconfig.c +1 -1
index 5239d89..5d608a7 100644
@@ -286,7 +286,7 @@ static int add_ifaddr(struct objdb_iface_ver0 *objdb, char *mcast, char *ifaddr,
 	int ret = 0;
 	const char *tx_mech_to_str[] = {
 		[TX_MECH_UDP] = "udp",
-		[TX_MECH_UDPB] = "udp",
+		[TX_MECH_UDPB] = "udpb",
 		[TX_MECH_UDPU] = "udpu",
 		[TX_MECH_RDMA] = "iba",
 	};
#2 lore raw

PATCH 1/1 cman: Fix udpb handling in cman-preconfig

Quoted 11 lines · Jan Friesse

Lon,
even it may "logically" seemed that original code is incorrect and your patch make sense, it's not the case.

Actually, corosync has 3 transports:
udp, udpu and iba.

Broadcast is same transport as udp but only has broadcast flag set on.

In other words, this patch is not correct.

So in order to configure broadcast we still need to use:

<cman transport="udp" broadcast="yes"/> ?

I find that redundant tho...

Cheers
Fabio

Quoted 23 lines

Regards,
Honza

Quoted 18 lines · Lon Hohberger
Signed-off-by Lon Hohberger <[email protected]>
---
 cman/daemon/cman-preconfig.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
cman/daemon/cman-preconfig.c +1 -1
index 5239d89..5d608a7 100644
@@ -286,7 +286,7 @@ static int add_ifaddr(struct objdb_iface_ver0 *objdb, char *mcast, char *ifaddr,
 	int ret = 0;
 	const char *tx_mech_to_str[] = {
 		[TX_MECH_UDP] = "udp",
-		[TX_MECH_UDPB] = "udp",
+		[TX_MECH_UDPB] = "udpb",
 		[TX_MECH_UDPU] = "udpu",
 		[TX_MECH_RDMA] = "iba",
 	};
#3 lore raw

PATCH 1/1 cman: Fix udpb handling in cman-preconfig

Quoted 13 lines · Fabio M. Di Nitto
Quoted 11 lines · Jan Friesse

Lon,
even it may "logically" seemed that original code is incorrect and your patch make sense, it's not the case.

Actually, corosync has 3 transports:
udp, udpu and iba.

Broadcast is same transport as udp but only has broadcast flag set on.

In other words, this patch is not correct.

Quoted 9 lines

So in order to configure broadcast we still need to use:

<cman transport="udp" broadcast="yes"/> ?

I find that redundant tho...

Cheers
Fabio

No.
Actually. Original idea was to use transport keyword and give user ability to use udp (for multicast), udpu (for unicast), udpb (for broadcast) and rdma. But also we must keep compatibility so thats why broadcast keyword still exists. So actually, transport="udpb" is same as transport="udp" broadcast="yes".

Regards,
Honza

Quoted 23 lines
Quoted 22 lines

Regards,
Honza

Quoted 18 lines · Lon Hohberger
Signed-off-by Lon Hohberger <[email protected]>
---
 cman/daemon/cman-preconfig.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
cman/daemon/cman-preconfig.c +1 -1
index 5239d89..5d608a7 100644
@@ -286,7 +286,7 @@ static int add_ifaddr(struct objdb_iface_ver0 *objdb, char *mcast, char *ifaddr,
 	int ret = 0;
 	const char *tx_mech_to_str[] = {
 		[TX_MECH_UDP] = "udp",
-		[TX_MECH_UDPB] = "udp",
+		[TX_MECH_UDPB] = "udpb",
 		[TX_MECH_UDPU] = "udpu",
 		[TX_MECH_RDMA] = "iba",
 	};
author
#4 lore raw

PATCH 1/1 cman: Fix udpb handling in cman-preconfig

In reply to Jan Friesse
Quoted 10 lines · Jan Friesse

Lon,
even it may "logically" seemed that original code is incorrect and your patch make sense, it's not the case.

Actually, corosync has 3 transports:
udp, udpu and iba.

Broadcast is same transport as udp but only has broadcast flag set on.

In other words, this patch is not correct.

Ok, reverted. Sorry for clouding the issue,

-- Lon