Re: [dpdk-dev] [PATCH 1/3] pcap: utilize underlying real interface properties

["Mcnamara, John" <john.mcnamara@intel.com>]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tero Aho
> Sent: Friday, February 27, 2015 1:43 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 1/3] pcap: utilize underlying real interface
> properties
> 
> These changes set pcap interface mac address to the real underlying
> interface address instead of the default one. Also real interface link
> status, speed and duplex are reported when eth_link_update is called for
> the pcap interface.
> 
> Signed-off-by: Tero Aho <tero.aho@coriant.com>

> ...
> +               /* get link status, speed and duplex from the underlying interface */
> +
> +               strncpy(ifr.ifr_name, internals->if_name,
> sizeof(ifr.ifr_name)-1);
> +               ifr.ifr_name[sizeof(ifr.ifr_name)-1] = 0;
> +               if (!ioctl(internals->if_fd, SIOCGIFFLAGS, &ifr))
> +                       dev->data->dev_link.link_status = (ifr.ifr_flags
> + & IFF_UP) ? 1 : 0;
> +
> +               cmd.cmd = ETHTOOL_GSET;
> +               ifr.ifr_data = (void *)&cmd;
> +               if (!ioctl(internals->if_fd, SIOCETHTOOL, &ifr)) {
> +                       dev->data->dev_link.link_speed =
> ethtool_cmd_speed(&cmd);


Hi Tero,

I can see the benefit of setting the link status and speed etc. but this method seems a little fragile. What if there isn't an underlying interface, or all the interfaces are already bound to DPDK? Also I don't think the ethtool calls will work on FreeBSD as is.

So overall, I think this is a nack for this part of the patch. Perhaps a better approach would be to provide a function so that the calling application can set the parameters instead.

John