multicast routing and multiple interfaces with same IP

multicast routing and multiple interfaces with same IP

[Ilia K.]

[-- Attachment #1: Type: text/plain, Size: 989 bytes --]

Hi All,
When routing daemon wants to enable forwarding of multicast traffic it
performs something like:

	struct vifctl vc = {
		.vifc_vifi  = 1,
		.vifc_flags = 0,
		.vifc_threshold = 1,
		.vifc_rate_limit = 0,
		.vifc_lcl_addr = ip, /* <--- ip address of physical interface, e.g. eth0 */
		.vifc_rmt_addr.s_addr = htonl(INADDR_ANY),
	  };
	setsockopt(fd, IPPROTO_IP, MRT_ADD_VIF, &vc, sizeof(vc));

This leads (in the kernel) to call to vif_add() function call which
search the (physical) device using assigned IP address:
	dev = ip_dev_find(net, vifc->vifc_lcl_addr.s_addr);

It seems like API (struct vifctl) does not allow to specify an
interface other way than using it's IP, and if there are more than a
single interface with specified IP only the first one will be found
(for example it makes problems when tunnel is configured using the
same IP as underlying interface).

Am I correct in identifying the problem?
I can propose the attached patch against 2.6.30.4.

Regards,
Ilia.

[-- Attachment #2: vif_add.patch --]
[-- Type: text/x-diff, Size: 1174 bytes --]

=== modified file 'include/linux/mroute.h'
--- old/include/linux/mroute.h	2009-08-10 11:17:32 +0000
+++ new/include/linux/mroute.h	2009-08-11 20:39:14 +0000
@@ -61,11 +61,13 @@
 	unsigned int vifc_rate_limit;	/* Rate limiter values (NI) */
 	struct in_addr vifc_lcl_addr;	/* Our address */
 	struct in_addr vifc_rmt_addr;	/* IPIP tunnel addr */
+	int ifindex;			/* Local interface index */
 };
 
 #define VIFF_TUNNEL	0x1	/* IPIP tunnel */
 #define VIFF_SRCRT	0x2	/* NI */
 #define VIFF_REGISTER	0x4	/* register vif	*/
+#define VIFF_USE_IFINDEX	0x8	/* use ifindex to find an interface */
 
 /*
  *	Cache manipulation structures for mrouted and PIMd

=== modified file 'net/ipv4/ipmr.c'
--- old/net/ipv4/ipmr.c	2009-08-10 11:17:32 +0000
+++ new/net/ipv4/ipmr.c	2009-08-11 22:15:24 +0000
@@ -470,6 +470,18 @@
 			return err;
 		}
 		break;
+	case VIFF_USE_IFINDEX:
+		dev = dev_get_by_index(net, vifc->ifindex);
+		if (!dev)
+			return -ENODEV;
+		if (dev->ip_ptr == NULL)
+			return -EADDRNOTAVAIL;
+		err = dev_set_allmulti(dev, 1);
+		if (err) {
+			dev_put(dev);
+			return err;
+		}
+		break;
 	case 0:
 		dev = ip_dev_find(net, vifc->vifc_lcl_addr.s_addr);
 		if (!dev)

Re: multicast routing and multiple interfaces with same IP

[Octavian Purdila]

On Wednesday 12 August 2009 02:26:21 Ilia K. wrote:
> Hi All,
> When routing daemon wants to enable forwarding of multicast traffic it
> performs something like:
>
> 	struct vifctl vc = {
> 		.vifc_vifi  = 1,
> 		.vifc_flags = 0,
> 		.vifc_threshold = 1,
> 		.vifc_rate_limit = 0,
> 		.vifc_lcl_addr = ip, /* <--- ip address of physical interface, e.g. eth0
> */ .vifc_rmt_addr.s_addr = htonl(INADDR_ANY),
> 	  };
> 	setsockopt(fd, IPPROTO_IP, MRT_ADD_VIF, &vc, sizeof(vc));
>
> This leads (in the kernel) to call to vif_add() function call which
> search the (physical) device using assigned IP address:
> 	dev = ip_dev_find(net, vifc->vifc_lcl_addr.s_addr);
>
> It seems like API (struct vifctl) does not allow to specify an
> interface other way than using it's IP, and if there are more than a
> single interface with specified IP only the first one will be found
> (for example it makes problems when tunnel is configured using the
> same IP as underlying interface).
>
> Am I correct in identifying the problem?
> I can propose the attached patch against 2.6.30.4.
>

Hi Ilia,

I don't have context on multicast routing, but this caught my attention:

>@@ -61,11 +61,13 @@ 
> 	unsigned int vifc_rate_limit;	/* Rate limiter values (NI) */
> 	struct in_addr vifc_lcl_addr;	/* Our address */
> 	struct in_addr vifc_rmt_addr;	/* IPIP tunnel addr */
>+	int ifindex;			/* Local interface index */
> };
>

Wouldn't this break userspace ABI? 

Perhaps you could use a union between vifc_lcl_addr and vifc_ifindex, they seem 
to be exclusive.

tavi

Re: multicast routing and multiple interfaces with same IP

[David Miller]

From: Octavian Purdila <opurdila@ixiacom.com>
Date: Thu, 27 Aug 2009 02:53:18 +0300

> I don't have context on multicast routing, but this caught my attention:
> 
>>@@ -61,11 +61,13 @@ 
>> 	unsigned int vifc_rate_limit;	/* Rate limiter values (NI) */
>> 	struct in_addr vifc_lcl_addr;	/* Our address */
>> 	struct in_addr vifc_rmt_addr;	/* IPIP tunnel addr */
>>+	int ifindex;			/* Local interface index */
>> };
>>
> 
> Wouldn't this break userspace ABI? 
> 
> Perhaps you could use a union between vifc_lcl_addr and vifc_ifindex, they seem 
> to be exclusive.

Indeed, this will need to be fixed up.

Re: multicast routing and multiple interfaces with same IP

[Ilia K.]

[-- Attachment #1: Type: text/plain, Size: 1106 bytes --]

Hi,
I'm attaching a new patch. The changes:
mroute.h:
- consistent name for interface index: vifc_lcl_ifindex
- union of vifc_lcl_addr and vifc_lcl_ifindex since either one of them
can be used
ipmr.c:
- case VIFF_USE_IFINDEX and case 0 had almost the same code, so using
fall through and if to eliminate code duplication

Regards,
Ilia.

On Sat, Aug 29, 2009 at 10:01 AM, David Miller<davem@davemloft.net> wrote:
> From: Octavian Purdila <opurdila@ixiacom.com>
> Date: Thu, 27 Aug 2009 02:53:18 +0300
>
>> I don't have context on multicast routing, but this caught my attention:
>>
>>>@@ -61,11 +61,13 @@
>>>      unsigned int vifc_rate_limit;   /* Rate limiter values (NI) */
>>>      struct in_addr vifc_lcl_addr;   /* Our address */
>>>      struct in_addr vifc_rmt_addr;   /* IPIP tunnel addr */
>>>+     int ifindex;                    /* Local interface index */
>>> };
>>>
>>
>> Wouldn't this break userspace ABI?
>>
>> Perhaps you could use a union between vifc_lcl_addr and vifc_ifindex, they seem
>> to be exclusive.
>
> Indeed, this will need to be fixed up.
>

[-- Attachment #2: vif_add.patch --]
[-- Type: text/x-diff, Size: 1587 bytes --]

=== modified file 'include/linux/mroute.h'
--- include/linux/mroute.h	2009-08-10 11:17:32 +0000
+++ include/linux/mroute.h	2009-09-07 15:16:43 +0000
@@ -59,13 +59,17 @@
 	unsigned char vifc_flags;	/* VIFF_ flags */
 	unsigned char vifc_threshold;	/* ttl limit */
 	unsigned int vifc_rate_limit;	/* Rate limiter values (NI) */
-	struct in_addr vifc_lcl_addr;	/* Our address */
+	union {
+		struct in_addr vifc_lcl_addr;     /* Local interface address */
+		int            vifc_lcl_ifindex;  /* Local interface index   */
+	};
 	struct in_addr vifc_rmt_addr;	/* IPIP tunnel addr */
 };
 
-#define VIFF_TUNNEL	0x1	/* IPIP tunnel */
-#define VIFF_SRCRT	0x2	/* NI */
-#define VIFF_REGISTER	0x4	/* register vif	*/
+#define VIFF_TUNNEL		0x1	/* IPIP tunnel */
+#define VIFF_SRCRT		0x2	/* NI */
+#define VIFF_REGISTER		0x4	/* register vif	*/
+#define VIFF_USE_IFINDEX	0x8	/* use vifc_lcl_ifindex to find an interface */
 
 /*
  *	Cache manipulation structures for mrouted and PIMd

=== modified file 'net/ipv4/ipmr.c'
--- net/ipv4/ipmr.c	2009-08-10 11:17:32 +0000
+++ net/ipv4/ipmr.c	2009-09-07 15:20:07 +0000
@@ -470,8 +470,18 @@
 			return err;
 		}
 		break;
+
+	case VIFF_USE_IFINDEX:
 	case 0:
-		dev = ip_dev_find(net, vifc->vifc_lcl_addr.s_addr);
+		if (vifc->vifc_flags==VIFF_USE_IFINDEX) {
+			dev = dev_get_by_index(net, vifc->vifc_lcl_ifindex);
+			if (dev && dev->ip_ptr == NULL) {
+				dev_put(dev);
+				return -EADDRNOTAVAIL;
+			}
+		} else {
+			dev = ip_dev_find(net, vifc->vifc_lcl_addr.s_addr);
+		}
 		if (!dev)
 			return -EADDRNOTAVAIL;
 		err = dev_set_allmulti(dev, 1);

Re: multicast routing and multiple interfaces with same IP

[Octavian Purdila]

On Monday 07 September 2009 18:35:41 you wrote:
> Hi,
> I'm attaching a new patch. The changes:
> mroute.h:
> - consistent name for interface index: vifc_lcl_ifindex
> - union of vifc_lcl_addr and vifc_lcl_ifindex since either one of them
> can be used
> ipmr.c:
> - case VIFF_USE_IFINDEX and case 0 had almost the same code, so using
> fall through and if to eliminate code duplication
> 

Hi Ilia,

Looks good to me, but there are a couple of code style issues reported by 
./scripts/checkpatch.pl. 

Also, here:

>+               } else {                                                                                                                       
>+                       dev = ip_dev_find(net, vifc->vifc_lcl_addr.s_addr); 
>+               }                                                                                                                              

Usually no braces are used for single line statements in if/else.

tavi