From: Eric Dumazet <eric.dumazet@gmail.com>
To: greearb@candelatech.com
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH 1/2 v2] af-packet: Use existing netdev reference for bound sockets.
Date: Fri, 27 May 2011 05:42:25 +0200 [thread overview]
Message-ID: <1306467745.2543.60.camel@edumazet-laptop> (raw)
In-Reply-To: <1306454141-1634-1-git-send-email-greearb@candelatech.com>
Le jeudi 26 mai 2011 à 16:55 -0700, greearb@candelatech.com a écrit :
> From: Ben Greear <greearb@candelatech.com>
>
> This saves a network device lookup on each packet transmitted,
> for sockets that are bound to a network device.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
>
> v2: Remove un-used ifindex, lookup dev in else branch.
>
> :100644 100644 4005b24... f9b807d... M net/packet/af_packet.c
> net/packet/af_packet.c | 27 +++++++++++++++------------
> 1 files changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 4005b24..f9b807d 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -989,7 +989,8 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
> struct sk_buff *skb;
> struct net_device *dev;
> __be16 proto;
> - int ifindex, err, reserve = 0;
> + bool need_rls_dev = false;
> + int err, reserve = 0;
> void *ph;
> struct sockaddr_ll *saddr = (struct sockaddr_ll *)msg->msg_name;
> int tp_len, size_max;
> @@ -1001,7 +1002,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
>
> err = -EBUSY;
> if (saddr == NULL) {
> - ifindex = po->ifindex;
> + dev = po->prot_hook.dev;
> proto = po->num;
> addr = NULL;
> } else {
> @@ -1012,12 +1013,12 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
> + offsetof(struct sockaddr_ll,
> sll_addr)))
> goto out;
> - ifindex = saddr->sll_ifindex;
> proto = saddr->sll_protocol;
> addr = saddr->sll_addr;
> + dev = dev_get_by_index(sock_net(&po->sk), saddr->sll_ifindex);
> + need_rls_dev = true;
> }
>
> - dev = dev_get_by_index(sock_net(&po->sk), ifindex);
> err = -ENXIO;
> if (unlikely(dev == NULL))
> goto out;
> @@ -1103,7 +1104,8 @@ out_status:
> __packet_set_status(po, ph, status);
> kfree_skb(skb);
> out_put:
> - dev_put(dev);
> + if (need_rls_dev)
> + dev_put(dev);
> out:
> mutex_unlock(&po->pg_vec_lock);
> return err;
> @@ -1141,8 +1143,9 @@ static int packet_snd(struct socket *sock,
> struct sk_buff *skb;
> struct net_device *dev;
> __be16 proto;
> + bool need_rls_dev = false;
> unsigned char *addr;
> - int ifindex, err, reserve = 0;
> + int err, reserve = 0;
> struct virtio_net_hdr vnet_hdr = { 0 };
> int offset = 0;
> int vnet_hdr_len;
> @@ -1160,7 +1163,7 @@ static int packet_snd(struct socket *sock,
> */
>
> if (saddr == NULL) {
> - ifindex = po->ifindex;
> + dev = po->prot_hook.dev;
> proto = po->num;
> addr = NULL;
> } else {
> @@ -1169,13 +1172,12 @@ static int packet_snd(struct socket *sock,
> goto out;
> if (msg->msg_namelen < (saddr->sll_halen + offsetof(struct sockaddr_ll, sll_addr)))
> goto out;
> - ifindex = saddr->sll_ifindex;
> proto = saddr->sll_protocol;
> addr = saddr->sll_addr;
> + dev = dev_get_by_index(sock_net(sk), saddr->sll_ifindex);
> + need_rls_dev = true;
> }
>
> -
> - dev = dev_get_by_index(sock_net(sk), ifindex);
> err = -ENXIO;
> if (dev == NULL)
> goto out_unlock;
> @@ -1315,14 +1317,15 @@ static int packet_snd(struct socket *sock,
> if (err > 0 && (err = net_xmit_errno(err)) != 0)
> goto out_unlock;
>
> - dev_put(dev);
> + if (need_rls_dev)
> + dev_put(dev);
>
> return len;
>
> out_free:
> kfree_skb(skb);
> out_unlock:
> - if (dev)
> + if (dev && need_rls_dev)
> dev_put(dev);
> out:
> return err;
Hmmm, I wonder why you want this Ben.
IMHO this is buggy, because we can sleep in this function.
We must take a ref on device (its really cheap these days, now we have a
percpu device refcnt)
next prev parent reply other threads:[~2011-05-27 3:42 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-26 23:55 [PATCH 1/2 v2] af-packet: Use existing netdev reference for bound sockets greearb
2011-05-26 23:55 ` [PATCH 2/2 v2] af-packet: Add flag to distinguish VID 0 from no-vlan greearb
2011-05-27 3:46 ` Eric Dumazet
2011-05-27 3:42 ` Eric Dumazet [this message]
2011-05-27 4:11 ` [PATCH 1/2 v2] af-packet: Use existing netdev reference for bound sockets Ben Greear
2011-05-27 20:08 ` Eric Dumazet
2011-05-27 20:15 ` David Miller
2011-05-27 20:18 ` Ben Greear
2011-05-28 6:20 ` Eric Dumazet
2011-05-28 17:01 ` Ben Greear
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=1306467745.2543.60.camel@edumazet-laptop \
--to=eric.dumazet@gmail.com \
--cc=greearb@candelatech.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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