* [PATCH] net: add __must_check to sk_add_backlog
From: Zhu Yi @ 2010-03-08 2:21 UTC (permalink / raw)
To: davem; +Cc: netdev, Zhu Yi
Add the "__must_check" tag to sk_add_backlog() so that any failure to
check and drop packets will be warned about.
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
---
include/net/sock.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index 170353d..092b055 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -604,7 +604,7 @@ static inline void __sk_add_backlog(struct sock *sk, struct sk_buff *skb)
}
/* The per-socket spinlock must be held here. */
-static inline int sk_add_backlog(struct sock *sk, struct sk_buff *skb)
+static inline __must_check int sk_add_backlog(struct sock *sk, struct sk_buff *skb)
{
if (sk->sk_backlog.len >= max(sk->sk_backlog.limit, sk->sk_rcvbuf << 1))
return -ENOBUFS;
--
1.6.3.3
^ permalink raw reply related
* Re: [RFC][ PATCH 1/3] vhost-net: support multiple buffer heads in receiver
From: Michael S. Tsirkin @ 2010-03-08 7:45 UTC (permalink / raw)
To: David Stevens; +Cc: kvm, netdev, rusty, virtualization
In-Reply-To: <OFEEE8B4AB.0C708EB8-ON882576DF.00831519-882576E0.000329C4@us.ibm.com>
On Sun, Mar 07, 2010 at 04:34:32PM -0800, David Stevens wrote:
> "Michael S. Tsirkin" <mst@redhat.com> wrote on 03/07/2010 07:31:30 AM:
>
> > On Tue, Mar 02, 2010 at 05:20:15PM -0700, David Stevens wrote:
> > > This patch generalizes buffer handling functions to
> NULL, NULL);
> > > + head.iov_base = (void *)vhost_get_vq_desc(&net->dev,
> vq,
> > > + vq->iov, ARRAY_SIZE(vq->iov), &out, &in, NULL,
>
> > > NULL);
> >
> > Should type for head be changed so that we do not need to cast?
> >
> > I also prefer aligning descendants on the opening (.
>
> Yes, that's probably better; the indentation with the cast would
> still wrap a lot, but I'll see what I can do here.
>
>
> > > err = sock->ops->sendmsg(NULL, sock, &msg, len);
> > > if (unlikely(err < 0)) {
> > > - vhost_discard_vq_desc(vq);
> > > + vhost_discard(vq, 1);
> >
> > Isn't the original name a bit more descriptive?
>
> During development, I had both and I generally like
> shorter names, but I can change it back.
>
> > > +static int skb_head_len(struct sk_buff_head *skq)
> > > +{
> > > + struct sk_buff *head;
> > > +
> > > + head = skb_peek(skq);
> > > + if (head)
> > > + return head->len;
> > > + return 0;
> > > +}
> > > +
> >
> > This is done without locking, which I think can crash
> > if skb is consumed after we peek it but before we read the
> > length.
>
> This thread is the only legitimate consumer, right? But
> qemu has the file descriptor and I guess we shouldn't trust
> that it won't give it to someone else; it'd break vhost, but
> a crash would be inappropriate, of course. I'd like to avoid
> the lock, but I have another idea here, so will investigate.
>
> >
> >
> > > /* Expects to be always run from workqueue - which acts as
> > > * read-size critical section for our kind of RCU. */
> > > static void handle_rx(struct vhost_net *net)
> > > {
> > > struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_RX];
> > > - unsigned head, out, in, log, s;
> > > + unsigned in, log, s;
> > > struct vhost_log *vq_log;
> > > struct msghdr msg = {
> > > .msg_name = NULL,
> > > @@ -204,10 +213,11 @@
> > > };
> > >
> > > size_t len, total_len = 0;
> > > - int err;
> > > + int err, headcount, datalen;
> > > size_t hdr_size;
> > > struct socket *sock = rcu_dereference(vq->private_data);
> > > - if (!sock || skb_queue_empty(&sock->sk->sk_receive_queue))
> > > +
> > > + if (!sock || !skb_head_len(&sock->sk->sk_receive_queue))
> > > return;
> > >
> >
> > Isn't this equivalent? Do you expect zero len skbs in socket?
> > If yes, maybe we should discard these, not stop processing ...
>
> A zero return means "no skb's". They are equivalent; I
> changed this call just to make it identical to the loop check,
> but I don't have a strong attachment to this.
>
>
> > > vq_log = unlikely(vhost_has_feature(&net->dev,
> VHOST_F_LOG_ALL)) ?
> > > vq->log : NULL;
> > >
> > > - for (;;) {
> > > - head = vhost_get_vq_desc(&net->dev, vq, vq->iov,
> > > - ARRAY_SIZE(vq->iov),
> > > - &out, &in,
> > > - vq_log, &log);
> > > + while ((datalen = skb_head_len(&sock->sk->sk_receive_queue)))
> {
> >
> > This peeks in the queue to figure out how much data we have.
> > It's a neat trick, but it does introduce new failure modes
> > where an skb is consumed after we did the peek:
> > - new skb could be shorter, we should return the unconsumed part
> > - new skb could be longer, this will drop a packet.
> > maybe this last is not an issue as the race should be rare in practice
>
> As before, this loop is the only legitimate consumer of skb's; if
> anyone else is reading the socket, it's broken. But since the file
> descriptor is available to qemu, it's probably trusting qemu too much.
> I don't believe there is a race at all on a working system; the head
> can't change until we read it after this test, but I'll see if I can
> come up with something better here. Closing the fd for qemu so vhost
> has exclusive access might do it, but otherwise we could just get a
> max-sized packet worth of buffers and then return them after the read.
> That'd force us to wait with small packets when the ring is nearly
> full, where we don't have to now, but I expect locking to read the head
> length will hurt performance in all cases. Will try these ideas out.k
>
> >
> > > + headcount = vhost_get_heads(vq, datalen, &in, vq_log,
> > > &log);
> > > /* OK, now we need to know about added descriptors. */
> > > - if (head == vq->num) {
> > > + if (!headcount) {
> > > if (unlikely(vhost_enable_notify(vq))) {
> > > /* They have slipped one in as we were
> > > * doing that: check again. */
> > > @@ -235,13 +242,6 @@
> > > * they refilled. */
> > > break;
> > > }
> > > - /* We don't need to be notified again. */
> >
> > you find this comment unhelpful?
>
> This code is changed in the later patches; the comment was
> removed with that, but I got it in the wrong patch on the split.
> I guess the comment is ok to stay, anyway, but notification may
> require multiple buffers to be available; I had fixed that here, but
> the final patch pushes that into the notify code, so yes, this can
> go back in.
>
> > > - if (out) {
> > > - vq_err(vq, "Unexpected descriptor format for
> RX: "
> > > - "out %d, int %d\n",
> > > - out, in);
> > > - break;
> > > - }
> >
> >
> > we still need this check, don't we?
>
> It's done in vhost_get_heads(); "out" isn't visible in handle_rx()
> anymore.
>
> > > +unsigned vhost_get_heads(struct vhost_virtqueue *vq, int datalen, int
>
> > > *iovcount,
> > > + struct vhost_log *log, unsigned int *log_num)
> >
> > Could you please document this function's parameters? It's not obvious
> > what iovcount, log, log_num are.
>
> Yes. To answer the question, they are the same as vhost_get_vq_desc(),
> except "iovcount" replaces "in" (and "out" is not needed in the caller).
>
> > > +{
> > > + struct iovec *heads = vq->heads;
> > > + int out, in;
> > > + int hc = 0;
> > > +
> > > + while (datalen > 0) {
> > > + if (hc >= VHOST_NET_MAX_SG) {
> > > + vhost_discard(vq, hc);
> > > + return 0;
> > > + }
> > > + heads[hc].iov_base = (void
> *)vhost_get_vq_desc(vq->dev,
> > > vq,
> > > + vq->iov, ARRAY_SIZE(vq->iov), &out, &in, log,
> > > log_num);
> > > + if (heads[hc].iov_base == (void *)vq->num) {
> > > + vhost_discard(vq, hc);
> > > + return 0;
> > > + }
> > > + if (out || in <= 0) {
> > > + vq_err(vq, "unexpected descriptor format for
> RX: "
> > > + "out %d, in %d\n", out, in);
> > > + vhost_discard(vq, hc);
> > > + return 0;
> > > + }
> > > + heads[hc].iov_len = iov_length(vq->iov, in);
> > > + hc++;
> > > + datalen -= heads[hc].iov_len;
> > > + }
> > > + *iovcount = in;
> >
> >
> > Only the last value?
>
> In this part of the split, it only goes through once; this is
> changed to the accumulated value "seg" in a later patch. So, split
> artifact.
>
> {
> > > struct vring_used_elem *used;
> > > + int i;
> > >
> > > - /* The virtqueue contains a ring of used buffers. Get a
> pointer
> > > to the
> > > - * next entry in that used ring. */
> > > - used = &vq->used->ring[vq->last_used_idx % vq->num];
> > > - if (put_user(head, &used->id)) {
> > > - vq_err(vq, "Failed to write used id");
> > > - return -EFAULT;
> > > - }
> > > - if (put_user(len, &used->len)) {
> > > - vq_err(vq, "Failed to write used len");
> > > - return -EFAULT;
> > > + for (i=0; i<count; i++, vq->last_used_idx++) {
> >
> > whitespace damage: I prefer space around =, <.
> > I also use ++i, etc in this driver, better be consistent?
> > Also for clarity, I prefer to put vq->last_used_idx inside the loop.
>
> OK.
> >
> > > + used = &vq->used->ring[vq->last_used_idx % vq->num];
> > > + if (put_user((unsigned)heads[i].iov_base, &used->id))
> {
> > > + vq_err(vq, "Failed to write used id");
> > > + return -EFAULT;
> > > + }
> > > + if (put_user(heads[i].iov_len, &used->len)) {
> > > + vq_err(vq, "Failed to write used len");
> > > + return -EFAULT;
> > > + }
> >
> > If this fails, last_used_idx will still be incremented, which I think is
> wrong.
>
> True, but if these fail, aren't we dead? I don't think we can
> recover
> from an EFAULT in any of these; I didn't test those paths from the
> original,
> but I think we need to bail out entirely for these cases, right?
>
> +-DLS
Yes, we stop on error, but host can fix uo the vq and redo a kick.
That's why there's errorfd.
--
MST
^ permalink raw reply
* Re: [RFC][ PATCH 2/3] vhost-net: handle vnet_hdr processing for MRG_RX_BUF
From: Michael S. Tsirkin @ 2010-03-08 7:54 UTC (permalink / raw)
To: David Stevens; +Cc: kvm, netdev, rusty, virtualization
In-Reply-To: <OF8A4807F2.DE42C086-ON882576E0.00033A96-882576E0.00080FCA@us.ibm.com>
On Sun, Mar 07, 2010 at 05:28:02PM -0800, David Stevens wrote:
> "Michael S. Tsirkin" <mst@redhat.com> wrote on 03/07/2010 08:12:29 AM:
>
> > On Tue, Mar 02, 2010 at 05:20:26PM -0700, David Stevens wrote:
> > > This patch adds vnet_hdr processing for mergeable buffer
> > > support to vhost-net.
> > >
> > > Signed-off-by: David L Stevens <dlstevens@us.ibm.com>
> > >
> > > diff -ruN net-next-p1/drivers/vhost/net.c
> net-next-p2/drivers/vhost/net.c
> >
> > Could you please add -p to diff flags so that it's easier
> > to figure out which function is changes?
>
> Sure.
>
>
> > > @@ -148,25 +146,45 @@
> > > "out %d, int %d\n", out, in);
> > > break;
> > > }
> > > + if (vq->guest_hlen > vq->sock_hlen) {
> > > + if (msg.msg_iov[0].iov_len == vq->guest_hlen)
> > > + msg.msg_iov[0].iov_len =
> vq->sock_hlen;
> > > + else if (out == ARRAY_SIZE(vq->iov))
> > > + vq_err(vq, "handle_tx iov overflow!");
> > > + else {
> > > + int i;
> > > +
> > > + /* give header its own iov */
> > > + for (i=out; i>0; ++i)
> > > + msg.msg_iov[i+1] =
> msg.msg_iov[i];
> > > + msg.msg_iov[0].iov_len =
> vq->sock_hlen;
> > > + msg.msg_iov[1].iov_base +=
> vq->guest_hlen;
> > > + msg.msg_iov[1].iov_len -=
> vq->guest_hlen;
> > > + out++;
> >
> > We seem to spend a fair bit of code here and elsewhere trying to cover
> > the diff between header size in guest and host. In hindsight, it was
> > not a good idea to put new padding between data and the header:
> > virtio-net should have added it *before*. But it is what it is.
> >
> > Wouldn't it be easier to just add an ioctl to tap so that
> > vnet header size is made bigger by 4 bytes?
>
> I'd be ok with that, but if we support raw sockets, we have
> some of the issues (easier, since it's 0). "num_buffers" is only
> 16 bits, so just add 2 bytes, but yeah-- pushing it to tap would
> be easier for vhost.
>
>
> > > /* TODO: Check specific error and bomb out unless
> ENOBUFS?
> > > */
> > > err = sock->ops->sendmsg(NULL, sock, &msg, len);
> > > if (unlikely(err < 0)) {
> > > - vhost_discard(vq, 1);
> > > - tx_poll_start(net, sock);
> > > + if (err == -EAGAIN) {
> >
> > The comment mentions ENOBUFS. Are you sure it's EAGAIN?
>
> The comment's from the original -- the code changes should do
> the "TODO", so probably should remove the comment.
> >
> > > + tx_poll_start(net, sock);
> >
> > Don't we need to call discard to move the last avail header back?
>
> Yes, I think you're right. We might consider dropping the packet
> in the EAGAIN case here instead, though. It's not clear retrying the
> same packet with a delay here is better than getting new ones when the
> socket buffer is full (esp. with queues on both sides already).
You mean already full? Well, this one triggers even if queue
on guest side is not full. TCP might be ok with it, but I suspect
packet drops would be very bad for UDP.
> Maybe
> we should fold EAGAIN into the other error cases? Otherwise, you're
> right, it looks like it should have a discard here.
>
> >
> > > + } else {
> > > + vq_err(vq, "sendmsg: errno %d\n",
> -err);
> > > + /* drop packet; do not discard/resend
> */
> > > + vhost_add_used_and_signal(&net->dev,vq,&head,1);
> > > + }
> > > break;
> > > - }
> > > - if (err != len)
> > > + } else if (err != len)
> >
> >
> > Previous if ends with break so no need for else here.
>
> Yes.
>
>
> > > @@ -232,25 +243,18 @@
> > > headcount = vhost_get_heads(vq, datalen, &in, vq_log,
> > > &log);
> > > /* OK, now we need to know about added descriptors. */
> > > if (!headcount) {
> > > - if (unlikely(vhost_enable_notify(vq))) {
> > > - /* They have slipped one in as we were
> > > - * doing that: check again. */
> > > - vhost_disable_notify(vq);
> > > - continue;
> > > - }
> > > - /* Nothing new? Wait for eventfd to tell us
> > > - * they refilled. */
> > > + vhost_enable_notify(vq);
> >
> >
> > You don't think this race can happen?
>
> The notify case has to allow for multiple heads, not just the one,
> so if he added just one head, it doesn't mean we're ok to continue-- the
> packet may require multiple. Instead, the notifier now checks for enough
> to handle a max-sized packet (whether or not NONOTIFY is set). I'll think
> about this some more, but I concluded it wasn't needed at the time. :-)
>
Hmm, need to think about it as well.
> > > @@ -519,6 +524,20 @@
> > >
> > > vhost_net_disable_vq(n, vq);
> > > rcu_assign_pointer(vq->private_data, sock);
> > > +
> > > + if (sock && sock->sk) {
> > > + if (!vhost_sock_is_raw(sock) ||
> >
> > I dislike this backend-specific code, it ties us with specifics of
> > backend implementations, which change without notice. Ideally all
> > backend-specific stuff should live in userspace, userspace programs
> > vhost device appropriately.
>
> We could do that; we need to know whether the socket has a
> vnet header on it or not and the existing flags don't tell us. If
> qemu sets a flag telling us the socket is has or doesn't have vnet,
> that'd be equivalent.
qemu tells us how much of the header to skip before passing
it to socket.
> > > + vhost_has_feature(&n->dev,
> > > VHOST_NET_F_VIRTIO_NET_HDR)) {
> > > + vq->sock_hlen = sizeof(struct virtio_net_hdr);
> > > + if (vhost_has_feature(&n->dev,
> > > VIRTIO_NET_F_MRG_RXBUF))
> > > + vq->guest_hlen =
> > > + sizeof(struct
> > > virtio_net_hdr_mrg_rxbuf);
> > > + else
> > > + vq->guest_hlen = sizeof(struct
> > > virtio_net_hdr);
> > > + } else
> > > + vq->guest_hlen = vq->sock_hlen = 0;
> > > + } else
> > > + vq_err(vq, "vhost_net_set_backend: sock->sk is NULL");
> >
> > As proposed above, IMO it would be nicer to add an ioctl in tap
> > that let us program header size.
>
> Do you know if Herbert or Dave have an opinion on that solution?
I'l post a patch, and we'll see.
> > > +static int
> > > +vhost_get_hdr(struct vhost_virtqueue *vq, int *in, struct vhost_log
> *log,
> > > + int *log_num)
> > > +{
> > > + struct iovec *heads = vq->heads;
> > > + struct iovec *iov = vq->iov;
> > > + int out;
> > > +
> > > + *in = 0;
> > > + iov[0].iov_len = 0;
> > > +
> > > + /* get buffer, starting from iov[1] */
> > > + heads[0].iov_base = (void *)vhost_get_vq_desc(vq->dev, vq,
> > > + vq->iov+1, ARRAY_SIZE(vq->iov)-1, &out, in, log,
> log_num);
> > > + if (out || *in <= 0) {
> > > + vq_err(vq, "unexpected descriptor format for RX: out
> %d, "
> > > + "in %d\n", out, *in);
> > > + return 0;
> > > + }
> > > + if (heads[0].iov_base == (void *)vq->num)
> > > + return 0;
> > > +
> > > + /* make iov[0] the header */
> > > + if (!vq->guest_hlen) {
> > > + if (vq->sock_hlen) {
> > > + static struct virtio_net_hdr junk; /* bit
> bucket
> > > */
> > > +
> > > + iov[0].iov_base = &junk;
> > > + iov[0].iov_len = sizeof(junk);
> > > + } else
> > > + iov[0].iov_len = 0;
> > > + }
> > > + if (vq->sock_hlen < vq->guest_hlen) {
> > > + iov[0].iov_base = iov[1].iov_base;
> > > + iov[0].iov_len = vq->sock_hlen;
> > > +
> > > + if (iov[1].iov_len < vq->sock_hlen) {
> > > + vq_err(vq, "can't fit header in one buffer!");
> > > + vhost_discard(vq, 1);
> > > + return 0;
> > > + }
> > > + if (!vq->sock_hlen) {
> > > + static const struct virtio_net_hdr_mrg_rxbuf
> hdr =
> > > {
> > > + .hdr.flags = 0,
> > > + .hdr.gso_type =
> VIRTIO_NET_HDR_GSO_NONE
> > > + };
> > > + memcpy(iov[0].iov_base, &hdr, vq->guest_hlen);
> > > + }
> > > + iov[1].iov_base += vq->guest_hlen;
> > > + iov[1].iov_len -= vq->guest_hlen;
> > > + }
> > > + return 1;
> >
> > The above looks kind of scary, lots of special-casing. I'll send a
> > patch for tap to set vnet header size, let's see if it makes life easier
> > for us.
>
> Yes, I try to handle all combinations of differing guest and
> socket
> header lengths (including 0-lengths). If we don't support raw sockets
> and/or
> we make the tap vnet header bigger in the MRXB case, it'll simplify this
> code.
Let's see about tap patch then.
> >
> > > +}
> > > +
> > > unsigned vhost_get_heads(struct vhost_virtqueue *vq, int datalen, int
>
> > > *iovcount,
> > > struct vhost_log *log, unsigned int *log_num)
> > > {
> > > struct iovec *heads = vq->heads;
> > > - int out, in;
> > > + int out, in = 0;
> > > + int seg = 0;
> > > int hc = 0;
> >
> > I think it's better to stick to simple names like i
> > for index variables, alternatively give it a meaningful
> > name like "count".
>
> I'm not sure which one you're talking about. "in & out"
> are inherited from the original, "seg" is an iovec segment index, and
> "hc" is a head counter. "headcount" or "count" (if that's the
> one you mean) instead of "hc" would probably cause some line wraps.
> I think "i" is too generic here, but if you mean "hc", I could stick
> a comment on the declaration:
>
> int hc = 0; /* head count */
>
> Does that address your concern, or do you mean something else?
I mean hc. Let's call it count and live with line wraps if you don't
like i.
> >
> > >
> > > + if (vq->guest_hlen != vq->sock_hlen) {
> >
> > Sticking guest_hlen/sock_hlen in vhost is somewhat ugly.
>
> Do you mean in the struct? This essentially splits the
> original "hdrsize" into the two (possibly different) header
> sizes and allows removing the "hdr" iovec and reading the
> (partial) header directly into the extended header mergeable
> buffers needs.
Yes. hdrsize is kind of generic, socket is obviously net specific ...
> Unless we remove raw socket support and add the
> ioctl to tap you suggested, I'm not sure it can be much cleaner.
> For me, the ugliness comes from tap, raw and guest headers not
> being the same.
>
> +-DLS
--
MST
^ permalink raw reply
* Re: [RFC][ PATCH 3/3] vhost-net: Add mergeable RX buffer support to vhost-net
From: Michael S. Tsirkin @ 2010-03-08 8:07 UTC (permalink / raw)
To: David Stevens; +Cc: kvm, netdev, rusty, virtualization
In-Reply-To: <OF8D4A9381.FF5A483F-ON882576E0.0008221D-882576E0.000B9D4A@us.ibm.com>
On Sun, Mar 07, 2010 at 06:06:51PM -0800, David Stevens wrote:
> "Michael S. Tsirkin" <mst@redhat.com> wrote on 03/07/2010 08:26:33 AM:
>
> > On Tue, Mar 02, 2010 at 05:20:34PM -0700, David Stevens wrote:
> > > This patch glues them all together and makes sure we
> > > notify whenever we don't have enough buffers to receive
> > > a max-sized packet, and adds the feature bit.
> > >
> > > Signed-off-by: David L Stevens <dlstevens@us.ibm.com>
> >
> > Maybe split this up?
>
> I can. I was looking mostly at size (and this is the smallest
> of the bunch). But the feature requires all of them together, of course.
> This last one is just "everything left over" from the other two.
>
> > > @@ -110,6 +90,7 @@
> > > size_t len, total_len = 0;
> > > int err, wmem;
> > > struct socket *sock = rcu_dereference(vq->private_data);
> > > +
> >
> > I tend not to add empty lines if line below it is already short.
>
> This leaves no blank line between the declarations and the start
> of code. It's habit for me-- not sure of kernel coding standards address
> that or not, but I don't think I've seen it anywhere else.
>
> >
> > > if (!sock)
> > > return;
> > >
> > > @@ -166,11 +147,11 @@
> > > /* Skip header. TODO: support TSO. */
> > > msg.msg_iovlen = out;
> > > head.iov_len = len = iov_length(vq->iov, out);
> > > +
> >
> > I tend not to add empty lines if line below it is a comment.
>
> I added this to separate the logical "skip header" block from
> the next, unrelated piece. Not important to me, though.
>
> >
> > > /* Sanity check */
> > > if (!len) {
> > > vq_err(vq, "Unexpected header len for TX: "
> > > - "%zd expected %zd\n",
> > > - len, vq->guest_hlen);
> > > + "%zd expected %zd\n", len,
> vq->guest_hlen);
> > > break;
> > > }
> > > /* TODO: Check specific error and bomb out unless
> ENOBUFS?
> > > */
>
>
> > > /* TODO: Should check and handle checksum. */
> > > + if (vhost_has_feature(&net->dev,
> VIRTIO_NET_F_MRG_RXBUF))
> > > {
> > > + struct virtio_net_hdr_mrg_rxbuf *vhdr =
> > > + (struct virtio_net_hdr_mrg_rxbuf *)
> > > + vq->iov[0].iov_base;
> > > + /* add num_bufs */
> > > + vq->iov[0].iov_len = vq->guest_hlen;
> > > + vhdr->num_buffers = headcount;
> >
> > I don't understand this. iov_base is a userspace pointer, isn't it.
> > How can you assign values to it like that?
> > Rusty also commented earlier that it's not a good idea to assume
> > specific layout, such as first chunk being large enough to
> > include virtio_net_hdr_mrg_rxbuf.
> >
> > I think we need to use memcpy to/from iovec etc.
>
> I guess you mean put_user() or copy_to_user(); yes, I suppose
> it could be paged since we read it.
> The code doesn't assume that it'll fit so much as arranged for
> it to fit. We allocate guest_hlen bytes in the buffer, but set the
> iovec to the (smaller) sock_hlen; do the read, then this code adds
> back the 2 bytes in the middle that we didn't read into (where
> num_buffers goes). But the allocator does require that guest_hlen
> will fit in a single buffer (and reports error if it doesn't). The
> alternative is significantly more complicated,
I'm not sure why. Can't we just call memcpy_from_iovec
and then read the structure as usual?
> and only fails if
> the guest doesn't give us at least the buffer size the guest header
> requires (a truly lame guest). I'm not sure it's worth a lot of
> complexity in vhost to support the guest giving us <12 byte buffers;
> those guests don't exist now and maybe they never should?
>
>
> > > /* This actually signals the guest, using eventfd. */
> > > void vhost_signal(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> > > {
> > > __u16 flags = 0;
> > > +
> >
> > I tend not to add empty lines if a line above it is already short.
>
> Again, separating declarations from code-- never seen different
> in any other kernel code.
>
> >
> > > if (get_user(flags, &vq->avail->flags)) {
> > > vq_err(vq, "Failed to get flags");
> > > return;
> > > @@ -1125,7 +1140,7 @@
> > >
> > > /* If they don't want an interrupt, don't signal, unless
> empty. */
> > > if ((flags & VRING_AVAIL_F_NO_INTERRUPT) &&
> > > - (vq->avail_idx != vq->last_avail_idx ||
> > > + (vhost_available(vq) > vq->maxheadcount ||
> >
> > I don't understand this change. It seems to make
> > code not match the comments.
>
> It redefines "empty". Without mergeable buffers, we can empty
> the ring down to nothing before we require notification. With
> mergeable buffers, if the packet requires, say, 3 buffers, and we
> have only 2 left, we are empty and require notification and new
> buffers to read anything. In both cases, we notify when we can't
> read another packet without more buffers.
I don't really see how you can find this out from just the number of
heads. A head can address buffer of any size. Need to think
about this code some more.
> I can think about changing the comment to reflect this more
> clearly.
Also, this is all for rx only, so names should reflect this I think.
> >
> > > !vhost_has_feature(dev, VIRTIO_F_NOTIFY_ON_EMPTY)))
> > > return;
> > >
> > > diff -ruN net-next-p2/drivers/vhost/vhost.h
> > > net-next-p3/drivers/vhost/vhost.h
> > > --- net-next-p2/drivers/vhost/vhost.h 2010-03-02 13:02:03.000000000
> > > -0800
> > > +++ net-next-p3/drivers/vhost/vhost.h 2010-03-02 14:29:44.000000000
> > > -0800
> > > @@ -85,6 +85,7 @@
> > > struct iovec iov[VHOST_NET_MAX_SG+1]; /* an extra for vnet hdr
> */
> > > struct iovec heads[VHOST_NET_MAX_SG];
> > > size_t guest_hlen, sock_hlen;
> > > + int maxheadcount;
> >
> > I don't completely understand what does this field does.
> > It seems to be only set on rx? Maybe name should reflect this?
>
> This is a way for me to dynamically guess how many heads I need
> for a
> max-sized packet for whatever the MTU/GRO settings are without waiting to
> detect the need for more buffers until a read fails. Without mergeable
> buffers,
> we can always fit a max-sized packet in 1 head, but with, we need more
> than
> one head to do the read.
>
> I didn't want to hard-code 64K (which it usually is, but not always), and
> I didn't want to wait until a read fails every time the ring is near full.
> I played with re-enabling notify on 1/4 available or some such, but that
> delays
> reads unnecessarily, so I came up with this method: use maxheadcount to
> track
> the biggest packet we've ever seen and always make sure we have at least
> that
> many available for the next read. If it increases, we may fail the read,
> which'll
> notify, but this allows us to notify before we try and fail in normal
> operation,
> while still not doing a notify on every read.
>
> +-DLS
Hmm, yes. One of the horrors of the mergeable buffer hack.
Not sure all this is worth the complexity though: I don't think this
covers all cases whether the size would be < 64K, anyway: you don't get
notified when user disables GRO on physical NIC, for example. So maybe
just go ahead with hard-coding 64K.
In any case, number of heads does not necessarily tell us much either,
does it?
Would interrupt when we actually don't have room on RX work better? I'll
think about it some more.
--
MST
^ permalink raw reply
* Re: [tcpdump-workers] Current wireless-testing breaks libpcap: mr_alen should be set
From: Jiri Pirko @ 2010-03-08 8:11 UTC (permalink / raw)
To: Guy Harris; +Cc: tcpdump-workers, linux-wireless, netdev
In-Reply-To: <7D621661-59E7-46C4-9E86-1D9654A626E7@alum.mit.edu>
Sat, Mar 06, 2010 at 10:23:12PM CET, guy@alum.mit.edu wrote:
>
>On Mar 2, 2010, at 5:00 PM, Pavel Roskin wrote:
>
>> This patch to libpcap helps:
>>
>> --- a/pcap-linux.c
>> +++ b/pcap-linux.c
>> @@ -1563,6 +1563,7 @@ live_open_new(pcap_t *handle, const char
>> memset(&mr, 0, sizeof(mr));
>> mr.mr_ifindex = handle->md.ifindex;
>> mr.mr_type = PACKET_MR_PROMISC;
>> + mr.mr_alen = 6;
>
>If there are any network types that support promiscuous mode and have link-layer addresses that aren't 6 octets long, that would still fail.
>
>It sounds as if the fix is not to care about the address length if the address isn't used, so you don't need to get the length right for PACKET_MR_PROMISC or PACKET_MR_ALLMULTI, so libpcap, and other clients setting promiscuous or "show me all multicast packets" mode, don't need to change. Is that the case?
This should be fixed in kernel (net-2.6
1162563f82b434e3099c9e6c1bbdba846d792f0d)
Jirka
^ permalink raw reply
* Re: [PATCH net-2.6] netfilter: ebt_ip6: Use ipv6_masked_addr_cmp().
From: Bart De Schuymer @ 2010-03-08 8:25 UTC (permalink / raw)
To: YOSHIFUJI Hideaki
Cc: bart.de.schuymer, kaber, davem, ebtables-devel, netfilter-devel,
netdev
In-Reply-To: <201003070918.o279IXtq029570@94.43.138.210.xn.2iij.net>
YOSHIFUJI Hideaki schreef:
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> ---
> net/bridge/netfilter/ebt_ip6.c | 18 ++++--------------
> 1 files changed, 4 insertions(+), 14 deletions(-)
>
> diff --git a/net/bridge/netfilter/ebt_ip6.c b/net/bridge/netfilter/ebt_ip6.c
> index bbf2534..4644cc9 100644
> --- a/net/bridge/netfilter/ebt_ip6.c
> +++ b/net/bridge/netfilter/ebt_ip6.c
> @@ -35,8 +35,6 @@ ebt_ip6_mt(const struct sk_buff *skb, const struct xt_match_param *par)
> struct ipv6hdr _ip6h;
> const struct tcpudphdr *pptr;
> struct tcpudphdr _ports;
> - struct in6_addr tmp_addr;
> - int i;
>
> ih6 = skb_header_pointer(skb, 0, sizeof(_ip6h), &_ip6h);
> if (ih6 == NULL)
> @@ -44,18 +42,10 @@ ebt_ip6_mt(const struct sk_buff *skb, const struct xt_match_param *par)
> if (info->bitmask & EBT_IP6_TCLASS &&
> FWINV(info->tclass != ipv6_get_dsfield(ih6), EBT_IP6_TCLASS))
> return false;
> - for (i = 0; i < 4; i++)
> - tmp_addr.in6_u.u6_addr32[i] = ih6->saddr.in6_u.u6_addr32[i] &
> - info->smsk.in6_u.u6_addr32[i];
> - if (info->bitmask & EBT_IP6_SOURCE &&
> - FWINV((ipv6_addr_cmp(&tmp_addr, &info->saddr) != 0),
> - EBT_IP6_SOURCE))
> - return false;
> - for (i = 0; i < 4; i++)
> - tmp_addr.in6_u.u6_addr32[i] = ih6->daddr.in6_u.u6_addr32[i] &
> - info->dmsk.in6_u.u6_addr32[i];
> - if (info->bitmask & EBT_IP6_DEST &&
> - FWINV((ipv6_addr_cmp(&tmp_addr, &info->daddr) != 0), EBT_IP6_DEST))
> + if (FWINV(ipv6_masked_addr_cmp(&ih6->saddr, &info->smsk,
> + &info->saddr), EBT_IP6_SOURCE) ||
> + FWINV(ipv6_masked_addr_cmp(&ih6->daddr, &info->dmsk,
> + &info->daddr), EBT_IP6_DEST))
> return false;
> if (info->bitmask & EBT_IP6_PROTO) {
> uint8_t nexthdr = ih6->nexthdr;
>
Signed-off-by: Bart De Schuymer <bdschuym@pandora.be>
Looks OK to me.
cheers,
Bart
--
Bart De Schuymer
www.artinalgorithms.be
^ permalink raw reply
* Re: [RFC][PATCH] ns: Syscalls for better namespace sharing control.
From: Eric W. Biederman @ 2010-03-08 8:32 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Pavel Emelyanov, Sukadev Bhattiprolu, Serge Hallyn,
Linux Netdev List, containers, Netfilter Development Mailinglist,
Ben Greear
In-Reply-To: <4B92C886.9020507@free.fr>
I have take an snapshot of my development tree and placed it at.
git://git.kernel.org/pub/scm/linux/people/ebiederm/linux-2.6.33-nsfd-v5.git
>> I am going to explore a bit more. Given that nsfd is using the same
>> permission checks as a proc file, I think I can just make it a proc
>> file. Something like "/proc/<pid>/ns/net". With a little luck that
>> won't suck too badly.
>>
> Ah ! yes. Good idea.
It is a hair more code to use proc files but nothing worth counting.
Probably the biggest thing I am aware of right now in my development
tree is in getting uids to pass properly between unix domain sockets
I would up writing this cred_to_ucred function.
Serge can you take a look and check my logic, and do you have
any idea of where we should place something like pid_vnr but
for the uid namespace?
void cred_to_ucred(struct pid *pid, const struct cred *cred,
struct ucred *ucred)
{
ucred->pid = pid_vnr(pid);
ucred->uid = ucred->gid = -1;
if (cred) {
struct user_namespace *cred_ns = cred->user->user_ns;
struct user_namespace *current_ns = current_user_ns();
struct user_namespace *tmp;
if (likely(cred_ns == current_ns)) {
ucred->uid = cred->euid;
ucred->gid = cred->egid;
} else {
/* Is cred in a child user namespace */
tmp = cred_ns;
do {
tmp = tmp->creator->user_ns;
if (tmp == current_ns) {
ucred->uid = tmp->creator->uid;
ucred->gid = overflowgid;
return;
}
} while (tmp != &init_user_ns);
/* Is cred the creator of my user namespace,
* or the creator of one of it's parents?
*/
for( tmp = current_ns; tmp != &init_user_ns;
tmp = tmp->creator->user_ns) {
if (cred->user == tmp->creator) {
ucred->uid = 0;
ucred->gid = 0;
return;
}
}
/* No user namespace relationship so no mapping */
ucred->uid = overflowuid;
ucred->gid = overflowgid;
}
}
}
Eric
^ permalink raw reply
* Re: KS8695: possible NAPI issue
From: figo zhang @ 2010-03-08 9:04 UTC (permalink / raw)
To: Yegor Yefremov; +Cc: Dick Hollenbeck, netdev, zealcook
In-Reply-To: <f69abfc31003050700y57718a6aje3c1ff41d04748ae@mail.gmail.com>
2010/3/5 Yegor Yefremov <yegorslists@googlemail.com>:
>>> My tests look like following:
>>>
>>> 1. system start
>>> 2. ping one host: O.K.
>>> 3. nc -l -p 5000 > /var/test (about 1Mb): O.K.
>>> 4. ping the same host: failed
>> 1. type "arp" command, see the arp is exit or expire?
>
> debian:~# nc -l -p 5000 > /var/zImage
>
> debian:~# ping -c 1 192.168.1.38
>
> PING 192.168.1.38 (192.168.1.38) 56(84) bytes of data.
>
> From 192.168.1.66 icmp_seq=1 Destination Host Unreachable
>
>
>
> --- 192.168.1.38 ping statistics ---
>
> 1 packets transmitted, 0 received, +1 errors, 100% packet loss, time 0ms
>
>
>
> debian:~# arp
>
> Address HWtype HWaddress Flags Mask Iface
> 192.168.1.38 (incomplete) eth0
>
> 192.168.1.36 ether 00:10:18:39:19:aa C eth0
>
>
>> 2. at this point, see is it still have RX interrpt and receive packet in
>> ks8695_rx()? (in some watchpoint add printk).
>
> I've inserted some printks and I can see that interrupts are coming
> and the received count will be increased. I can also see my system
> sending arp requests. Even this ping request is visible in wireshark
> and its reply, but the ping utility sees nothing of it.
>
when you netcat finished, it cannot ping, right? at this point, would
you like to add some printk at RX and TX?
i want to see the target board have send arp packet, or have receive
arp reply packet.
you can add such funtion to print the packet buffer:
void print_mem(unsigned char *p,u32 len, u8 * s)
{
u32 i;
printk(" %s ram addr = %x , data len = %x",s, p, len);
for (i=0;i<len;i++) {
if (0==i%16) {
printk("\n 0x%02x : ",(p+i));
}
printk("%02x",*(u8 *)(p+i));
printk(" ");
}
printk("\n\n");
}
for TX: ks8695_start_xmit(struct sk_buff *skb, struct net_device *ndev)
=>
ksp->tx_ring[buff_n].status =
cpu_to_le32(TDES_IC | TDES_FS | TDES_LS |
(skb->len & TDES_TBS));
print_mem(skb->data, skb->len, "tx");
wmb();
for RX: static int ks8695_rx(struct ks8695_priv *ksp, int budget)
=>
/* Retrieve the sk_buff */
skb = ksp->rx_buffers[buff_n].skb;
print_mem(skb->data, skb->len, "rx");
/* Clear it from the ring */
ksp->rx_buffers[buff_n].skb = NULL;
^ permalink raw reply
* Re: [PATCH V3 2/8] tcp: use limited socket backlog
From: Eric Dumazet @ 2010-03-08 9:21 UTC (permalink / raw)
To: Zhu Yi, David Miller; +Cc: netdev
In-Reply-To: <1267769972.2867.1.camel@edumazet-laptop>
Le vendredi 05 mars 2010 à 07:19 +0100, Eric Dumazet a écrit :
> I'll submit a followup patch to add a MIB counter if your patch gets in.
>
As promised, here it is.
[PATCH] tcp: Add SNMP counters for backlog and min_ttl drops
Commit 6b03a53a (tcp: use limited socket backlog) added the possibility
of dropping frames when backlog queue is full.
Commit d218d111 (tcp: Generalized TTL Security Mechanism) added the
possibility of dropping frames when TTL is under a given limit.
This patch adds new SNMP MIB entries, named TCPBacklogDrop and
TCPMinTTLDrop, published in /proc/net/netstat in TcpExt: line
netstat -s | egrep "TCPBacklogDrop|TCPMinTTLDrop"
TCPBacklogDrop: 0
TCPMinTTLDrop: 0
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
include/linux/snmp.h | 2 ++
net/ipv4/proc.c | 2 ++
net/ipv4/tcp_ipv4.c | 7 +++++--
net/ipv6/tcp_ipv6.c | 3 ++-
4 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/include/linux/snmp.h b/include/linux/snmp.h
index e28f5a0..4435d10 100644
--- a/include/linux/snmp.h
+++ b/include/linux/snmp.h
@@ -225,6 +225,8 @@ enum
LINUX_MIB_SACKSHIFTED,
LINUX_MIB_SACKMERGED,
LINUX_MIB_SACKSHIFTFALLBACK,
+ LINUX_MIB_TCPBACKLOGDROP,
+ LINUX_MIB_TCPMINTTLDROP, /* RFC 5082 */
__LINUX_MIB_MAX
};
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 242ed23..4f1f337 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -249,6 +249,8 @@ static const struct snmp_mib snmp4_net_list[] = {
SNMP_MIB_ITEM("TCPSackShifted", LINUX_MIB_SACKSHIFTED),
SNMP_MIB_ITEM("TCPSackMerged", LINUX_MIB_SACKMERGED),
SNMP_MIB_ITEM("TCPSackShiftFallback", LINUX_MIB_SACKSHIFTFALLBACK),
+ SNMP_MIB_ITEM("TCPBacklogDrop", LINUX_MIB_TCPBACKLOGDROP),
+ SNMP_MIB_ITEM("TCPMinTTLDrop", LINUX_MIB_TCPMINTTLDROP),
SNMP_MIB_SENTINEL
};
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 1915f7d..8d51d39 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1651,8 +1651,10 @@ int tcp_v4_rcv(struct sk_buff *skb)
if (!sk)
goto no_tcp_socket;
- if (iph->ttl < inet_sk(sk)->min_ttl)
+ if (unlikely(iph->ttl < inet_sk(sk)->min_ttl)) {
+ NET_INC_STATS_BH(net, LINUX_MIB_TCPMINTTLDROP);
goto discard_and_relse;
+ }
process:
if (sk->sk_state == TCP_TIME_WAIT)
@@ -1682,8 +1684,9 @@ process:
if (!tcp_prequeue(sk, skb))
ret = tcp_v4_do_rcv(sk, skb);
}
- } else if (sk_add_backlog(sk, skb)) {
+ } else if (unlikely(sk_add_backlog(sk, skb))) {
bh_unlock_sock(sk);
+ NET_INC_STATS_BH(net, LINUX_MIB_TCPBACKLOGDROP);
goto discard_and_relse;
}
bh_unlock_sock(sk);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 2c378b1..9b6dbba 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1740,8 +1740,9 @@ process:
if (!tcp_prequeue(sk, skb))
ret = tcp_v6_do_rcv(sk, skb);
}
- } else if (sk_add_backlog(sk, skb)) {
+ } else if (unlikely(sk_add_backlog(sk, skb))) {
bh_unlock_sock(sk);
+ NET_INC_STATS_BH(net, LINUX_MIB_TCPBACKLOGDROP);
goto discard_and_relse;
}
bh_unlock_sock(sk);
^ permalink raw reply related
* Re: [PATCH 00/13] TProxy IPv6 support 2nd round
From: Amos Jeffries @ 2010-03-08 9:38 UTC (permalink / raw)
To: Balazs Scheidler; +Cc: Harald Welte, netfilter-devel, netdev
In-Reply-To: <1256547636.11361.18.camel@bzorp.balabit>
Balazs Scheidler wrote:
> On Sun, 2009-10-25 at 11:16 +0100, Harald Welte wrote:
>> Dear Balazs,
>>
>> as you might have read from other mails (and by the long period of silence),
>> Patrick McHardy is currently unavailable to perform his usual maintainer
>> role.
>>
>> I personally am too much out of touch with recent developments in
>> netfitler-land to be able to confidently review your patches...
>>
>> So unless somebody else from the team (Jozsef?, Pablo?) feels confident in
>> ACKing your patchset, I will have to ask for your patience until Patrick is
>> back and can do it by himself.
>
> Thanks for letting me know, hopefully Patrick gets better soon. I've
> planned another round of the TProxy patches as I got some comments at
> the previous round I'm yet to address.
>
> So no need to hurry.
>
Just bumping this topic up again.
What is the current status of these patches?
Our release which might make use of them goes into production sites in a
few weeks and I'm starting to see a little more interest in them from
our users.
AYJ
Squid Project
^ permalink raw reply
* [PATCH 3/6] qlcnic: fix multicast handling
From: Amit Kumar Salecha @ 2010-03-08 10:14 UTC (permalink / raw)
To: davem; +Cc: netdev, dhananjay.phadke, ameen.rahman, Sucheta Chakraborty
In-Reply-To: <1268043290-19929-1-git-send-email-amit.salecha@qlogic.com>
From: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
For promiscuous mode, driver send request to device for deleting
multicast addresses and again it send request for adding them back
while exiting from this mode, this is bad for performance.
Just setting device in promiscuous mode is enough, no need to del/add
multicast addresses.
Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
drivers/net/qlcnic/qlcnic_hw.c | 31 ++++++-------------------------
1 files changed, 6 insertions(+), 25 deletions(-)
diff --git a/drivers/net/qlcnic/qlcnic_hw.c b/drivers/net/qlcnic/qlcnic_hw.c
index e95646b..da00e16 100644
--- a/drivers/net/qlcnic/qlcnic_hw.c
+++ b/drivers/net/qlcnic/qlcnic_hw.c
@@ -398,20 +398,16 @@ qlcnic_sre_macaddr_change(struct qlcnic_adapter *adapter, u8 *addr,
return qlcnic_send_cmd_descs(adapter, (struct cmd_desc_type0 *)&req, 1);
}
-static int qlcnic_nic_add_mac(struct qlcnic_adapter *adapter,
- u8 *addr, struct list_head *del_list)
+static int qlcnic_nic_add_mac(struct qlcnic_adapter *adapter, u8 *addr)
{
struct list_head *head;
struct qlcnic_mac_list_s *cur;
/* look up if already exists */
- list_for_each(head, del_list) {
+ list_for_each(head, &adapter->mac_list) {
cur = list_entry(head, struct qlcnic_mac_list_s, list);
-
- if (memcmp(addr, cur->mac_addr, ETH_ALEN) == 0) {
- list_move_tail(head, &adapter->mac_list);
+ if (memcmp(addr, cur->mac_addr, ETH_ALEN) == 0)
return 0;
- }
}
cur = kzalloc(sizeof(struct qlcnic_mac_list_s), GFP_ATOMIC);
@@ -433,14 +429,9 @@ void qlcnic_set_multi(struct net_device *netdev)
struct dev_mc_list *mc_ptr;
u8 bcast_addr[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
u32 mode = VPORT_MISS_MODE_DROP;
- LIST_HEAD(del_list);
- struct list_head *head;
- struct qlcnic_mac_list_s *cur;
- list_splice_tail_init(&adapter->mac_list, &del_list);
-
- qlcnic_nic_add_mac(adapter, adapter->mac_addr, &del_list);
- qlcnic_nic_add_mac(adapter, bcast_addr, &del_list);
+ qlcnic_nic_add_mac(adapter, adapter->mac_addr);
+ qlcnic_nic_add_mac(adapter, bcast_addr);
if (netdev->flags & IFF_PROMISC) {
mode = VPORT_MISS_MODE_ACCEPT_ALL;
@@ -455,22 +446,12 @@ void qlcnic_set_multi(struct net_device *netdev)
if (!netdev_mc_empty(netdev)) {
netdev_for_each_mc_addr(mc_ptr, netdev) {
- qlcnic_nic_add_mac(adapter, mc_ptr->dmi_addr,
- &del_list);
+ qlcnic_nic_add_mac(adapter, mc_ptr->dmi_addr);
}
}
send_fw_cmd:
qlcnic_nic_set_promisc(adapter, mode);
- head = &del_list;
- while (!list_empty(head)) {
- cur = list_entry(head->next, struct qlcnic_mac_list_s, list);
-
- qlcnic_sre_macaddr_change(adapter,
- cur->mac_addr, QLCNIC_MAC_DEL);
- list_del(&cur->list);
- kfree(cur);
- }
}
int qlcnic_nic_set_promisc(struct qlcnic_adapter *adapter, u32 mode)
--
1.6.0.2
^ permalink raw reply related
* [PATCH 2/6] qlcnic: additional driver statistics.
From: Amit Kumar Salecha @ 2010-03-08 10:14 UTC (permalink / raw)
To: davem; +Cc: netdev, dhananjay.phadke, ameen.rahman, Sucheta Chakraborty
In-Reply-To: <1268043290-19929-1-git-send-email-amit.salecha@qlogic.com>
From: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
Statistics added for lro/lso bytes, count for tx stop queue and
wake queue and skb alloc failure count.
Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
drivers/net/qlcnic/qlcnic.h | 5 +++++
drivers/net/qlcnic/qlcnic_ethtool.c | 11 +++++++++++
drivers/net/qlcnic/qlcnic_hw.c | 1 +
drivers/net/qlcnic/qlcnic_init.c | 8 ++++++--
drivers/net/qlcnic/qlcnic_main.c | 5 +++++
5 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/net/qlcnic/qlcnic.h b/drivers/net/qlcnic/qlcnic.h
index b40a851..9897b69 100644
--- a/drivers/net/qlcnic/qlcnic.h
+++ b/drivers/net/qlcnic/qlcnic.h
@@ -423,6 +423,11 @@ struct qlcnic_adapter_stats {
u64 lro_pkts;
u64 rxbytes;
u64 txbytes;
+ u64 lrobytes;
+ u64 lso_frames;
+ u64 xmit_on;
+ u64 xmit_off;
+ u64 skb_alloc_failure;
};
/*
diff --git a/drivers/net/qlcnic/qlcnic_ethtool.c b/drivers/net/qlcnic/qlcnic_ethtool.c
index ef12792..f83e15f 100644
--- a/drivers/net/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/qlcnic/qlcnic_ethtool.c
@@ -59,6 +59,17 @@ static const struct qlcnic_stats qlcnic_gstrings_stats[] = {
QLC_SIZEOF(stats.rxbytes), QLC_OFF(stats.rxbytes)},
{"tx_bytes",
QLC_SIZEOF(stats.txbytes), QLC_OFF(stats.txbytes)},
+ {"lrobytes",
+ QLC_SIZEOF(stats.lrobytes), QLC_OFF(stats.lrobytes)},
+ {"lso_frames",
+ QLC_SIZEOF(stats.lso_frames), QLC_OFF(stats.lso_frames)},
+ {"xmit_on",
+ QLC_SIZEOF(stats.xmit_on), QLC_OFF(stats.xmit_on)},
+ {"xmit_off",
+ QLC_SIZEOF(stats.xmit_off), QLC_OFF(stats.xmit_off)},
+ {"skb_alloc_failure", QLC_SIZEOF(stats.skb_alloc_failure),
+ QLC_OFF(stats.skb_alloc_failure)},
+
};
#define QLCNIC_STATS_LEN ARRAY_SIZE(qlcnic_gstrings_stats)
diff --git a/drivers/net/qlcnic/qlcnic_hw.c b/drivers/net/qlcnic/qlcnic_hw.c
index 99a4d13..e95646b 100644
--- a/drivers/net/qlcnic/qlcnic_hw.c
+++ b/drivers/net/qlcnic/qlcnic_hw.c
@@ -349,6 +349,7 @@ qlcnic_send_cmd_descs(struct qlcnic_adapter *adapter,
if (nr_desc >= qlcnic_tx_avail(tx_ring)) {
netif_tx_stop_queue(tx_ring->txq);
__netif_tx_unlock_bh(tx_ring->txq);
+ adapter->stats.xmit_off++;
return -EBUSY;
}
diff --git a/drivers/net/qlcnic/qlcnic_init.c b/drivers/net/qlcnic/qlcnic_init.c
index ea00ab4..f0df971 100644
--- a/drivers/net/qlcnic/qlcnic_init.c
+++ b/drivers/net/qlcnic/qlcnic_init.c
@@ -1114,8 +1114,10 @@ qlcnic_alloc_rx_skb(struct qlcnic_adapter *adapter,
struct pci_dev *pdev = adapter->pdev;
buffer->skb = dev_alloc_skb(rds_ring->skb_size);
- if (!buffer->skb)
+ if (!buffer->skb) {
+ adapter->stats.skb_alloc_failure++;
return -ENOMEM;
+ }
skb = buffer->skb;
@@ -1289,7 +1291,7 @@ qlcnic_process_lro(struct qlcnic_adapter *adapter,
netif_receive_skb(skb);
adapter->stats.lro_pkts++;
- adapter->stats.rxbytes += length;
+ adapter->stats.lrobytes += length;
return buffer;
}
@@ -1505,6 +1507,8 @@ qlcnic_process_rcv_diag(struct qlcnic_adapter *adapter,
adapter->diag_cnt++;
dev_kfree_skb_any(skb);
+ adapter->stats.rx_pkts++;
+ adapter->stats.rxbytes += length;
return buffer;
}
diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
index 665e8e5..fc72156 100644
--- a/drivers/net/qlcnic/qlcnic_main.c
+++ b/drivers/net/qlcnic/qlcnic_main.c
@@ -118,6 +118,7 @@ qlcnic_update_cmd_producer(struct qlcnic_adapter *adapter,
if (qlcnic_tx_avail(tx_ring) <= TX_STOP_THRESH) {
netif_stop_queue(adapter->netdev);
smp_mb();
+ adapter->stats.xmit_off++;
}
}
@@ -1385,6 +1386,7 @@ qlcnic_tso_check(struct net_device *netdev,
int copied, offset, copy_len, hdr_len = 0, tso = 0, vlan_oob = 0;
struct cmd_desc_type0 *hwdesc;
struct vlan_ethhdr *vh;
+ struct qlcnic_adapter *adapter = netdev_priv(netdev);
if (protocol == cpu_to_be16(ETH_P_8021Q)) {
@@ -1494,6 +1496,7 @@ qlcnic_tso_check(struct net_device *netdev,
tx_ring->producer = producer;
barrier();
+ adapter->stats.lso_frames++;
}
static int
@@ -1573,6 +1576,7 @@ qlcnic_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
if (unlikely(no_of_desc + 2 > qlcnic_tx_avail(tx_ring))) {
netif_stop_queue(netdev);
+ adapter->stats.xmit_off++;
return NETDEV_TX_BUSY;
}
@@ -1880,6 +1884,7 @@ static int qlcnic_process_cmd_ring(struct qlcnic_adapter *adapter)
if (qlcnic_tx_avail(tx_ring) > TX_STOP_THRESH) {
netif_wake_queue(netdev);
adapter->tx_timeo_cnt = 0;
+ adapter->stats.xmit_on++;
}
__netif_tx_unlock(tx_ring->txq);
}
--
1.6.0.2
^ permalink raw reply related
* [PATCH 4/6] qlcnic: validate unified fw image
From: Amit Kumar Salecha @ 2010-03-08 10:14 UTC (permalink / raw)
To: davem; +Cc: netdev, dhananjay.phadke, ameen.rahman, Sucheta Chakraborty
In-Reply-To: <1268043290-19929-1-git-send-email-amit.salecha@qlogic.com>
From: Sucheta Chakraborty <sucheta@dut6195.unminc.com>
Validate all sections of unified fw image, before accessing them,
to avoid seg fault.
Signed-off-by: Sucheta Chakraborty <sucheta@dut6195.unminc.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
drivers/net/qlcnic/qlcnic_init.c | 146 ++++++++++++++++++++++++++++++++++++--
1 files changed, 139 insertions(+), 7 deletions(-)
diff --git a/drivers/net/qlcnic/qlcnic_init.c b/drivers/net/qlcnic/qlcnic_init.c
index f0df971..21a6e9f 100644
--- a/drivers/net/qlcnic/qlcnic_init.c
+++ b/drivers/net/qlcnic/qlcnic_init.c
@@ -568,21 +568,123 @@ struct uni_table_desc *qlcnic_get_table_desc(const u8 *unirom, int section)
return NULL;
}
+#define FILEHEADER_SIZE (14 * 4)
+
static int
-qlcnic_set_product_offs(struct qlcnic_adapter *adapter)
+qlcnic_validate_header(struct qlcnic_adapter *adapter)
{
- struct uni_table_desc *ptab_descr;
const u8 *unirom = adapter->fw->data;
- u32 i;
+ struct uni_table_desc *directory = (struct uni_table_desc *) &unirom[0];
+ __le32 fw_file_size = adapter->fw->size;
__le32 entries;
+ __le32 entry_size;
+ __le32 tab_size;
+
+ if (fw_file_size < FILEHEADER_SIZE)
+ return -EINVAL;
+
+ entries = cpu_to_le32(directory->num_entries);
+ entry_size = cpu_to_le32(directory->entry_size);
+ tab_size = cpu_to_le32(directory->findex) + (entries * entry_size);
+
+ if (fw_file_size < tab_size)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int
+qlcnic_validate_bootld(struct qlcnic_adapter *adapter)
+{
+ struct uni_table_desc *tab_desc;
+ struct uni_data_desc *descr;
+ const u8 *unirom = adapter->fw->data;
+ int idx = cpu_to_le32(*((int *)&unirom[adapter->file_prd_off] +
+ QLCNIC_UNI_BOOTLD_IDX_OFF));
+ __le32 offs;
+ __le32 tab_size;
+ __le32 data_size;
+
+ tab_desc = qlcnic_get_table_desc(unirom, QLCNIC_UNI_DIR_SECT_BOOTLD);
+
+ if (!tab_desc)
+ return -EINVAL;
+
+ tab_size = cpu_to_le32(tab_desc->findex) +
+ (cpu_to_le32(tab_desc->entry_size * (idx + 1)));
+
+ if (adapter->fw->size < tab_size)
+ return -EINVAL;
+
+ offs = cpu_to_le32(tab_desc->findex) +
+ (cpu_to_le32(tab_desc->entry_size) * (idx));
+ descr = (struct uni_data_desc *)&unirom[offs];
+
+ data_size = descr->findex + cpu_to_le32(descr->size);
+
+ if (adapter->fw->size < data_size)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int
+qlcnic_validate_fw(struct qlcnic_adapter *adapter)
+{
+ struct uni_table_desc *tab_desc;
+ struct uni_data_desc *descr;
+ const u8 *unirom = adapter->fw->data;
+ int idx = cpu_to_le32(*((int *)&unirom[adapter->file_prd_off] +
+ QLCNIC_UNI_FIRMWARE_IDX_OFF));
+ __le32 offs;
+ __le32 tab_size;
+ __le32 data_size;
+
+ tab_desc = qlcnic_get_table_desc(unirom, QLCNIC_UNI_DIR_SECT_FW);
+
+ if (!tab_desc)
+ return -EINVAL;
+
+ tab_size = cpu_to_le32(tab_desc->findex) +
+ (cpu_to_le32(tab_desc->entry_size * (idx + 1)));
+
+ if (adapter->fw->size < tab_size)
+ return -EINVAL;
+
+ offs = cpu_to_le32(tab_desc->findex) +
+ (cpu_to_le32(tab_desc->entry_size) * (idx));
+ descr = (struct uni_data_desc *)&unirom[offs];
+ data_size = descr->findex + cpu_to_le32(descr->size);
+
+ if (adapter->fw->size < data_size)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int
+qlcnic_validate_product_offs(struct qlcnic_adapter *adapter)
+{
+ struct uni_table_desc *ptab_descr;
+ const u8 *unirom = adapter->fw->data;
int mn_present = qlcnic_has_mn(adapter);
+ __le32 entries;
+ __le32 entry_size;
+ __le32 tab_size;
+ u32 i;
ptab_descr = qlcnic_get_table_desc(unirom,
QLCNIC_UNI_DIR_SECT_PRODUCT_TBL);
- if (ptab_descr == NULL)
- return -1;
+ if (!ptab_descr)
+ return -EINVAL;
entries = cpu_to_le32(ptab_descr->num_entries);
+ entry_size = cpu_to_le32(ptab_descr->entry_size);
+ tab_size = cpu_to_le32(ptab_descr->findex) + (entries * entry_size);
+
+ if (adapter->fw->size < tab_size)
+ return -EINVAL;
+
nomn:
for (i = 0; i < entries; i++) {
@@ -609,7 +711,37 @@ nomn:
mn_present = 0;
goto nomn;
}
- return -1;
+ return -EINVAL;
+}
+
+static int
+qlcnic_validate_unified_romimage(struct qlcnic_adapter *adapter)
+{
+ if (qlcnic_validate_header(adapter)) {
+ dev_err(&adapter->pdev->dev,
+ "unified image: header validation failed\n");
+ return -EINVAL;
+ }
+
+ if (qlcnic_validate_product_offs(adapter)) {
+ dev_err(&adapter->pdev->dev,
+ "unified image: product validation failed\n");
+ return -EINVAL;
+ }
+
+ if (qlcnic_validate_bootld(adapter)) {
+ dev_err(&adapter->pdev->dev,
+ "unified image: bootld validation failed\n");
+ return -EINVAL;
+ }
+
+ if (qlcnic_validate_fw(adapter)) {
+ dev_err(&adapter->pdev->dev,
+ "unified image: firmware validation failed\n");
+ return -EINVAL;
+ }
+
+ return 0;
}
static
@@ -858,7 +990,7 @@ qlcnic_validate_firmware(struct qlcnic_adapter *adapter)
u8 fw_type = adapter->fw_type;
if (fw_type == QLCNIC_UNIFIED_ROMIMAGE) {
- if (qlcnic_set_product_offs(adapter))
+ if (qlcnic_validate_unified_romimage(adapter))
return -EINVAL;
min_size = QLCNIC_UNI_FW_MIN_SIZE;
--
1.6.0.2
^ permalink raw reply related
* [PATCH 1/6] qlcnic: fix tx csum status
From: Amit Kumar Salecha @ 2010-03-08 10:14 UTC (permalink / raw)
To: davem; +Cc: netdev, dhananjay.phadke, ameen.rahman, Sucheta Chakraborty
In-Reply-To: <1268043290-19929-1-git-send-email-amit.salecha@qlogic.com>
From: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
Kernel default tx csum function (ethtool_op_get_tx_csum) doesn't show
correct csum status. It takes various FLAGS (NETIF_F_ALL_CSUM) in account
to show tx csum status, which driver doesn't set while disabling tx csum.
Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
drivers/net/qlcnic/qlcnic_ethtool.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/net/qlcnic/qlcnic_ethtool.c b/drivers/net/qlcnic/qlcnic_ethtool.c
index 8da6ec8..ef12792 100644
--- a/drivers/net/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/qlcnic/qlcnic_ethtool.c
@@ -785,6 +785,11 @@ qlcnic_get_ethtool_stats(struct net_device *dev,
}
}
+static u32 qlcnic_get_tx_csum(struct net_device *dev)
+{
+ return dev->features & NETIF_F_IP_CSUM;
+}
+
static u32 qlcnic_get_rx_csum(struct net_device *dev)
{
struct qlcnic_adapter *adapter = netdev_priv(dev);
@@ -995,6 +1000,7 @@ const struct ethtool_ops qlcnic_ethtool_ops = {
.set_ringparam = qlcnic_set_ringparam,
.get_pauseparam = qlcnic_get_pauseparam,
.set_pauseparam = qlcnic_set_pauseparam,
+ .get_tx_csum = qlcnic_get_tx_csum,
.set_tx_csum = ethtool_op_set_tx_csum,
.set_sg = ethtool_op_set_sg,
.get_tso = qlcnic_get_tso,
--
1.6.0.2
^ permalink raw reply related
* [PATCH 0/6]qlcnic: bug fixes
From: Amit Kumar Salecha @ 2010-03-08 10:14 UTC (permalink / raw)
To: davem; +Cc: netdev, dhananjay.phadke, ameen.rahman
Hi
Series of 6 patches to fix minor bugs and to enhance driver statistics.
Please apply them in net-2.6 tree.
-Amit Salecha
^ permalink raw reply
* [PATCH 5/6] qlcnic: fix bios version check
From: Amit Kumar Salecha @ 2010-03-08 10:14 UTC (permalink / raw)
To: davem; +Cc: netdev, dhananjay.phadke, ameen.rahman
In-Reply-To: <1268043290-19929-1-git-send-email-amit.salecha@qlogic.com>
Bios sub version from unified fw image is calculated incorrect.
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
drivers/net/qlcnic/qlcnic_init.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/qlcnic/qlcnic_init.c b/drivers/net/qlcnic/qlcnic_init.c
index 21a6e9f..7c34e4e 100644
--- a/drivers/net/qlcnic/qlcnic_init.c
+++ b/drivers/net/qlcnic/qlcnic_init.c
@@ -847,7 +847,7 @@ qlcnic_get_bios_version(struct qlcnic_adapter *adapter)
bios_ver = cpu_to_le32(*((u32 *) (&fw->data[prd_off])
+ QLCNIC_UNI_BIOS_VERSION_OFF));
- return (bios_ver << 24) + ((bios_ver >> 8) & 0xff00) + (bios_ver >> 24);
+ return (bios_ver << 16) + ((bios_ver >> 8) & 0xff00) + (bios_ver >> 24);
}
int
--
1.6.0.2
^ permalink raw reply related
* [PATCH 6/6] qlcnic: remove extra space from board names
From: Amit Kumar Salecha @ 2010-03-08 10:14 UTC (permalink / raw)
To: davem; +Cc: netdev, dhananjay.phadke, ameen.rahman
In-Reply-To: <1268043290-19929-1-git-send-email-amit.salecha@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
drivers/net/qlcnic/qlcnic.h | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/qlcnic/qlcnic.h b/drivers/net/qlcnic/qlcnic.h
index 9897b69..0da94b2 100644
--- a/drivers/net/qlcnic/qlcnic.h
+++ b/drivers/net/qlcnic/qlcnic.h
@@ -1100,11 +1100,11 @@ struct qlcnic_brdinfo {
static const struct qlcnic_brdinfo qlcnic_boards[] = {
{0x1077, 0x8020, 0x1077, 0x203,
- "8200 Series Single Port 10GbE Converged Network Adapter \
- (TCP/IP Networking)"},
+ "8200 Series Single Port 10GbE Converged Network Adapter "
+ "(TCP/IP Networking)"},
{0x1077, 0x8020, 0x1077, 0x207,
- "8200 Series Dual Port 10GbE Converged Network Adapter \
- (TCP/IP Networking)"},
+ "8200 Series Dual Port 10GbE Converged Network Adapter "
+ "(TCP/IP Networking)"},
{0x1077, 0x8020, 0x1077, 0x20b,
"3200 Series Dual Port 10Gb Intelligent Ethernet Adapter"},
{0x1077, 0x8020, 0x1077, 0x20c,
--
1.6.0.2
^ permalink raw reply related
* RE: bnx2x crash dump on high network load
From: Fischer, Anna @ 2010-03-08 11:02 UTC (permalink / raw)
To: Eilon Greenstein; +Cc: eliezert, Michael Chan, netdev@vger.kernel.org
In-Reply-To: <BD3F7F1EFBA6D54DB056C4FFA451400803A1F8F2F7@SJEXCHCCR01.corp.ad.broadcom.com>
I have this option set in my /etc/modprobe.conf.local. The problem must be something else.
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Eilon Greenstein
> Sent: 05 March 2010 17:32
> To: Fischer, Anna
> Cc: eliezert; Michael Chan; netdev@vger.kernel.org
> Subject: RE: bnx2x crash dump on high network load
>
> Hi Anna,
>
> On this kernel, enabling bridging (like you do on virtualization) does
> not turn off LRO automatically. Please try to load the bnx2x with
> disable_tpa=1 (TPA is the HW based LRO feature).
>
> Regards,
> Eilon
>
> -----Original Message-----
> From: Fischer, Anna [mailto:anna.fischer@hp.com]
> Sent: Friday, March 05, 2010 7:08 PM
> To: netdev@vger.kernel.org
> Cc: eliezert; Eilon Greenstein; Michael Chan
> Subject: bnx2x crash dump on high network load
>
> I am seeing driver crashes with a bnx2x version 1.50.16. I have attached
> two crash dumps that I have captured.
>
> I run Linux with the following kernel:
>
> # uname -a
> Linux sup-prj-441106 2.6.16.60-0.54.5-debug #1 SMP Fri Sep 4 01:28:03
> UTC 2009 x86_64 x86_64 x86_64 GNU/Linux
>
> I am seeing these driver crashes when I run virtual machines (VMware
> Server) and those VMs do high bandwidth network I/O, so there are a lot
> of packets going through the NIC.
>
> Can you give me any hints for what the problem here could be?
>
> Thanks,
> Anna
>
> --
> 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
^ permalink raw reply
* Re: [PATCH v1 3/3] Let host NIC driver to DMA to guest user space.
From: Michael S. Tsirkin @ 2010-03-08 11:18 UTC (permalink / raw)
To: xiaohui.xin; +Cc: netdev, kvm, linux-kernel, mingo, jdike, Zhao Yu
In-Reply-To: <1267868318-19268-4-git-send-email-xiaohui.xin@intel.com>
On Sat, Mar 06, 2010 at 05:38:38PM +0800, xiaohui.xin@intel.com wrote:
> From: Xin Xiaohui <xiaohui.xin@intel.com>
>
> The patch let host NIC driver to receive user space skb,
> then the driver has chance to directly DMA to guest user
> space buffers thru single ethX interface.
>
> Signed-off-by: Xin Xiaohui <xiaohui.xin@intel.com>
> Signed-off-by: Zhao Yu <yzhao81@gmail.com>
> Sigend-off-by: Jeff Dike <jdike@c2.user-mode-linux.org>
I have a feeling I commented on some of the below issues already.
Do you plan to send a version with comments addressed?
> ---
> include/linux/netdevice.h | 76 ++++++++++++++++++++++++++++++++++++++++++-
> include/linux/skbuff.h | 30 +++++++++++++++--
> net/core/dev.c | 32 ++++++++++++++++++
> net/core/skbuff.c | 79 +++++++++++++++++++++++++++++++++++++++++----
> 4 files changed, 205 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 94958c1..97bf12c 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -485,6 +485,17 @@ struct netdev_queue {
> unsigned long tx_dropped;
> } ____cacheline_aligned_in_smp;
>
> +#if defined(CONFIG_VHOST_PASSTHRU) || defined(CONFIG_VHOST_PASSTHRU_MODULE)
> +struct mpassthru_port {
> + int hdr_len;
> + int data_len;
> + int npages;
> + unsigned flags;
> + struct socket *sock;
> + struct skb_user_page *(*ctor)(struct mpassthru_port *,
> + struct sk_buff *, int);
> +};
> +#endif
>
> /*
> * This structure defines the management hooks for network devices.
> @@ -636,6 +647,10 @@ struct net_device_ops {
> int (*ndo_fcoe_ddp_done)(struct net_device *dev,
> u16 xid);
> #endif
> +#if defined(CONFIG_VHOST_PASSTHRU) || defined(CONFIG_VHOST_PASSTHRU_MODULE)
> + int (*ndo_mp_port_prep)(struct net_device *dev,
> + struct mpassthru_port *port);
> +#endif
> };
>
> /*
> @@ -891,7 +906,8 @@ struct net_device
> struct macvlan_port *macvlan_port;
> /* GARP */
> struct garp_port *garp_port;
> -
> + /* mpassthru */
> + struct mpassthru_port *mp_port;
> /* class/net/name entry */
> struct device dev;
> /* space for optional statistics and wireless sysfs groups */
> @@ -2013,6 +2029,62 @@ static inline u32 dev_ethtool_get_flags(struct net_device *dev)
> return 0;
> return dev->ethtool_ops->get_flags(dev);
> }
> -#endif /* __KERNEL__ */
>
> +#if defined(CONFIG_VHOST_PASSTHRU) || defined(CONFIG_VHOST_PASSTHRU_MODULE)
> +static inline int netdev_mp_port_prep(struct net_device *dev,
> + struct mpassthru_port *port)
> +{
This function lacks documentation.
> + int rc;
> + int npages, data_len;
> + const struct net_device_ops *ops = dev->netdev_ops;
> +
> + /* needed by packet split */
> + if (ops->ndo_mp_port_prep) {
> + rc = ops->ndo_mp_port_prep(dev, port);
> + if (rc)
> + return rc;
> + } else { /* should be temp */
> + port->hdr_len = 128;
> + port->data_len = 2048;
> + port->npages = 1;
where do the numbers come from?
> + }
> +
> + if (port->hdr_len <= 0)
> + goto err;
> +
> + npages = port->npages;
> + data_len = port->data_len;
> + if (npages <= 0 || npages > MAX_SKB_FRAGS ||
> + (data_len < PAGE_SIZE * (npages - 1) ||
> + data_len > PAGE_SIZE * npages))
> + goto err;
> +
> + return 0;
> +err:
> + dev_warn(&dev->dev, "invalid page constructor parameters\n");
> +
> + return -EINVAL;
> +}
> +
> +static inline int netdev_mp_port_attach(struct net_device *dev,
> + struct mpassthru_port *port)
> +{
> + if (rcu_dereference(dev->mp_port))
> + return -EBUSY;
> +
> + rcu_assign_pointer(dev->mp_port, port);
> +
> + return 0;
> +}
> +
> +static inline void netdev_mp_port_detach(struct net_device *dev)
> +{
> + if (!rcu_dereference(dev->mp_port))
> + return;
> +
> + rcu_assign_pointer(dev->mp_port, NULL);
> + synchronize_rcu();
> +}
The above looks wrong, rcu_dereference should be called
under rcu read side, rcu_assign_pointer usually should not,
synchronize_rcu definitely should not.
As I suggested already, these functions are better opencoded,
rcu is tricky as is without hiding it in inline helpers.
> +#endif /* CONFIG_VHOST_PASSTHRU */
> +#endif /* __KERNEL__ */
> #endif /* _LINUX_NETDEVICE_H */
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index df7b23a..e59fa57 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -209,6 +209,13 @@ struct skb_shared_info {
> void * destructor_arg;
> };
>
> +struct skb_user_page {
> + u8 *start;
> + int size;
> + struct skb_frag_struct *frags;
> + struct skb_shared_info *ushinfo;
> + void (*dtor)(struct skb_user_page *);
> +};
> /* We divide dataref into two halves. The higher 16 bits hold references
> * to the payload part of skb->data. The lower 16 bits hold references to
> * the entire skb->data. A clone of a headerless skb holds the length of
> @@ -441,17 +448,18 @@ extern void kfree_skb(struct sk_buff *skb);
> extern void consume_skb(struct sk_buff *skb);
> extern void __kfree_skb(struct sk_buff *skb);
> extern struct sk_buff *__alloc_skb(unsigned int size,
> - gfp_t priority, int fclone, int node);
> + gfp_t priority, int fclone,
> + int node, struct net_device *dev);
> static inline struct sk_buff *alloc_skb(unsigned int size,
> gfp_t priority)
> {
> - return __alloc_skb(size, priority, 0, -1);
> + return __alloc_skb(size, priority, 0, -1, NULL);
> }
>
> static inline struct sk_buff *alloc_skb_fclone(unsigned int size,
> gfp_t priority)
> {
> - return __alloc_skb(size, priority, 1, -1);
> + return __alloc_skb(size, priority, 1, -1, NULL);
> }
>
> extern int skb_recycle_check(struct sk_buff *skb, int skb_size);
> @@ -1509,6 +1517,22 @@ static inline void netdev_free_page(struct net_device *dev, struct page *page)
> __free_page(page);
> }
>
> +extern struct skb_user_page *netdev_alloc_user_pages(struct net_device *dev,
> + struct sk_buff *skb, int npages);
> +
> +static inline struct skb_user_page *netdev_alloc_user_page(
> + struct net_device *dev,
> + struct sk_buff *skb, unsigned int size)
> +{
> + struct skb_user_page *user;
> + int npages = (size < PAGE_SIZE) ? 1 : (size / PAGE_SIZE);
Should round up to full pages?
> +
> + user = netdev_alloc_user_pages(dev, skb, npages);
> + if (likely(user))
> + return user;
> + return NULL;
> +}
> +
> /**
> * skb_clone_writable - is the header of a clone writable
> * @skb: buffer to check
> diff --git a/net/core/dev.c b/net/core/dev.c
> index b8f74cf..ab8b082 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2265,6 +2265,30 @@ void netif_nit_deliver(struct sk_buff *skb)
> rcu_read_unlock();
> }
>
> +#if defined(CONFIG_VHOST_PASSTHRU) || defined(CONFIG_VHOST_PASSTHRU_MODULE)
> +static inline struct sk_buff *handle_mpassthru(struct sk_buff *skb,
> + struct packet_type **pt_prev,
> + int *ret, struct net_device *orig_dev)
please document
pt_prev, orig_dev and ret seem unused?
> +{
> + struct mpassthru_port *ctor = NULL;
Why do you call the port "ctor"?
> + struct sock *sk = NULL;
> +
> + if (skb->dev)
> + ctor = skb->dev->mp_port;
> + if (!ctor)
> + return skb;
> +
> + sk = ctor->sock->sk;
> +
> + skb_queue_tail(&sk->sk_receive_queue, skb);
> +
> + sk->sk_data_ready(sk, skb->len);
> + return NULL;
> +}
> +#else
> +#define handle_mpassthru(skb, pt_prev, ret, orig_dev) (skb)
> +#endif
> +
> /**
> * netif_receive_skb - process receive buffer from network
> * @skb: buffer to process
> @@ -2342,6 +2366,9 @@ int netif_receive_skb(struct sk_buff *skb)
> goto out;
> ncls:
> #endif
> + skb = handle_mpassthru(skb, &pt_prev, &ret, orig_dev);
> + if (!skb)
> + goto out;
>
> skb = handle_bridge(skb, &pt_prev, &ret, orig_dev);
> if (!skb)
> @@ -2455,6 +2482,11 @@ int dev_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
> if (skb_is_gso(skb) || skb_has_frags(skb))
> goto normal;
>
> +#if defined(CONFIG_VHOST_PASSTHRU) || defined(CONFIG_VHOST_PASSTHRU_MODULE)
> + if (skb->dev && skb->dev->mp_port)
> + goto normal;
> +#endif
> +
> rcu_read_lock();
> list_for_each_entry_rcu(ptype, head, list) {
> if (ptype->type != type || ptype->dev || !ptype->gro_receive)
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 80a9616..6510e5b 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -170,13 +170,15 @@ EXPORT_SYMBOL(skb_under_panic);
> * %GFP_ATOMIC.
> */
> struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
> - int fclone, int node)
> + int fclone, int node, struct net_device *dev)
> {
> struct kmem_cache *cache;
> struct skb_shared_info *shinfo;
> struct sk_buff *skb;
> u8 *data;
> -
> +#if defined(CONFIG_VHOST_PASSTHRU) || defined(CONFIG_VHOST_PASSTHRU_MODULE)
> + struct skb_user_page *user = NULL;
> +#endif
> cache = fclone ? skbuff_fclone_cache : skbuff_head_cache;
>
> /* Get the HEAD */
> @@ -185,8 +187,26 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
> goto out;
>
> size = SKB_DATA_ALIGN(size);
> - data = kmalloc_node_track_caller(size + sizeof(struct skb_shared_info),
> - gfp_mask, node);
> +#if defined(CONFIG_VHOST_PASSTHRU) || defined(CONFIG_VHOST_PASSTHRU_MODULE)
> + if (!dev || !dev->mp_port) { /* Legacy alloc func */
> +#endif
> + data = kmalloc_node_track_caller(
> + size + sizeof(struct skb_shared_info),
> + gfp_mask, node);
> +#if defined(CONFIG_VHOST_PASSTHRU) || defined(CONFIG_VHOST_PASSTHRU_MODULE)
> + } else { /* Allocation may from page constructor of device */
what does the comment mean?
> + user = netdev_alloc_user_page(dev, skb, size);
> + if (!user) {
> + data = kmalloc_node_track_caller(
> + size + sizeof(struct skb_shared_info),
> + gfp_mask, node);
> + printk(KERN_INFO "can't alloc user buffer.\n");
> + } else {
> + data = user->start;
> + size = SKB_DATA_ALIGN(user->size);
> + }
> + }
> +#endif
> if (!data)
> goto nodata;
>
> @@ -208,6 +228,11 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
> skb->mac_header = ~0U;
> #endif
>
> +#if defined(CONFIG_VHOST_PASSTHRU) || defined(CONFIG_VHOST_PASSTHRU_MODULE)
> + if (user)
> + memcpy(user->ushinfo, skb_shinfo(skb),
> + sizeof(struct skb_shared_info));
> +#endif
> /* make sure we initialize shinfo sequentially */
> shinfo = skb_shinfo(skb);
> atomic_set(&shinfo->dataref, 1);
> @@ -231,6 +256,10 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
>
> child->fclone = SKB_FCLONE_UNAVAILABLE;
> }
> +#if defined(CONFIG_VHOST_PASSTHRU) || defined(CONFIG_VHOST_PASSTHRU_MODULE)
> + shinfo->destructor_arg = user;
> +#endif
> +
> out:
> return skb;
> nodata:
> @@ -259,7 +288,7 @@ struct sk_buff *__netdev_alloc_skb(struct net_device *dev,
> int node = dev->dev.parent ? dev_to_node(dev->dev.parent) : -1;
> struct sk_buff *skb;
>
> - skb = __alloc_skb(length + NET_SKB_PAD, gfp_mask, 0, node);
> + skb = __alloc_skb(length + NET_SKB_PAD, gfp_mask, 0, node, dev);
> if (likely(skb)) {
> skb_reserve(skb, NET_SKB_PAD);
> skb->dev = dev;
> @@ -278,6 +307,29 @@ struct page *__netdev_alloc_page(struct net_device *dev, gfp_t gfp_mask)
> }
> EXPORT_SYMBOL(__netdev_alloc_page);
>
> +#if defined(CONFIG_VHOST_PASSTHRU) || defined(CONFIG_VHOST_PASSTHRU_MODULE)
> +struct skb_user_page *netdev_alloc_user_pages(struct net_device *dev,
> + struct sk_buff *skb, int npages)
> +{
> + struct mpassthru_port *ctor;
> + struct skb_user_page *user = NULL;
> +
> + rcu_read_lock();
> + ctor = rcu_dereference(dev->mp_port);
> + if (!ctor)
> + goto out;
> +
> + BUG_ON(npages > ctor->npages);
> +
> + user = ctor->ctor(ctor, skb, npages);
With the assumption that "ctor" pins userspace pages,
can't it sleep? If yes you can't call it under rcu read side
critical section.
> +out:
> + rcu_read_unlock();
> +
> + return user;
> +}
> +EXPORT_SYMBOL(netdev_alloc_user_pages);
> +#endif
> +
> void skb_add_rx_frag(struct sk_buff *skb, int i, struct page *page, int off,
> int size)
> {
> @@ -338,6 +390,10 @@ static void skb_clone_fraglist(struct sk_buff *skb)
>
> static void skb_release_data(struct sk_buff *skb)
> {
> +#if defined(CONFIG_VHOST_PASSTHRU) || defined(CONFIG_VHOST_PASSTHRU_MODULE)
> + struct skb_user_page *user = skb_shinfo(skb)->destructor_arg;
> +#endif
> +
> if (!skb->cloned ||
> !atomic_sub_return(skb->nohdr ? (1 << SKB_DATAREF_SHIFT) + 1 : 1,
> &skb_shinfo(skb)->dataref)) {
> @@ -349,7 +405,10 @@ static void skb_release_data(struct sk_buff *skb)
>
> if (skb_has_frags(skb))
> skb_drop_fraglist(skb);
> -
> +#if defined(CONFIG_VHOST_PASSTHRU) || defined(CONFIG_VHOST_PASSTHRU_MODULE)
> + if (skb->dev && skb->dev->mp_port && user && user->dtor)
> + user->dtor(user);
> +#endif
> kfree(skb->head);
> }
> }
> @@ -503,8 +562,14 @@ int skb_recycle_check(struct sk_buff *skb, int skb_size)
> if (skb_shared(skb) || skb_cloned(skb))
> return 0;
>
> - skb_release_head_state(skb);
> +#if defined(CONFIG_VHOST_PASSTHRU) || defined(CONFIG_VHOST_PASSTHRU_MODULE)
> + if (skb->dev && skb->dev->mp_port)
> + return 0;
> +#endif
> +
> shinfo = skb_shinfo(skb);
> +
> + skb_release_head_state(skb);
> atomic_set(&shinfo->dataref, 1);
> shinfo->nr_frags = 0;
> shinfo->gso_size = 0;
> --
> 1.5.4.4
^ permalink raw reply
* Re: [PATCH v1 1/3] A device for zero-copy based on KVM virtio-net.
From: Michael S. Tsirkin @ 2010-03-08 11:28 UTC (permalink / raw)
To: xiaohui.xin; +Cc: netdev, kvm, linux-kernel, mingo, jdike, Zhao Yu
In-Reply-To: <1267868318-19268-2-git-send-email-xiaohui.xin@intel.com>
On Sat, Mar 06, 2010 at 05:38:36PM +0800, xiaohui.xin@intel.com wrote:
> From: Xin Xiaohui <xiaohui.xin@intel.com>
>
> Add a device to utilize the vhost-net backend driver for
> copy-less data transfer between guest FE and host NIC.
> It pins the guest user space to the host memory and
> provides proto_ops as sendmsg/recvmsg to vhost-net.
>
> Signed-off-by: Xin Xiaohui <xiaohui.xin@intel.com>
> Signed-off-by: Zhao Yu <yzhao81@gmail.com>
> Sigend-off-by: Jeff Dike <jdike@c2.user-mode-linux.org>
I think some of the comments below are repeated.
Do you plan addressing them?
> ---
> drivers/vhost/Kconfig | 5 +
> drivers/vhost/Makefile | 2 +
> drivers/vhost/mpassthru.c | 1202 +++++++++++++++++++++++++++++++++++++++++++++
> include/linux/mpassthru.h | 29 ++
I'm not sure it's wise to limit the device to
vhost even if that's the only mode that you are going to
support in the first version.
How about locating the char device under drivers/net/?
> 4 files changed, 1238 insertions(+), 0 deletions(-)
> create mode 100644 drivers/vhost/mpassthru.c
> create mode 100644 include/linux/mpassthru.h
>
> diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
> index 9f409f4..ee32a3b 100644
> --- a/drivers/vhost/Kconfig
> +++ b/drivers/vhost/Kconfig
> @@ -9,3 +9,8 @@ config VHOST_NET
> To compile this driver as a module, choose M here: the module will
> be called vhost_net.
>
> +config VHOST_PASSTHRU
> + tristate "Zerocopy network driver (EXPERIMENTAL)"
> + depends on VHOST_NET
> + ---help---
> + zerocopy network I/O support
> diff --git a/drivers/vhost/Makefile b/drivers/vhost/Makefile
> index 72dd020..3f79c79 100644
> --- a/drivers/vhost/Makefile
> +++ b/drivers/vhost/Makefile
> @@ -1,2 +1,4 @@
> obj-$(CONFIG_VHOST_NET) += vhost_net.o
> vhost_net-y := vhost.o net.o
> +
> +obj-$(CONFIG_VHOST_PASSTHRU) += mpassthru.o
> diff --git a/drivers/vhost/mpassthru.c b/drivers/vhost/mpassthru.c
> new file mode 100644
> index 0000000..744d6cd
> --- /dev/null
> +++ b/drivers/vhost/mpassthru.c
> @@ -0,0 +1,1202 @@
> +/*
> + * MPASSTHRU - Mediate passthrough device.
> + * Copyright (C) 2009 ZhaoYu, XinXiaohui, Dike, Jeffery G
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#define DRV_NAME "mpassthru"
> +#define DRV_DESCRIPTION "Mediate passthru device driver"
> +#define DRV_COPYRIGHT "(C) 2009 ZhaoYu, XinXiaohui, Dike, Jeffery G"
> +
> +#include <linux/module.h>
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +#include <linux/major.h>
> +#include <linux/slab.h>
> +#include <linux/smp_lock.h>
> +#include <linux/poll.h>
> +#include <linux/fcntl.h>
> +#include <linux/init.h>
> +#include <linux/skbuff.h>
> +#include <linux/netdevice.h>
> +#include <linux/etherdevice.h>
> +#include <linux/miscdevice.h>
> +#include <linux/ethtool.h>
> +#include <linux/rtnetlink.h>
> +#include <linux/if.h>
> +#include <linux/if_arp.h>
> +#include <linux/if_ether.h>
> +#include <linux/crc32.h>
> +#include <linux/nsproxy.h>
> +#include <linux/uaccess.h>
> +#include <linux/virtio_net.h>
> +#include <linux/mpassthru.h>
> +#include <net/net_namespace.h>
> +#include <net/netns/generic.h>
> +#include <net/rtnetlink.h>
> +#include <net/sock.h>
> +
> +#include <asm/system.h>
> +
> +#include "vhost.h"
> +
> +/* Uncomment to enable debugging */
> +/* #define MPASSTHRU_DEBUG 1 */
> +
> +#ifdef MPASSTHRU_DEBUG
> +static int debug;
> +
> +#define DBG if (mp->debug) printk
> +#define DBG1 if (debug == 2) printk
> +#else
> +#define DBG(a...)
> +#define DBG1(a...)
> +#endif
> +
> +#define COPY_THRESHOLD (L1_CACHE_BYTES * 4)
> +#define COPY_HDR_LEN (L1_CACHE_BYTES < 64 ? 64 : L1_CACHE_BYTES)
> +
> +struct frag {
> + u16 offset;
> + u16 size;
> +};
> +
> +struct page_ctor {
> + struct list_head readq;
> + int w_len;
> + int r_len;
> + spinlock_t read_lock;
> + atomic_t refcnt;
> + struct kmem_cache *cache;
> + struct net_device *dev;
> + struct mpassthru_port port;
> + void *sendctrl;
> + void *recvctrl;
> +};
> +
> +struct page_info {
> + struct list_head list;
> + int header;
> + /* indicate the actual length of bytes
> + * send/recv in the user space buffers
> + */
> + int total;
> + int offset;
> + struct page *pages[MAX_SKB_FRAGS+1];
> + struct skb_frag_struct frag[MAX_SKB_FRAGS+1];
> + struct sk_buff *skb;
> + struct page_ctor *ctor;
> +
> + /* The pointer relayed to skb, to indicate
> + * it's a user space allocated skb or kernel
> + */
> + struct skb_user_page user;
> + struct skb_shared_info ushinfo;
> +
> +#define INFO_READ 0
> +#define INFO_WRITE 1
> + unsigned flags;
> + unsigned pnum;
> +
> + /* It's meaningful for receive, means
> + * the max length allowed
> + */
> + size_t len;
> +
> + /* The fields after that is for backend
> + * driver, now for vhost-net.
> + */
> + struct vhost_notifier notifier;
> + unsigned int desc_pos;
> + unsigned int log;
> + struct iovec hdr[VHOST_NET_MAX_SG];
> + struct iovec iov[VHOST_NET_MAX_SG];
> + void *ctl;
> +};
> +
> +struct mp_struct {
> + struct mp_file *mfile;
> + struct net_device *dev;
> + struct page_ctor *ctor;
> + struct socket socket;
> +
> +#ifdef MPASSTHRU_DEBUG
> + int debug;
> +#endif
> +};
> +
> +struct mp_file {
> + atomic_t count;
> + struct mp_struct *mp;
> + struct net *net;
> +};
> +
> +struct mp_sock {
> + struct sock sk;
> + struct mp_struct *mp;
> +};
> +
> +static int mp_dev_change_flags(struct net_device *dev, unsigned flags)
> +{
> + int ret = 0;
> +
> + rtnl_lock();
> + ret = dev_change_flags(dev, flags);
> + rtnl_unlock();
> +
> + if (ret < 0)
> + printk(KERN_ERR "failed to change dev state of %s", dev->name);
> +
> + return ret;
> +}
> +
> +/* The main function to allocate user space buffers */
> +static struct skb_user_page *page_ctor(struct mpassthru_port *port,
> + struct sk_buff *skb, int npages)
> +{
> + int i;
> + unsigned long flags;
> + struct page_ctor *ctor;
> + struct page_info *info = NULL;
> +
> + ctor = container_of(port, struct page_ctor, port);
> +
> + spin_lock_irqsave(&ctor->read_lock, flags);
> + if (!list_empty(&ctor->readq)) {
> + info = list_first_entry(&ctor->readq, struct page_info, list);
> + list_del(&info->list);
> + }
> + spin_unlock_irqrestore(&ctor->read_lock, flags);
> + if (!info)
> + return NULL;
> +
> + for (i = 0; i < info->pnum; i++) {
> + get_page(info->pages[i]);
> + info->frag[i].page = info->pages[i];
> + info->frag[i].page_offset = i ? 0 : info->offset;
> + info->frag[i].size = port->npages > 1 ? PAGE_SIZE :
> + port->data_len;
> + }
> + info->skb = skb;
> + info->user.frags = info->frag;
> + info->user.ushinfo = &info->ushinfo;
> + return &info->user;
> +}
> +
> +static struct vhost_notifier *create_vhost_notifier(struct vhost_virtqueue *vq,
> + struct page_info *info, int size);
> +
> +static void mp_vhost_notifier_dtor(struct vhost_notifier *vnotify)
> +{
> + struct page_info *info = (struct page_info *)(vnotify->ctrl);
> + int i;
> +
> + for (i = 0; i < info->pnum; i++) {
> + if (info->pages[i])
> + put_page(info->pages[i]);
> + }
> +
> + if (info->flags == INFO_READ) {
> + skb_shinfo(info->skb)->destructor_arg = &info->user;
> + info->skb->destructor = NULL;
> + kfree(info->skb);
> + }
> +
> + kmem_cache_free(info->ctor->cache, info);
> +
> + return;
> +}
> +
> +/* A helper to clean the skb before the kfree_skb() */
> +
> +static void page_dtor_prepare(struct page_info *info)
> +{
> + if (info->flags == INFO_READ)
> + if (info->skb)
> + info->skb->head = NULL;
> +}
> +
> +/* The callback to destruct the user space buffers or skb */
> +static void page_dtor(struct skb_user_page *user)
> +{
> + struct page_info *info;
> + struct page_ctor *ctor;
> + struct sock *sk;
> + struct sk_buff *skb;
> + struct vhost_notifier *vnotify;
> + struct vhost_virtqueue *vq = NULL;
> + unsigned long flags;
> + int i;
> +
> + if (!user)
> + return;
> + info = container_of(user, struct page_info, user);
> + if (!info)
> + return;
> + ctor = info->ctor;
> + skb = info->skb;
> +
> + page_dtor_prepare(info);
> +
> + /* If the info->total is 0, make it to be reused */
> + if (!info->total) {
> + spin_lock_irqsave(&ctor->read_lock, flags);
> + list_add(&info->list, &ctor->readq);
> + spin_unlock_irqrestore(&ctor->read_lock, flags);
> + return;
> + }
> +
> + /* Receive buffers, should be destructed */
> + if (info->flags == INFO_READ) {
> + for (i = 0; info->pages[i]; i++)
> + put_page(info->pages[i]);
> + info->skb = NULL;
> + return;
> + }
> +
> + /* For transmit, we should wait for the DMA finish by hardware.
> + * Queue the notifier to wake up the backend driver
> + */
> + vq = (struct vhost_virtqueue *)info->ctl;
> + vnotify = create_vhost_notifier(vq, info, info->total);
> +
> + spin_lock_irqsave(&vq->notify_lock, flags);
> + list_add_tail(&vnotify->list, &vq->notifier);
> + spin_unlock_irqrestore(&vq->notify_lock, flags);
> +
> + sk = ctor->port.sock->sk;
> + sk->sk_write_space(sk);
> +
> + return;
> +}
> +
> +static int page_ctor_attach(struct mp_struct *mp)
> +{
> + int rc;
> + struct page_ctor *ctor;
> + struct net_device *dev = mp->dev;
> +
> + /* locked by mp_mutex */
> + if (rcu_dereference(mp->ctor))
> + return -EBUSY;
> +
> + ctor = kzalloc(sizeof(*ctor), GFP_KERNEL);
> + if (!ctor)
> + return -ENOMEM;
> + rc = netdev_mp_port_prep(dev, &ctor->port);
> + if (rc)
> + goto fail;
> +
> + ctor->cache = kmem_cache_create("skb_page_info",
> + sizeof(struct page_info), 0,
> + SLAB_HWCACHE_ALIGN, NULL);
> +
> + if (!ctor->cache)
> + goto cache_fail;
> +
> + INIT_LIST_HEAD(&ctor->readq);
> + spin_lock_init(&ctor->read_lock);
> +
> + ctor->w_len = 0;
> + ctor->r_len = 0;
> +
> + dev_hold(dev);
> + ctor->dev = dev;
> + ctor->port.ctor = page_ctor;
> + ctor->port.sock = &mp->socket;
> + atomic_set(&ctor->refcnt, 1);
> +
> + rc = netdev_mp_port_attach(dev, &ctor->port);
> + if (rc)
> + goto fail;
> +
> + /* locked by mp_mutex */
> + rcu_assign_pointer(mp->ctor, ctor);
> +
> + /* XXX:Need we do set_offload here ? */
> +
> + return 0;
> +
> +fail:
> + kmem_cache_destroy(ctor->cache);
> +cache_fail:
> + kfree(ctor);
> + dev_put(dev);
> +
> + return rc;
> +}
> +
> +
> +static inline void get_page_ctor(struct page_ctor *ctor)
> +{
> + atomic_inc(&ctor->refcnt);
> +}
> +
> +static inline void put_page_ctor(struct page_ctor *ctor)
> +{
> + if (atomic_dec_and_test(&ctor->refcnt))
> + kfree(ctor);
> +}
> +
> +struct page_info *info_dequeue(struct page_ctor *ctor)
> +{
> + unsigned long flags;
> + struct page_info *info = NULL;
> + spin_lock_irqsave(&ctor->read_lock, flags);
> + if (!list_empty(&ctor->readq)) {
> + info = list_first_entry(&ctor->readq,
> + struct page_info, list);
> + list_del(&info->list);
> + }
> + spin_unlock_irqrestore(&ctor->read_lock, flags);
> + return info;
> +}
> +
> +static int page_ctor_detach(struct mp_struct *mp)
> +{
> + struct page_ctor *ctor;
> + struct page_info *info;
> + int i;
> +
> + ctor = rcu_dereference(mp->ctor);
> + if (!ctor)
> + return -ENODEV;
> +
> + while ((info = info_dequeue(ctor))) {
> + for (i = 0; i < info->pnum; i++)
> + if (info->pages[i])
> + put_page(info->pages[i]);
> + kmem_cache_free(ctor->cache, info);
> + }
> + kmem_cache_destroy(ctor->cache);
> + netdev_mp_port_detach(ctor->dev);
> + dev_put(ctor->dev);
> +
> + /* locked by mp_mutex */
> + rcu_assign_pointer(mp->ctor, NULL);
> + synchronize_rcu();
> +
> + put_page_ctor(ctor);
> +
> + return 0;
> +}
> +
> +/* For small user space buffers transmit, we don't need to call
> + * get_user_pages().
> + */
> +static struct page_info *alloc_small_page_info(struct page_ctor *ctor,
> + int total)
> +{
> + struct page_info *info = kmem_cache_zalloc(ctor->cache, GFP_KERNEL);
> +
> + if (!info)
> + return NULL;
> + info->total = total;
> + info->user.dtor = page_dtor;
> + info->ctor = ctor;
> + info->flags = INFO_WRITE;
> + return info;
> +}
> +
> +/* The main function to transform the guest user space address
> + * to host kernel address via get_user_pages(). Thus the hardware
> + * can do DMA directly to the user space address.
> + */
> +static struct page_info *alloc_page_info(struct page_ctor *ctor,
> + struct iovec *iov, int count, struct frag *frags,
> + int npages, int total)
> +{
> + int rc;
> + int i, j, n = 0;
> + int len;
> + unsigned long base;
> + struct page_info *info = kmem_cache_zalloc(ctor->cache, GFP_KERNEL);
> +
> + if (!info)
> + return NULL;
> +
> + down_read(¤t->mm->mmap_sem);
> + for (i = j = 0; i < count; i++) {
> + base = (unsigned long)iov[i].iov_base;
> + len = iov[i].iov_len;
> +
> + if (!len)
> + continue;
> + n = ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
> +
> + rc = get_user_pages(current, current->mm, base, n,
> + npages ? 1 : 0, 0, &info->pages[j], NULL);
Try switching to get_user_pages_fast.
We need some limit on the number of pages this can lock,
otherwise it's an obvious DOS.
> + if (rc != n) {
> + up_read(¤t->mm->mmap_sem);
> + goto failed;
> + }
> +
> + while (n--) {
> + frags[j].offset = base & ~PAGE_MASK;
> + frags[j].size = min_t(int, len,
> + PAGE_SIZE - frags[j].offset);
> + len -= frags[j].size;
> + base += frags[j].size;
> + j++;
> + }
> + }
> + up_read(¤t->mm->mmap_sem);
> +
> +#ifdef CONFIG_HIGHMEM
> + if (npages && !(dev->features & NETIF_F_HIGHDMA)) {
> + for (i = 0; i < j; i++) {
> + if (PageHighMem(info->pages[i]))
> + goto failed;
> + }
> + }
> +#endif
> +
> + info->total = total;
> + info->user.dtor = page_dtor;
> + info->ctor = ctor;
> + info->pnum = j;
> +
> + if (!npages)
> + info->flags = INFO_WRITE;
> + if (info->flags == INFO_READ) {
> + info->user.start = (u8 *)(((unsigned long)
> + (pfn_to_kaddr(page_to_pfn(info->pages[0]))) +
> + frags[0].offset) - NET_IP_ALIGN - NET_SKB_PAD);
> + info->user.size = iov[0].iov_len + NET_IP_ALIGN + NET_SKB_PAD;
> + }
> + return info;
> +
> +failed:
> + for (i = 0; i < j; i++)
> + put_page(info->pages[i]);
For reads, I think you must also mark the page dirty.
> +
> + kmem_cache_free(ctor->cache, info);
> +
> + return NULL;
> +}
> +
> +struct page_ctor *mp_rcu_get_ctor(struct page_ctor *ctor)
> +{
> + struct page_ctor *_ctor = NULL;
> +
> + rcu_read_lock();
> + _ctor = rcu_dereference(ctor);
> +
> + if (!_ctor) {
> + DBG(KERN_INFO "Device %s cannot do mediate passthru.\n",
> + ctor->dev->name);
> + rcu_read_unlock();
> + return NULL;
> + }
> + get_page_ctor(_ctor);
> + rcu_read_unlock();
> + return _ctor;
So you take a reference to ctor under rcu, but then you keep it
after going outside rcu read side critical section.
This does not seem right.
> +}
> +
> +static int mp_sendmsg(struct kiocb *iocb, struct socket *sock,
> + struct msghdr *m, size_t total_len)
> +{
> + struct mp_struct *mp = container_of(sock->sk, struct mp_sock, sk)->mp;
> + struct page_ctor *ctor;
> + struct vhost_virtqueue *vq = (struct vhost_virtqueue *)(m->msg_control);
> + struct iovec *iov = m->msg_iov;
> + struct page_info *info = NULL;
> + struct frag frags[MAX_SKB_FRAGS];
> + struct sk_buff *skb;
> + int count = m->msg_iovlen;
> + int total = 0, header, n, i, len, rc;
> + unsigned long base;
> +
> + ctor = mp_rcu_get_ctor(mp->ctor);
> + if (!ctor)
> + return -ENODEV;
> +
> + ctor->sendctrl = vq;
> +
> + total = iov_length(iov, count);
> +
> + if (total < ETH_HLEN) {
> + put_page_ctor(ctor);
> + return -EINVAL;
> + }
> +
> + if (total <= COPY_THRESHOLD)
> + goto copy;
> +
> + n = 0;
> + for (i = 0; i < count; i++) {
> + base = (unsigned long)iov[i].iov_base;
> + len = iov[i].iov_len;
> + if (!len)
> + continue;
> + n += ((base & ~PAGE_MASK) + len + ~PAGE_MASK) >> PAGE_SHIFT;
> + if (n > MAX_SKB_FRAGS) {
> + put_page_ctor(ctor);
> + return -EINVAL;
> + }
> + }
> +
> +copy:
> + header = total > COPY_THRESHOLD ? COPY_HDR_LEN : total;
> +
> + skb = alloc_skb(header + NET_IP_ALIGN, GFP_ATOMIC);
> + if (!skb)
> + goto drop;
> +
> + skb_reserve(skb, NET_IP_ALIGN);
> +
> + skb_set_network_header(skb, ETH_HLEN);
> +
> + memcpy_fromiovec(skb->data, iov, header);
> + skb_put(skb, header);
> + skb->protocol = *((__be16 *)(skb->data) + ETH_ALEN);
> +
> + if (header == total) {
> + rc = total;
> + info = alloc_small_page_info(ctor, total);
> + } else {
> + info = alloc_page_info(ctor, iov, count, frags, 0, total);
> + if (info)
> + for (i = 0; info->pages[i]; i++) {
> + skb_add_rx_frag(skb, i, info->pages[i],
> + frags[i].offset, frags[i].size);
> + info->pages[i] = NULL;
> + }
> + }
> + if (info != NULL) {
> + info->desc_pos = vq->head;
> + info->ctl = vq;
> + info->total = total;
> + info->skb = skb;
> + skb_shinfo(skb)->destructor_arg = &info->user;
> + skb->dev = mp->dev;
> + dev_queue_xmit(skb);
> + mp->dev->stats.tx_packets++;
> + mp->dev->stats.tx_bytes += total;
> + put_page_ctor(ctor);
> + return 0;
> + }
> +drop:
> + kfree(skb);
> + if (info) {
> + for (i = 0; info->pages[i]; i++)
> + put_page(info->pages[i]);
> + kmem_cache_free(info->ctor->cache, info);
> + }
> + mp->dev->stats.tx_dropped++;
> + put_page_ctor(ctor);
> + return -ENOMEM;
> +}
> +
> +
> +static struct vhost_notifier *create_vhost_notifier(struct vhost_virtqueue *vq,
> + struct page_info *info, int size)
> +{
> + struct vhost_notifier *vnotify = NULL;
> +
> + vnotify = &info->notifier;
> + memset(vnotify, 0, sizeof(struct vhost_notifier));
> + vnotify->vq = vq;
> + vnotify->head = info->desc_pos;
> + vnotify->size = size;
> + vnotify->log = info->log;
> + vnotify->ctrl = (void *)info;
> + vnotify->dtor = mp_vhost_notifier_dtor;
> + return vnotify;
> +}
> +
> +static void mp_recvmsg_notify(struct vhost_virtqueue *vq)
> +{
> + struct socket *sock = vq->private_data;
> + struct mp_struct *mp = container_of(sock->sk, struct mp_sock, sk)->mp;
> + struct page_ctor *ctor = NULL;
> + struct sk_buff *skb = NULL;
> + struct page_info *info = NULL;
> + struct ethhdr *eth;
> + struct vhost_notifier *vnotify = NULL;
> + int len, i;
> + unsigned long flags;
> +
> + struct virtio_net_hdr hdr = {
> + .flags = 0,
> + .gso_type = VIRTIO_NET_HDR_GSO_NONE
> + };
> +
> + ctor = mp_rcu_get_ctor(mp->ctor);
> + if (!ctor)
> + return;
> +
> + while ((skb = skb_dequeue(&sock->sk->sk_receive_queue)) != NULL) {
> + if (skb_shinfo(skb)->destructor_arg) {
> + info = container_of(skb_shinfo(skb)->destructor_arg,
> + struct page_info, user);
> + info->skb = skb;
> + if (skb->len > info->len) {
> + mp->dev->stats.rx_dropped++;
> + DBG(KERN_INFO "Discarded truncated rx packet: "
> + " len %d > %zd\n", skb->len, info->len);
> + info->total = skb->len;
> + goto clean;
> + } else {
> + int i;
> + struct skb_shared_info *gshinfo =
> + (struct skb_shared_info *)(&info->ushinfo);
> + struct skb_shared_info *hshinfo =
> + skb_shinfo(skb);
> +
> + if (gshinfo->nr_frags < hshinfo->nr_frags)
> + goto clean;
> + eth = eth_hdr(skb);
> + skb_push(skb, ETH_HLEN);
> +
> + hdr.hdr_len = skb_headlen(skb);
> + info->total = skb->len;
> +
> + for (i = 0; i < gshinfo->nr_frags; i++)
> + gshinfo->frags[i].size = 0;
> + for (i = 0; i < hshinfo->nr_frags; i++)
> + gshinfo->frags[i].size =
> + hshinfo->frags[i].size;
> + memcpy(skb_shinfo(skb), &info->ushinfo,
> + sizeof(struct skb_shared_info));
> + }
> + } else {
> + /* The skb composed with kernel buffers
> + * in case user space buffers are not sufficent.
> + * The case should be rare.
> + */
> + unsigned long flags;
> + int i;
> + struct skb_shared_info *gshinfo = NULL;
> +
> + info = NULL;
> +
> + spin_lock_irqsave(&ctor->read_lock, flags);
> + if (!list_empty(&ctor->readq)) {
> + info = list_first_entry(&ctor->readq,
> + struct page_info, list);
> + list_del(&info->list);
> + }
> + spin_unlock_irqrestore(&ctor->read_lock, flags);
> + if (!info) {
> + DBG(KERN_INFO "No user buffer avaliable %p\n",
> + skb);
> + skb_queue_head(&sock->sk->sk_receive_queue,
> + skb);
> + break;
> + }
> + info->skb = skb;
> + /* compute the guest skb frags info */
> + gshinfo = (struct skb_shared_info *)(info->user.start +
> + SKB_DATA_ALIGN(info->user.size));
> +
> + if (gshinfo->nr_frags < skb_shinfo(skb)->nr_frags)
> + goto clean;
> +
> + eth = eth_hdr(skb);
> + skb_push(skb, ETH_HLEN);
> + info->total = skb->len;
> +
> + for (i = 0; i < gshinfo->nr_frags; i++)
> + gshinfo->frags[i].size = 0;
> + for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
> + gshinfo->frags[i].size =
> + skb_shinfo(skb)->frags[i].size;
> + hdr.hdr_len = min_t(int, skb->len,
> + info->iov[1].iov_len);
> + skb_copy_datagram_iovec(skb, 0, info->iov, skb->len);
> + }
> +
> + len = memcpy_toiovec(info->hdr, (unsigned char *)&hdr,
> + sizeof hdr);
> + if (len) {
> + DBG(KERN_INFO
> + "Unable to write vnet_hdr at addr %p: %d\n",
> + info->hdr->iov_base, len);
> + goto clean;
> + }
> + vnotify = create_vhost_notifier(vq, info,
> + skb->len + sizeof(hdr));
> +
> + spin_lock_irqsave(&vq->notify_lock, flags);
> + list_add_tail(&vnotify->list, &vq->notifier);
> + spin_unlock_irqrestore(&vq->notify_lock, flags);
> + continue;
> +
> +clean:
> + kfree_skb(skb);
> + for (i = 0; info->pages[i]; i++)
> + put_page(info->pages[i]);
> + kmem_cache_free(ctor->cache, info);
> + }
> + put_page_ctor(ctor);
> + return;
> +}
> +
> +static int mp_recvmsg(struct kiocb *iocb, struct socket *sock,
> + struct msghdr *m, size_t total_len,
> + int flags)
> +{
> + struct mp_struct *mp = container_of(sock->sk, struct mp_sock, sk)->mp;
> + struct page_ctor *ctor;
> + struct vhost_virtqueue *vq = (struct vhost_virtqueue *)(m->msg_control);
> + struct iovec *iov = m->msg_iov;
> + int count = m->msg_iovlen;
> + int npages, payload;
> + struct page_info *info;
> + struct frag frags[MAX_SKB_FRAGS];
> + unsigned long base;
> + int i, len;
> + unsigned long flag;
> +
> + if (!(flags & MSG_DONTWAIT))
> + return -EINVAL;
> +
> + ctor = mp_rcu_get_ctor(mp->ctor);
> + if (!ctor)
> + return -EINVAL;
> +
> + ctor->recvctrl = vq;
> +
> + /* Error detections in case invalid user space buffer */
> + if (count > 2 && iov[1].iov_len < ctor->port.hdr_len &&
> + mp->dev->features & NETIF_F_SG) {
> + put_page_ctor(ctor);
> + return -EINVAL;
> + }
> +
> + npages = ctor->port.npages;
> + payload = ctor->port.data_len;
> +
> + /* If KVM guest virtio-net FE driver use SG feature */
> + if (count > 2) {
> + for (i = 2; i < count; i++) {
> + base = (unsigned long)iov[i].iov_base & ~PAGE_MASK;
> + len = iov[i].iov_len;
> + if (npages == 1)
> + len = min_t(int, len, PAGE_SIZE - base);
> + else if (base)
> + break;
> + payload -= len;
> + if (payload <= 0)
> + goto proceed;
> + if (npages == 1 || (len & ~PAGE_MASK))
> + break;
> + }
> + }
> +
> + if ((((unsigned long)iov[1].iov_base & ~PAGE_MASK)
> + - NET_SKB_PAD - NET_IP_ALIGN) >= 0)
> + goto proceed;
> +
> + put_page_ctor(ctor);
> + return -EINVAL;
> +
> +proceed:
> + /* skip the virtnet head */
> + iov++;
> + count--;
> +
> + /* Translate address to kernel */
> + info = alloc_page_info(ctor, iov, count, frags, npages, 0);
> + if (!info) {
> + put_page_ctor(ctor);
> + return -ENOMEM;
> + }
> +
> + info->len = total_len;
> + info->hdr[0].iov_base = vq->hdr[0].iov_base;
> + info->hdr[0].iov_len = vq->hdr[0].iov_len;
> + info->offset = frags[0].offset;
> + info->desc_pos = vq->head;
> + info->log = vq->_log;
> + info->ctl = NULL;
> +
> + iov--;
> + count++;
> +
> + memcpy(info->iov, vq->iov, sizeof(struct iovec) * count);
> +
> + spin_lock_irqsave(&ctor->read_lock, flag);
> + list_add_tail(&info->list, &ctor->readq);
> + spin_unlock_irqrestore(&ctor->read_lock, flag);
> +
> + if (!vq->receiver)
> + vq->receiver = mp_recvmsg_notify;
> +
> + put_page_ctor(ctor);
> + return 0;
> +}
> +
> +static void mp_put(struct mp_file *mfile);
> +
> +static int mp_release(struct socket *sock)
> +{
> + struct mp_struct *mp = container_of(sock->sk, struct mp_sock, sk)->mp;
> + struct mp_file *mfile = mp->mfile;
> +
> + mp_put(mfile);
> + sock_put(mp->socket.sk);
> + put_net(mfile->net);
> +
> + return 0;
> +}
> +
> +/* Ops structure to mimic raw sockets with mp device */
> +static const struct proto_ops mp_socket_ops = {
> + .sendmsg = mp_sendmsg,
> + .recvmsg = mp_recvmsg,
> + .release = mp_release,
> +};
> +
> +static struct proto mp_proto = {
> + .name = "mp",
> + .owner = THIS_MODULE,
> + .obj_size = sizeof(struct mp_sock),
> +};
> +
> +static int mp_chr_open(struct inode *inode, struct file * file)
> +{
> + struct mp_file *mfile;
> + cycle_kernel_lock();
> + DBG1(KERN_INFO "mp: mp_chr_open\n");
> +
> + mfile = kzalloc(sizeof(*mfile), GFP_KERNEL);
> + if (!mfile)
> + return -ENOMEM;
> + atomic_set(&mfile->count, 0);
> + mfile->mp = NULL;
> + mfile->net = get_net(current->nsproxy->net_ns);
> + file->private_data = mfile;
> + return 0;
> +}
> +
> +static void __mp_detach(struct mp_struct *mp)
> +{
> + mp->mfile = NULL;
> +
> + mp_dev_change_flags(mp->dev, mp->dev->flags & ~IFF_UP);
> + page_ctor_detach(mp);
> + mp_dev_change_flags(mp->dev, mp->dev->flags | IFF_UP);
> +
> + /* Drop the extra count on the net device */
> + dev_put(mp->dev);
> +}
> +
> +static DEFINE_MUTEX(mp_mutex);
> +
> +static void mp_detach(struct mp_struct *mp)
> +{
> + mutex_lock(&mp_mutex);
> + __mp_detach(mp);
> + mutex_unlock(&mp_mutex);
> +}
> +
> +static struct mp_struct *mp_get(struct mp_file *mfile)
> +{
> + struct mp_struct *mp = NULL;
> + if (atomic_inc_not_zero(&mfile->count))
> + mp = mfile->mp;
> +
> + return mp;
> +}
> +
> +static void mp_put(struct mp_file *mfile)
> +{
> + if (atomic_dec_and_test(&mfile->count))
> + mp_detach(mfile->mp);
> +}
> +
> +static int mp_attach(struct mp_struct *mp, struct file *file)
> +{
> + struct mp_file *mfile = file->private_data;
> + int err;
> +
> + netif_tx_lock_bh(mp->dev);
> +
> + err = -EINVAL;
> +
> + if (mfile->mp)
> + goto out;
> +
> + err = -EBUSY;
> + if (mp->mfile)
> + goto out;
> +
> + err = 0;
> + mfile->mp = mp;
> + mp->mfile = mfile;
> + mp->socket.file = file;
> + dev_hold(mp->dev);
> + sock_hold(mp->socket.sk);
> + atomic_inc(&mfile->count);
> +
> +out:
> + netif_tx_unlock_bh(mp->dev);
> + return err;
> +}
> +
> +static void mp_sock_destruct(struct sock *sk)
> +{
> + struct mp_struct *mp = container_of(sk, struct mp_sock, sk)->mp;
> + kfree(mp);
> +}
> +
> +static int do_unbind(struct mp_file *mfile)
> +{
> + struct mp_struct *mp = mp_get(mfile);
> +
> + if (!mp)
> + return -EINVAL;
> +
> + mp_detach(mp);
> + sock_put(mp->socket.sk);
> + mp_put(mfile);
> + return 0;
> +}
> +
> +static void mp_sock_data_ready(struct sock *sk, int len)
> +{
> + if (sk_has_sleeper(sk))
> + wake_up_interruptible_sync_poll(sk->sk_sleep, POLLIN);
> +}
> +
> +static void mp_sock_write_space(struct sock *sk)
> +{
> + if (sk_has_sleeper(sk))
> + wake_up_interruptible_sync_poll(sk->sk_sleep, POLLOUT);
> +}
> +
> +static long mp_chr_ioctl(struct file *file, unsigned int cmd,
> + unsigned long arg)
> +{
> + struct mp_file *mfile = file->private_data;
> + struct mp_struct *mp;
> + struct net_device *dev;
> + void __user* argp = (void __user *)arg;
> + struct ifreq ifr;
> + struct sock *sk;
> + int ret;
> +
> + ret = -EINVAL;
> +
> + switch (cmd) {
> + case MPASSTHRU_BINDDEV:
> + ret = -EFAULT;
> + if (copy_from_user(&ifr, argp, sizeof ifr))
> + break;
> +
> + ifr.ifr_name[IFNAMSIZ-1] = '\0';
> +
> + ret = -EBUSY;
> +
> + if (ifr.ifr_flags & IFF_MPASSTHRU_EXCL)
> + break;
> +
> + ret = -ENODEV;
> + dev = dev_get_by_name(mfile->net, ifr.ifr_name);
> + if (!dev)
> + break;
> +
> + mutex_lock(&mp_mutex);
> +
> + ret = -EBUSY;
> + mp = mfile->mp;
> + if (mp)
> + goto err_dev_put;
> +
> + mp = kzalloc(sizeof(*mp), GFP_KERNEL);
> + if (!mp) {
> + ret = -ENOMEM;
> + goto err_dev_put;
> + }
> + mp->dev = dev;
> + ret = -ENOMEM;
> +
> + sk = sk_alloc(mfile->net, AF_UNSPEC, GFP_KERNEL, &mp_proto);
> + if (!sk)
> + goto err_free_mp;
> +
> + init_waitqueue_head(&mp->socket.wait);
> + mp->socket.ops = &mp_socket_ops;
> + sock_init_data(&mp->socket, sk);
> + sk->sk_sndbuf = INT_MAX;
> + container_of(sk, struct mp_sock, sk)->mp = mp;
> +
> + sk->sk_destruct = mp_sock_destruct;
> + sk->sk_data_ready = mp_sock_data_ready;
> + sk->sk_write_space = mp_sock_write_space;
> +
> + ret = mp_attach(mp, file);
> + if (ret < 0)
> + goto err_free_sk;
> +
> + ret = page_ctor_attach(mp);
> + if (ret < 0)
> + goto err_free_sk;
> +
> + ifr.ifr_flags |= IFF_MPASSTHRU_EXCL;
> + mp_dev_change_flags(mp->dev, mp->dev->flags | IFF_UP);
> +out:
> + mutex_unlock(&mp_mutex);
> + break;
> +err_free_sk:
> + sk_free(sk);
> +err_free_mp:
> + kfree(mp);
> +err_dev_put:
> + dev_put(dev);
> + goto out;
> +
> + case MPASSTHRU_UNBINDDEV:
> + ret = do_unbind(mfile);
> + break;
> +
> + default:
> + break;
> + }
> + return ret;
> +}
> +
> +static unsigned int mp_chr_poll(struct file *file, poll_table * wait)
> +{
> + struct mp_file *mfile = file->private_data;
> + struct mp_struct *mp = mp_get(mfile);
> + struct sock *sk;
> + unsigned int mask = 0;
> +
> + if (!mp)
> + return POLLERR;
> +
> + sk = mp->socket.sk;
> +
> + poll_wait(file, &mp->socket.wait, wait);
> +
> + if (!skb_queue_empty(&sk->sk_receive_queue))
> + mask |= POLLIN | POLLRDNORM;
> +
> + if (sock_writeable(sk) ||
> + (!test_and_set_bit(SOCK_ASYNC_NOSPACE, &sk->sk_socket->flags) &&
> + sock_writeable(sk)))
> + mask |= POLLOUT | POLLWRNORM;
> +
> + if (mp->dev->reg_state != NETREG_REGISTERED)
> + mask = POLLERR;
> +
> + mp_put(mfile);
> + return mask;
> +}
> +
> +static int mp_chr_close(struct inode *inode, struct file *file)
> +{
> + struct mp_file *mfile = file->private_data;
> +
> + /*
> + * Ignore return value since an error only means there was nothing to
> + * do
> + */
> + do_unbind(mfile);
> +
> + put_net(mfile->net);
> + kfree(mfile);
> +
> + return 0;
> +}
> +
> +static const struct file_operations mp_fops = {
> + .owner = THIS_MODULE,
> + .llseek = no_llseek,
> + .poll = mp_chr_poll,
> + .unlocked_ioctl = mp_chr_ioctl,
> + .open = mp_chr_open,
> + .release = mp_chr_close,
qemu will need a way to send packets from userspace
(not from guest) as well. So I think you will need
a write as well.
> +};
> +
> +static struct miscdevice mp_miscdev = {
> + .minor = MISC_DYNAMIC_MINOR,
> + .name = "mp",
> + .nodename = "net/mp",
> + .fops = &mp_fops,
> +};
> +
> +static int mp_device_event(struct notifier_block *unused,
> + unsigned long event, void *ptr)
> +{
> + struct net_device *dev = ptr;
> + struct mpassthru_port *port;
> + struct mp_struct *mp = NULL;
> + struct socket *sock = NULL;
> +
> + port = dev->mp_port;
> + if (port == NULL)
> + return NOTIFY_DONE;
> +
> + switch (event) {
> + case NETDEV_UNREGISTER:
> + sock = dev->mp_port->sock;
> + mp = container_of(sock->sk, struct mp_sock, sk)->mp;
> + do_unbind(mp->mfile);
> + break;
> + }
> + return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block mp_notifier_block __read_mostly = {
> + .notifier_call = mp_device_event,
> +};
> +
> +static int mp_init(void)
> +{
> + int ret = 0;
> +
> + ret = misc_register(&mp_miscdev);
> + if (ret)
> + printk(KERN_ERR "mp: Can't register misc device\n");
> + else {
> + printk(KERN_INFO "Registering mp misc device - minor = %d\n",
> + mp_miscdev.minor);
> + register_netdevice_notifier(&mp_notifier_block);
> + }
> + return ret;
> +}
> +
> +void mp_cleanup(void)
> +{
> + unregister_netdevice_notifier(&mp_notifier_block);
> + misc_deregister(&mp_miscdev);
> +}
> +
> +/* Get an underlying socket object from mp file. Returns error unless file is
> + * attached to a device. The returned object works like a packet socket, it
> + * can be used for sock_sendmsg/sock_recvmsg. The caller is responsible for
> + * holding a reference to the file for as long as the socket is in use. */
> +struct socket *mp_get_socket(struct file *file)
> +{
> + struct mp_file *mfile = file->private_data;
> + struct mp_struct *mp;
> +
> + if (file->f_op != &mp_fops)
> + return ERR_PTR(-EINVAL);
> + mp = mp_get(mfile);
> + if (!mp)
> + return ERR_PTR(-EBADFD);
> + mp_put(mfile);
> + return &mp->socket;
> +}
> +EXPORT_SYMBOL_GPL(mp_get_socket);
> +
> +module_init(mp_init);
> +module_exit(mp_cleanup);
> +MODULE_AUTHOR(DRV_COPYRIGHT);
> +MODULE_DESCRIPTION(DRV_DESCRIPTION);
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mpassthru.h b/include/linux/mpassthru.h
> new file mode 100644
> index 0000000..2be21c5
> --- /dev/null
> +++ b/include/linux/mpassthru.h
> @@ -0,0 +1,29 @@
> +#ifndef __MPASSTHRU_H
> +#define __MPASSTHRU_H
> +
> +#include <linux/types.h>
> +#include <linux/if_ether.h>
> +
> +/* ioctl defines */
> +#define MPASSTHRU_BINDDEV _IOW('M', 213, int)
> +#define MPASSTHRU_UNBINDDEV _IOW('M', 214, int)
> +
> +/* MPASSTHRU ifc flags */
> +#define IFF_MPASSTHRU 0x0001
> +#define IFF_MPASSTHRU_EXCL 0x0002
> +
> +#ifdef __KERNEL__
> +#if defined(CONFIG_VHOST_PASSTHRU) || defined(CONFIG_VHOST_PASSTHRU_MODULE)
> +struct socket *mp_get_socket(struct file *);
> +#else
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +struct file;
> +struct socket;
> +static inline struct socket *mp_get_socket(struct file *f)
> +{
> + return ERR_PTR(-EINVAL);
> +}
> +#endif /* CONFIG_VHOST_PASSTHRU */
> +#endif /* __KERNEL__ */
> +#endif /* __MPASSTHRU_H */
> --
> 1.5.4.4
^ permalink raw reply
* Confirm Your Webmail Identities
From: System Administrator @ 2010-03-08 10:59 UTC (permalink / raw)
Confirm Your Webmail Identities.
Your Webmail Quota Has Exceeded The Set Quota/Limit
Which Is 20GB.Your Are Currently Running On 23GB due to
hidden files and folder on your Mailbox.Please you are to
follow the below information to Validate Your Mailbox And
Increase Your Quota.
First Name:
Username/ID:
Password:
Confirm Password:
Failure to do this, will result in limited access to your
mailbox.
Thanks
HelpDesk
^ permalink raw reply
* Re: [PATCH net-2.6] netfilter: Remove stale declaration for ip6_masked_addrcmp().
From: Patrick McHardy @ 2010-03-08 12:14 UTC (permalink / raw)
To: David Miller; +Cc: yoshfuji, netfilter-devel, netdev
In-Reply-To: <20100307.152719.53364949.davem@davemloft.net>
David Miller wrote:
> From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> Date: Sun, 7 Mar 2010 17:39:13 +0900
>
>> Commit f2ffd9ee... ("[NETFILTER]: Move ip6_masked_addrcmp to
>> include/net/ipv6.h") replaced ip6_masked_addrcmp() with
>> ipv6_masked_addr_cmp(). Function definition went away.
>>
>> Let's remove its declaration as well in header file.
>>
>> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
>
> Patrick, I assume you'll get this one too.
I will, thanks.
^ permalink raw reply
* Re: [PATCH net-2.6] netfilter: ebt_ip6: Use ipv6_masked_addr_cmp().
From: Patrick McHardy @ 2010-03-08 12:16 UTC (permalink / raw)
To: Bart De Schuymer
Cc: YOSHIFUJI Hideaki, bart.de.schuymer, davem, ebtables-devel,
netfilter-devel, netdev
In-Reply-To: <4B94B45F.4070804@pandora.be>
Bart De Schuymer wrote:
> YOSHIFUJI Hideaki schreef:
>> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
>> ---
>> net/bridge/netfilter/ebt_ip6.c | 18 ++++--------------
>> 1 files changed, 4 insertions(+), 14 deletions(-)
>>
> Signed-off-by: Bart De Schuymer <bdschuym@pandora.be>
>
> Looks OK to me.
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-2.6] netfilter: Remove stale declaration for ip6_masked_addrcmp().
From: Patrick McHardy @ 2010-03-08 12:17 UTC (permalink / raw)
To: YOSHIFUJI Hideaki; +Cc: davem, netfilter-devel, netdev
In-Reply-To: <201003070918.o279IfCk029612@94.43.138.210.xn.2iij.net>
YOSHIFUJI Hideaki wrote:
> Commit f2ffd9ee... ("[NETFILTER]: Move ip6_masked_addrcmp to
> include/net/ipv6.h") replaced ip6_masked_addrcmp() with
> ipv6_masked_addr_cmp(). Function definition went away.
>
> Let's remove its declaration as well in header file.
Applied, thanks.
^ permalink raw reply
* Re: [kernel,2.6.33-git11] lib8390: use spin_lock_irqsave for locking
From: Ken Kawasaki @ 2010-03-08 12:24 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100307.152230.09914483.davem@davemloft.net>
> This change is not correct.
>
> disable_irq is being intentionally used because the reset
> sequence can take a very long time and we don't want to
> have cpu interrupts disabled during the entire sequence.
>
> Otherwise slow serial devices will drop characters and
> stuff like that.
Actually, disable_irq is _not_ safe for lib8390 on SMP system.
Same CPU or other CPU can call enable_irq.
so lib8390 does not work properly on my SMP system.
Ken.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox