[patch net-next] net: make packet_type->ak_packet_priv generic

[patch net-next] net: make packet_type->ak_packet_priv generic

[Jiri Pirko]

The priv is used now by af_packet. Rename it to make it useable for
others as well. Also, introduce packet_id_match to move the sk matching
from skb_loop_sk to af_packet code where it belongs.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 include/linux/netdevice.h |  2 +-
 net/core/dev.c            |  4 +---
 net/packet/af_packet.c    | 22 +++++++++++++++-------
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 775cc95..04a9375 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1773,7 +1773,7 @@ struct packet_type {
 					 struct net_device *);
 	bool			(*id_match)(struct packet_type *ptype,
 					    struct sock *sk);
-	void			*af_packet_priv;
+	void			*priv;
 	struct list_head	list;
 };
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 48d81e4..36e562e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1714,13 +1714,11 @@ static inline int deliver_skb(struct sk_buff *skb,
 
 static inline bool skb_loop_sk(struct packet_type *ptype, struct sk_buff *skb)
 {
-	if (!ptype->af_packet_priv || !skb->sk)
+	if (!ptype->priv || !skb->sk)
 		return false;
 
 	if (ptype->id_match)
 		return ptype->id_match(ptype, skb->sk);
-	else if ((struct sock *)ptype->af_packet_priv == skb->sk)
-		return true;
 
 	return false;
 }
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 01039d2..1c99be3 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1343,7 +1343,7 @@ static bool fanout_has_flag(struct packet_fanout *f, u16 flag)
 static int packet_rcv_fanout(struct sk_buff *skb, struct net_device *dev,
 			     struct packet_type *pt, struct net_device *orig_dev)
 {
-	struct packet_fanout *f = pt->af_packet_priv;
+	struct packet_fanout *f = pt->priv;
 	unsigned int num = f->num_members;
 	struct packet_sock *po;
 	unsigned int idx;
@@ -1424,7 +1424,7 @@ static void __fanout_unlink(struct sock *sk, struct packet_sock *po)
 
 static bool match_fanout_group(struct packet_type *ptype, struct sock *sk)
 {
-	if (ptype->af_packet_priv == (void *)((struct packet_sock *)sk)->fanout)
+	if (ptype->priv == (void *)((struct packet_sock *)sk)->fanout)
 		return true;
 
 	return false;
@@ -1486,7 +1486,7 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
 		match->prot_hook.type = po->prot_hook.type;
 		match->prot_hook.dev = po->prot_hook.dev;
 		match->prot_hook.func = packet_rcv_fanout;
-		match->prot_hook.af_packet_priv = match;
+		match->prot_hook.priv = match;
 		match->prot_hook.id_match = match_fanout_group;
 		dev_add_pack(&match->prot_hook);
 		list_add(&match->list, &fanout_list);
@@ -1544,7 +1544,7 @@ static int packet_rcv_spkt(struct sk_buff *skb, struct net_device *dev,
 	 *	field for just this event.
 	 */
 
-	sk = pt->af_packet_priv;
+	sk = pt->priv;
 
 	/*
 	 *	Yank back the headers [hope the device set this
@@ -1767,7 +1767,7 @@ static int packet_rcv(struct sk_buff *skb, struct net_device *dev,
 	if (skb->pkt_type == PACKET_LOOPBACK)
 		goto drop;
 
-	sk = pt->af_packet_priv;
+	sk = pt->priv;
 	po = pkt_sk(sk);
 
 	if (!net_eq(dev_net(dev), sock_net(sk)))
@@ -1892,7 +1892,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
 	if (skb->pkt_type == PACKET_LOOPBACK)
 		goto drop;
 
-	sk = pt->af_packet_priv;
+	sk = pt->priv;
 	po = pkt_sk(sk);
 
 	if (!net_eq(dev_net(dev), sock_net(sk)))
@@ -2767,6 +2767,13 @@ static struct proto packet_proto = {
 	.obj_size = sizeof(struct packet_sock),
 };
 
+static bool packet_id_match(struct packet_type *ptype, struct sock *sk)
+{
+	if ((struct sock *) ptype->priv == sk)
+		return true;
+	return false;
+}
+
 /*
  *	Create a packet of type SOCK_PACKET.
  */
@@ -2823,7 +2830,8 @@ static int packet_create(struct net *net, struct socket *sock, int protocol,
 	if (sock->type == SOCK_PACKET)
 		po->prot_hook.func = packet_rcv_spkt;
 
-	po->prot_hook.af_packet_priv = sk;
+	po->prot_hook.id_match = packet_id_match,
+	po->prot_hook.priv = sk;
 
 	if (proto) {
 		po->prot_hook.type = proto;
-- 
1.9.0

Re: [patch net-next] net: make packet_type->ak_packet_priv generic

[Nicolas Dichtel]

Le 04/04/2014 14:20, Jiri Pirko a écrit :
> The priv is used now by af_packet. Rename it to make it useable for
> others as well. Also, introduce packet_id_match to move the sk matching
> from skb_loop_sk to af_packet code where it belongs.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>   include/linux/netdevice.h |  2 +-
>   net/core/dev.c            |  4 +---
>   net/packet/af_packet.c    | 22 +++++++++++++++-------
>   3 files changed, 17 insertions(+), 11 deletions(-)
Hmm, net-next is closed I think.

Re: [patch net-next] net: make packet_type->ak_packet_priv generic

[Jiri Pirko]

Fri, Apr 04, 2014 at 02:58:24PM CEST, nicolas.dichtel@6wind.com wrote:
>Le 04/04/2014 14:20, Jiri Pirko a écrit :
>>The priv is used now by af_packet. Rename it to make it useable for
>>others as well. Also, introduce packet_id_match to move the sk matching
>>from skb_loop_sk to af_packet code where it belongs.
>>
>>Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>---
>>  include/linux/netdevice.h |  2 +-
>>  net/core/dev.c            |  4 +---
>>  net/packet/af_packet.c    | 22 +++++++++++++++-------
>>  3 files changed, 17 insertions(+), 11 deletions(-)
>Hmm, net-next is closed I think.

I did not see the announcement, maybe I missed it.

Re: [patch net-next] net: make packet_type->ak_packet_priv generic

[Nicolas Dichtel]

Le 04/04/2014 15:23, Jiri Pirko a écrit :
> Fri, Apr 04, 2014 at 02:58:24PM CEST, nicolas.dichtel@6wind.com wrote:
>> Le 04/04/2014 14:20, Jiri Pirko a écrit :
>>> The priv is used now by af_packet. Rename it to make it useable for
>>> others as well. Also, introduce packet_id_match to move the sk matching
>> >from skb_loop_sk to af_packet code where it belongs.
>>>
>>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>> ---
>>>   include/linux/netdevice.h |  2 +-
>>>   net/core/dev.c            |  4 +---
>>>   net/packet/af_packet.c    | 22 +++++++++++++++-------
>>>   3 files changed, 17 insertions(+), 11 deletions(-)
>> Hmm, net-next is closed I think.
>
> I did not see the announcement, maybe I missed it.
http://www.spinics.net/lists/netdev/msg278012.html

Re: [patch net-next] net: make packet_type->ak_packet_priv generic

[David Miller]

From: Jiri Pirko <jiri@resnulli.us>
Date: Fri,  4 Apr 2014 14:20:38 +0200

> The priv is used now by af_packet. Rename it to make it useable for
> others as well. Also, introduce packet_id_match to move the sk matching
> from skb_loop_sk to af_packet code where it belongs.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>

Firstly, net-next is closed.

Secondly, submit this with the unmentioned and mysterious "other
user", there currently are none.

Re: [patch net-next] net: make packet_type->ak_packet_priv generic

[Jiri Pirko]

Fri, Apr 04, 2014 at 03:54:20PM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Fri,  4 Apr 2014 14:20:38 +0200
>
>> The priv is used now by af_packet. Rename it to make it useable for
>> others as well. Also, introduce packet_id_match to move the sk matching
>> from skb_loop_sk to af_packet code where it belongs.
>> 
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>
>Firstly, net-next is closed.

Dave, it would be really great if you stick with the same email subject
when you declare net-next closed. I was looking for "CLOSED" but now the
subject is "no new feature patches please" which I missed.

Subject like "*net-next is CLOSED*" or some other constant nicely
visible string would avoid people like me miss this and people like you
and Nicolas to remind me :)

>
>Secondly, submit this with the unmentioned and mysterious "other
>user", there currently are none.

There is none atm. But anyway, the struct field name should be generic,
cause this is not specific to af_packet. Anybody might want to pass a
"priv".

Re: [patch net-next] net: make packet_type->ak_packet_priv generic

[David Miller]

From: Jiri Pirko <jiri@resnulli.us>
Date: Fri, 4 Apr 2014 16:03:25 +0200

> Fri, Apr 04, 2014 at 03:54:20PM CEST, davem@davemloft.net wrote:
>>From: Jiri Pirko <jiri@resnulli.us>
>>Date: Fri,  4 Apr 2014 14:20:38 +0200
>>
>>> The priv is used now by af_packet. Rename it to make it useable for
>>> others as well. Also, introduce packet_id_match to move the sk matching
>>> from skb_loop_sk to af_packet code where it belongs.
>>> 
>>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>
>>Firstly, net-next is closed.
> 
> Dave, it would be really great if you stick with the same email subject
> when you declare net-next closed. I was looking for "CLOSED" but now the
> subject is "no new feature patches please" which I missed.

Sorry, that's not a reasonable request.

It is very clear from the subject line I choose what the message is
about.  I don't have to use an ALL CAPS subject line "just because"

>>Secondly, submit this with the unmentioned and mysterious "other
>>user", there currently are none.
> 
> There is none atm. But anyway, the struct field name should be generic,
> cause this is not specific to af_packet. Anybody might want to pass a
> "priv".

No, because it makes it clear what the one and only user is.

Any new user would be seriously frowned upon, because this is
an easy side channel for proprietary extensions to the networking.

Re: [patch net-next] net: make packet_type->ak_packet_priv generic

[Nicolas Dichtel]

Le 04/04/2014 16:07, David Miller a écrit :
> From: Jiri Pirko <jiri@resnulli.us>
> Date: Fri, 4 Apr 2014 16:03:25 +0200
>
>> Fri, Apr 04, 2014 at 03:54:20PM CEST, davem@davemloft.net wrote:
>>> From: Jiri Pirko <jiri@resnulli.us>
>>> Date: Fri,  4 Apr 2014 14:20:38 +0200
>>>
>>>> The priv is used now by af_packet. Rename it to make it useable for
>>>> others as well. Also, introduce packet_id_match to move the sk matching
>>>> from skb_loop_sk to af_packet code where it belongs.
>>>>
>>>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>>
>>> Firstly, net-next is closed.
>>
>> Dave, it would be really great if you stick with the same email subject
>> when you declare net-next closed. I was looking for "CLOSED" but now the
>> subject is "no new feature patches please" which I missed.
>
> Sorry, that's not a reasonable request.
>
> It is very clear from the subject line I choose what the message is
> about.  I don't have to use an ALL CAPS subject line "just because"
Sure, the subject is very clear. Personally I don't care, but I think that a
part of netdev subscribers/submitters don't read every emails. Having always
the same prefix (like the one you already used: '[README]') may ease these guys
to put some filters to highlight these emails. At the end, this may also save
you time ;-)

My two cents,
Nicolas

Re: [patch net-next] net: make packet_type->ak_packet_priv generic

[Eric Dumazet]

On Fri, 2014-04-04 at 16:23 +0200, Nicolas Dichtel wrote:

> Sure, the subject is very clear. Personally I don't care, but I think that a
> part of netdev subscribers/submitters don't read every emails. Having always
> the same prefix (like the one you already used: '[README]') may ease these guys
> to put some filters to highlight these emails. At the end, this may also save
> you time ;-)
> 
> My two cents,

My 2 cents would be for a developer to use updated git tree before
submitting a patch, right ?

Then, if you simply do following its pretty obvious :

$ git show | head -8
commit cd6362befe4cc7bf589a5236d2a780af2d47bcc9
Merge: 0f1b1e6d73cb b1586f099ba8
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Wed Apr 2 20:53:45 2014 -0700

    Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next
    
    Pull networking updates from David Miller:

Re: [patch net-next] net: make packet_type->ak_packet_priv generic

[Jiri Pirko]

Fri, Apr 04, 2014 at 04:07:54PM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Fri, 4 Apr 2014 16:03:25 +0200
>
>> Fri, Apr 04, 2014 at 03:54:20PM CEST, davem@davemloft.net wrote:
>>>From: Jiri Pirko <jiri@resnulli.us>
>>>Date: Fri,  4 Apr 2014 14:20:38 +0200
>>>
>>>> The priv is used now by af_packet. Rename it to make it useable for
>>>> others as well. Also, introduce packet_id_match to move the sk matching
>>>> from skb_loop_sk to af_packet code where it belongs.
>>>> 
>>>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>>
>>>Firstly, net-next is closed.
>> 
>> Dave, it would be really great if you stick with the same email subject
>> when you declare net-next closed. I was looking for "CLOSED" but now the
>> subject is "no new feature patches please" which I missed.
>
>Sorry, that's not a reasonable request.
>
>It is very clear from the subject line I choose what the message is
>about.  I don't have to use an ALL CAPS subject line "just because"

That was just an example. You used that already, for example:
Date: Tue, 19 Feb 2013 00:56:53 -0500 (EST)
From: David Miller <davem@davemloft.net>
To: netdev@vger.kernel.org
Subject: net-next is CLOSED

Please don't get me wrong, I'm just making a suggestion. As Nicolas wrote,
it would save your time. And others as well when they know what to 'grep'.


>
>>>Secondly, submit this with the unmentioned and mysterious "other
>>>user", there currently are none.
>> 
>> There is none atm. But anyway, the struct field name should be generic,
>> cause this is not specific to af_packet. Anybody might want to pass a
>> "priv".
>
>No, because it makes it clear what the one and only user is.

Well there are others which would use this priv as well. For example:
drivers/scsi/bnx2fc/bnx2fc_fcoe.c

They work around this by embedding struct packet_type into their priv and in
_rcv they access it by container_of.

At the end I think it would be nicer to allow struct packet_type to be
defined static by users and pass priv and dev as parameters to dev_add_pack()

>
>Any new user would be seriously frowned upon, because this is
>an easy side channel for proprietary extensions to the networking.