From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Borkmann Subject: Re: [PATCH net-next v2 2/2] netlink: specify netlink packet direction for nlmon Date: Mon, 23 Dec 2013 11:46:46 +0100 Message-ID: <52B81496.6060807@redhat.com> References: <1387788519-17722-1-git-send-email-dborkman@redhat.com> <1387788519-17722-3-git-send-email-dborkman@redhat.com> <52B813E8.8010508@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: davem@davemloft.net, netdev@vger.kernel.org, Jakub Zawadzki To: nicolas.dichtel@6wind.com Return-path: Received: from mx1.redhat.com ([209.132.183.28]:1241 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757255Ab3LWKqy (ORCPT ); Mon, 23 Dec 2013 05:46:54 -0500 In-Reply-To: <52B813E8.8010508@6wind.com> Sender: netdev-owner@vger.kernel.org List-ID: On 12/23/2013 11:43 AM, Nicolas Dichtel wrote: > Le 23/12/2013 09:48, Daniel Borkmann a =C3=A9crit : >> In order to facilitate development for netlink protocol dissector, >> fill the unused field skb->pkt_type of the cloned skb with a hint >> of the address space of the new owner (receiver) socket in the >> notion of "to kernel" resp. "to user". >> >> At the time we invoke __netlink_deliver_tap_skb(), we already have >> set the new skb owner via netlink_skb_set_owner_r(), so we can use >> that for netlink_is_kernel() probing. >> >> In normal PF_PACKET network traffic, this field denotes if the >> packet is destined for us (PACKET_HOST), if it's broadcast >> (PACKET_BROADCAST), etc. >> >> As we only have 3 bit reserved, we can use the value (=3D 6) of >> PACKET_FASTROUTE as it's _not used_ anywhere in the whole kernel >> and packets of such type were never exposed to user space, so >> there are no overlapping users of such kind. Thus, as wished, >> that seems the only way to make both PACKET_* values non-overlapping >> and therefore device agnostic. >> >> By using those two flags for netlink skbs on nlmon devices, they >> can be made available and picked up via sll_pkttype (previously >> unused in netlink context) in struct sockaddr_ll. We now have >> these two directions: >> >> - PACKET_USER (=3D 6) -> to user space >> - PACKET_KERNEL (=3D 7) -> to kernel space >> >> Partial `ip a` example strace for sa_family=3DAF_NETLINK with >> detected nl msg direction: >> >> syscall: direction: >> sendto(3, ...) =3D 40 /* to kernel */ >> recvmsg(3, ...) =3D 3404 /* to user */ >> recvmsg(3, ...) =3D 1120 /* to user */ >> recvmsg(3, ...) =3D 20 /* to user */ >> sendto(3, ...) =3D 40 /* to kernel */ >> recvmsg(3, ...) =3D 168 /* to user */ >> recvmsg(3, ...) =3D 144 /* to user */ >> recvmsg(3, ...) =3D 20 /* to user */ >> >> Signed-off-by: Daniel Borkmann >> Signed-off-by: Jakub Zawadzki >> --- >> v1->v2: >> - let PACKET_* values not overlap as wished by Dave >> >> include/uapi/linux/if_packet.h | 4 +++- >> net/netlink/af_netlink.c | 2 ++ >> 2 files changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/include/uapi/linux/if_packet.h b/include/uapi/linux/if_= packet.h >> index e9d844c..06e2a28 100644 >> --- a/include/uapi/linux/if_packet.h >> +++ b/include/uapi/linux/if_packet.h >> @@ -26,8 +26,10 @@ struct sockaddr_ll { >> #define PACKET_MULTICAST 2 /* To group */ >> #define PACKET_OTHERHOST 3 /* To someone else */ >> #define PACKET_OUTGOING 4 /* Outgoing of any type */ >> -/* These ones are invisible by user level */ >> #define PACKET_LOOPBACK 5 /* MC/BRD frame looped bac= k */ >> +#define PACKET_USER 6 /* To user space */ > Reusing this value is like changing the API. If some userland apps an= d external > modules rely on it, this patch may break them. Sorry, but I thought I made it clear in the commit message that PACKET_FASTROUTE is *not* used anywhere in the whole kernel tree. And as the comment said as well, this type was never exposed to user land. >> +#define PACKET_KERNEL 7 /* To kernel space */ >> +/* Unused, PACKET_FASTROUTE and PACKET_LOOPBACK are invisble to use= r space */ > nitpicking: s/invisble/invisible > >> #define PACKET_FASTROUTE 6 /* Fastrouted frame */ >> >> /* Packet socket options */ >> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c >> index 56e09d8..3f75f1c 100644 >> --- a/net/netlink/af_netlink.c >> +++ b/net/netlink/af_netlink.c >> @@ -204,6 +204,8 @@ static int __netlink_deliver_tap_skb(struct sk_b= uff *skb, >> if (nskb) { >> nskb->dev =3D dev; >> nskb->protocol =3D htons((u16) sk->sk_protocol); >> + nskb->pkt_type =3D netlink_is_kernel(sk) ? >> + PACKET_KERNEL : PACKET_USER; >> >> ret =3D dev_queue_xmit(nskb); >> if (unlikely(ret > 0)) >> > -- > 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