From: ebiederm@xmission•com (Eric W. Biederman)
To: Stephen Hemminger <stephen@networkplumber•org>
Cc: David Miller <davem@davemloft•net>,
yamato@redhat•com, netdev@vger•kernel.org
Subject: Re: [PATCH] veth: Showing peer of veth type dev in ip link (kernel side)
Date: Wed, 09 Oct 2013 17:17:15 -0700 [thread overview]
Message-ID: <87li22vv1w.fsf@xmission.com> (raw)
In-Reply-To: <20131009165254.2e1c8332@nehalam.linuxnetplumber.net> (Stephen Hemminger's message of "Wed, 9 Oct 2013 16:52:54 -0700")
Stephen Hemminger <stephen@networkplumber•org> writes:
> On Tue, 8 Oct 2013 14:13:37 -0700
> Stephen Hemminger <stephen@networkplumber•org> wrote:
>
>> On Tue, 08 Oct 2013 15:23:49 -0400 (EDT)
>> David Miller <davem@davemloft•net> wrote:
>>
>> > From: Masatake YAMATO <yamato@redhat•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@redhat•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@vger•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
next prev parent reply other threads:[~2013-10-10 0:17 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-04 2:34 [PATCH] veth: Showing peer of veth type dev in ip link (kernel side) Masatake YAMATO
2013-10-08 19:23 ` David Miller
2013-10-08 21:13 ` Stephen Hemminger
2013-10-09 1:52 ` David Miller
[not found] ` <20131009165254.2e1c8332@nehalam.linuxnetplumber.net>
2013-10-10 0:17 ` Eric W. Biederman [this message]
2013-10-15 16:44 ` Nicolas Dichtel
2013-10-15 20:34 ` Eric W. Biederman
2013-10-16 10:08 ` Nicolas Dichtel
2013-10-16 19:53 ` Eric W. Biederman
2013-10-17 16:05 ` Nicolas Dichtel
2013-10-17 19:28 ` Eric W. Biederman
2013-10-18 15:34 ` Nicolas Dichtel
-- strict thread matches above, loose matches on Subject: below --
2013-10-04 4:05 Masatake YAMATO
2013-10-04 4:49 ` Eric Dumazet
2013-10-04 15:21 ` Nicolas Dichtel
2013-10-04 17:55 ` Stephen Hemminger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87li22vv1w.fsf@xmission.com \
--to=ebiederm@xmission$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=stephen@networkplumber$(echo .)org \
--cc=yamato@redhat$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox