[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Wed, 09 Oct 2013 17:17:15 -0700
From: ebiederm@...ssion.com (Eric W. Biederman)
To: Stephen Hemminger <stephen@...workplumber.org>
Cc: David Miller <davem@...emloft.net>, yamato@...hat.com,
netdev@...r.kernel.org
Subject: Re: [PATCH] veth: Showing peer of veth type dev in ip link (kernel side)
Stephen Hemminger <stephen@...workplumber.org> writes:
> On Tue, 8 Oct 2013 14:13:37 -0700
> Stephen Hemminger <stephen@...workplumber.org> wrote:
>
>> On Tue, 08 Oct 2013 15:23:49 -0400 (EDT)
>> David Miller <davem@...emloft.net> wrote:
>>
>> > From: Masatake YAMATO <yamato@...hat.com>
>> > Date: Fri, 4 Oct 2013 11:34:21 +0900
>> >
>> > > ip link has ability to show extra information of net work device if
>> > > kernel provides sunh information. With this patch veth driver can
>> > > provide its peer ifindex information to ip command via netlink
>> > > interface.
>> > >
>> > > Signed-off-by: Masatake YAMATO <yamato@...hat.com>
>> >
>> > Applied to net-next, thank you.
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe netdev" in
>> > the body of a message to majordomo@...r.kernel.org
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>> Please revert this. It is incorrect.
>> The info returned by any netlink message should be equal to the message
>> for setting.
>>
>> I think the correct patch would be something like this (compile tested only).
>>
>> --- a/drivers/net/veth.c 2013-10-06 14:48:23.806461177 -0700
>> +++ b/drivers/net/veth.c 2013-10-08 14:11:42.434074690 -0700
>> @@ -434,6 +434,35 @@ static const struct nla_policy veth_poli
>> [VETH_INFO_PEER] = { .len = sizeof(struct ifinfomsg) },
>> };
>>
>> +static size_t veth_get_size(const struct net_device *dev)
>> +{
>> + return nla_total_size(sizeof(struct ifinfomsg)) + /* VETH_INFO_PEER */
>> + 0;
>> +}
>> +
>> +static int veth_fill_info(struct sk_buff *skb, const struct net_device *dev)
>> +{
>> + struct veth_priv *priv = netdev_priv(dev);
>> + struct net_device *peer = rtnl_dereference(priv->peer);
>> +
>> + if (peer) {
>> + struct ifinfomsg ifi = {
>> + .ifi_family = AF_UNSPEC,
>> + .ifi_type = peer->type,
>> + .ifi_index = peer->ifindex,
>> + .ifi_flags = dev_get_flags(peer),
>> + };
>> +
>> + if (nla_put(skb, VETH_INFO_PEER, sizeof(ifi), &ifi))
>> + goto nla_put_failure;
>> + }
>> +
>> + return 0;
>> +
>> +nla_put_failure:
>> + return -EMSGSIZE;
>> +}
>> +
>> static struct rtnl_link_ops veth_link_ops = {
>> .kind = DRV_NAME,
>> .priv_size = sizeof(struct veth_priv),
>> @@ -443,6 +472,8 @@ static struct rtnl_link_ops veth_link_op
>> .dellink = veth_dellink,
>> .policy = veth_policy,
>> .maxtype = VETH_INFO_MAX,
>> + .get_size = veth_get_size,
>> + .fill_info = veth_fill_info,
>> };
>>
>> /*
>>
>>
>
> This patch is ok as RFC starting point but the full implementation needs to
> add on IFLA_NAME and other attributes such that the full peer can be reconstructed.
>
> Ideally, the output of 'ip link' command can be in a format that can be used
> to recreate the same veth pair.
>
> One issue is that veth has the ability to make a peer in a different namespace
> and the network namespace code does not appear to have the ability to be invertable.
> I.e it is not possible to construct IFLA_NET_NS_PID or IFLA_NET_NS_FD attributes
> from an existing network device namespace.
Right.
IFLA_NET_NS_PID is not invertible as there may be no processes running
in a pid namespace.
IFLA_NET_NS_FD is in principle invertible. We just need to add a file
descriptor to the callers fd table. I don't see IFLA_NET_NS_FD being
invertible for broadcast messages, but for unicast it looks like a bit
of a pain but there are no fundamental problems.
I don't know if we care enough yet to write the code for the
IFLA_NET_NS_FD attribute but it is doable.
Eric
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists