[dpdk-dev,/,RFC] kni: Add set_rx_mode callback to handle multicast groups

Message ID 20150507151754.1620c4cb@miho (mailing list archive)
State Superseded, archived
Headers

Commit Message

Simon Kagstrom May 7, 2015, 1:17 p.m. UTC
  This is needed to add / remove interfaces in multicast groups via the
ip tool.

The callback does nothing - the same as the kernel tun.c.

Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
---
Marked RFC since I'm by no means an expert on this. We noticed this
when playing with KNI and IGMP handling.

 lib/librte_eal/linuxapp/kni/kni_net.c | 6 ++++++
 1 file changed, 6 insertions(+)
  

Comments

Simon Kagstrom May 20, 2015, 6:04 a.m. UTC | #1
Ping?

// Simon

On 2015-05-07 15:17, Simon Kagstrom wrote:
> This is needed to add / remove interfaces in multicast groups via the
> ip tool.
> 
> The callback does nothing - the same as the kernel tun.c.
> 
> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> ---
> Marked RFC since I'm by no means an expert on this. We noticed this
> when playing with KNI and IGMP handling.
> 
>  lib/librte_eal/linuxapp/kni/kni_net.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c
> b/lib/librte_eal/linuxapp/kni/kni_net.c index dd95db5..cf93c4b 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_net.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_net.c
> @@ -495,6 +495,11 @@ kni_net_ioctl(struct net_device *dev, struct ifreq
> *rq, int cmd) return 0;
>  }
>  
> +static void
> +kni_net_set_rx_mode(struct net_device *dev)
> +{
> +}
> +
>  static int
>  kni_net_change_mtu(struct net_device *dev, int new_mtu)
>  {
> @@ -645,6 +650,7 @@ static const struct net_device_ops
> kni_net_netdev_ops = { .ndo_start_xmit = kni_net_tx,
>  	.ndo_change_mtu = kni_net_change_mtu,
>  	.ndo_do_ioctl = kni_net_ioctl,
> +	.ndo_set_rx_mode = kni_net_set_rx_mode,
>  	.ndo_get_stats = kni_net_stats,
>  	.ndo_tx_timeout = kni_net_tx_timeout,
>  	.ndo_set_mac_address = kni_net_set_mac,
>
  
Simon Kagstrom May 28, 2015, 9:11 a.m. UTC | #2
Stephen, Helin, perhaps you have some comment about this patch?

// Simon

On 2015-05-07 15:17, Simon Kagstrom wrote:
> This is needed to add / remove interfaces in multicast groups via the
> ip tool.
> 
> The callback does nothing - the same as the kernel tun.c.
> 
> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> ---
> Marked RFC since I'm by no means an expert on this. We noticed this
> when playing with KNI and IGMP handling.
> 
>  lib/librte_eal/linuxapp/kni/kni_net.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c
> b/lib/librte_eal/linuxapp/kni/kni_net.c index dd95db5..cf93c4b 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_net.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_net.c
> @@ -495,6 +495,11 @@ kni_net_ioctl(struct net_device *dev, struct ifreq
> *rq, int cmd) return 0;
>  }
>  
> +static void
> +kni_net_set_rx_mode(struct net_device *dev)
> +{
> +}
> +
>  static int
>  kni_net_change_mtu(struct net_device *dev, int new_mtu)
>  {
> @@ -645,6 +650,7 @@ static const struct net_device_ops
> kni_net_netdev_ops = { .ndo_start_xmit = kni_net_tx,
>  	.ndo_change_mtu = kni_net_change_mtu,
>  	.ndo_do_ioctl = kni_net_ioctl,
> +	.ndo_set_rx_mode = kni_net_set_rx_mode,
>  	.ndo_get_stats = kni_net_stats,
>  	.ndo_tx_timeout = kni_net_tx_timeout,
>  	.ndo_set_mac_address = kni_net_set_mac,
>
  
Stephen Hemminger May 28, 2015, 2:56 p.m. UTC | #3
On Thu, 7 May 2015 15:17:54 +0200
Simon Kagstrom <simon.kagstrom@netinsight.net> wrote:

> This is needed to add / remove interfaces in multicast groups via the
> ip tool.
> 
> The callback does nothing - the same as the kernel tun.c.
> 
> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>

Yes, the dummy callback is needed, otherwise SIOCADDMULTI ioctl will
fail.
  
Zhang, Helin June 2, 2015, 3:44 a.m. UTC | #4
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Simon Kagstrom
> Sent: Thursday, May 7, 2015 9:18 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH / RFC] kni: Add set_rx_mode callback to handle
> multicast groups
> 
> This is needed to add / remove interfaces in multicast groups via the ip tool.
Could you help to explain with more details of why it is needed?

Thanks,
Helin

> 
> The callback does nothing - the same as the kernel tun.c.
> 
> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> ---
> Marked RFC since I'm by no means an expert on this. We noticed this when
> playing with KNI and IGMP handling.
> 
>  lib/librte_eal/linuxapp/kni/kni_net.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c
> b/lib/librte_eal/linuxapp/kni/kni_net.c index dd95db5..cf93c4b 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_net.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_net.c
> @@ -495,6 +495,11 @@ kni_net_ioctl(struct net_device *dev, struct ifreq *rq, int
> cmd) return 0;  }
> 
> +static void
> +kni_net_set_rx_mode(struct net_device *dev) { }
> +
>  static int
>  kni_net_change_mtu(struct net_device *dev, int new_mtu)  { @@ -645,6
> +650,7 @@ static const struct net_device_ops kni_net_netdev_ops =
> { .ndo_start_xmit = kni_net_tx,
>  	.ndo_change_mtu = kni_net_change_mtu,
>  	.ndo_do_ioctl = kni_net_ioctl,
> +	.ndo_set_rx_mode = kni_net_set_rx_mode,
>  	.ndo_get_stats = kni_net_stats,
>  	.ndo_tx_timeout = kni_net_tx_timeout,
>  	.ndo_set_mac_address = kni_net_set_mac,
> --
> 1.9.1
  
Simon Kagstrom June 2, 2015, 5:43 a.m. UTC | #5
On 2015-06-02 05:44, Zhang, Helin wrote:
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Simon Kagstrom
>> Sent: Thursday, May 7, 2015 9:18 PM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH / RFC] kni: Add set_rx_mode callback to handle
>> multicast groups
>>
>> This is needed to add / remove interfaces in multicast groups via the ip tool.
> Could you help to explain with more details of why it is needed?

We did some (very basic) tests with IGMP, which involves adding
multicast addresses to ETH interfaces. This is done via the ip tool, an
example can be found on e.g.,


http://superuser.com/questions/324824/linux-built-in-or-open-source-program-to-join-multicast-group

and this will fail on KNI interfaces with the current code because of an
unimplemented ioctl (as Stephen Hemminger said earlier). The patch
simply adds an empty callback so that the ioctl succeeds, and this is
the same thing as the Linux tap interface does (so I think it should be
enough for KNI as well).


If you want, I can update the patch with a bit more description
(something like the above).

// Simon
  
Zhang, Helin June 2, 2015, 5:48 a.m. UTC | #6
> -----Original Message-----
> From: Simon Kågström [mailto:simon.kagstrom@netinsight.net]
> Sent: Tuesday, June 2, 2015 1:44 PM
> To: Zhang, Helin; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH / RFC] kni: Add set_rx_mode callback to handle
> multicast groups
> 
> On 2015-06-02 05:44, Zhang, Helin wrote:
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Simon Kagstrom
> >> Sent: Thursday, May 7, 2015 9:18 PM
> >> To: dev@dpdk.org
> >> Subject: [dpdk-dev] [PATCH / RFC] kni: Add set_rx_mode callback to
> >> handle multicast groups
> >>
> >> This is needed to add / remove interfaces in multicast groups via the ip tool.
> > Could you help to explain with more details of why it is needed?
> 
> We did some (very basic) tests with IGMP, which involves adding multicast
> addresses to ETH interfaces. This is done via the ip tool, an example can be found
> on e.g.,
> 
> 
> http://superuser.com/questions/324824/linux-built-in-or-open-source-program-
> to-join-multicast-group
> 
> and this will fail on KNI interfaces with the current code because of an
> unimplemented ioctl (as Stephen Hemminger said earlier). The patch simply adds
> an empty callback so that the ioctl succeeds, and this is the same thing as the
> Linux tap interface does (so I think it should be enough for KNI as well).
Yes, the root cause "null ioctl causes the failure" should be added in the commit log
for future reference by others.
I am OK for the reason, please add the details to the commit logs.

Thanks,
Helin

> 
> 
> If you want, I can update the patch with a bit more description (something like
> the above).


> 
> // Simon
  

Patch

diff --git a/lib/librte_eal/linuxapp/kni/kni_net.c
b/lib/librte_eal/linuxapp/kni/kni_net.c index dd95db5..cf93c4b 100644
--- a/lib/librte_eal/linuxapp/kni/kni_net.c
+++ b/lib/librte_eal/linuxapp/kni/kni_net.c
@@ -495,6 +495,11 @@  kni_net_ioctl(struct net_device *dev, struct ifreq
*rq, int cmd) return 0;
 }
 
+static void
+kni_net_set_rx_mode(struct net_device *dev)
+{
+}
+
 static int
 kni_net_change_mtu(struct net_device *dev, int new_mtu)
 {
@@ -645,6 +650,7 @@  static const struct net_device_ops
kni_net_netdev_ops = { .ndo_start_xmit = kni_net_tx,
 	.ndo_change_mtu = kni_net_change_mtu,
 	.ndo_do_ioctl = kni_net_ioctl,
+	.ndo_set_rx_mode = kni_net_set_rx_mode,
 	.ndo_get_stats = kni_net_stats,
 	.ndo_tx_timeout = kni_net_tx_timeout,
 	.ndo_set_mac_address = kni_net_set_mac,