All of lore.kernel.org
 help / color / mirror / Atom feed
From: Carsten Andrich <carsten.andrich-hs6bpBdVsEZfm0AUMx9V0g@public.gmane.org>
To: Daniel Borkmann <dborkman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	Stefan Puiu <stefan.puiu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	lnx-man <linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Willem de Bruijn
	<willemb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	Neil Horman <nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Subject: Re: status in PACKET_RX_RING is actually a bit mask
Date: Thu, 24 Apr 2014 11:17:02 +0200	[thread overview]
Message-ID: <a8bac93e5e046516a01b2b1952c8cae5@localhost> (raw)
In-Reply-To: <5358C7E0.9040704-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

I'd ack the patch, too.
However if we start clarifying the bitmask situation, we should do it
properly. packet.7 mentions TP_STATUS_xyz several times and never
actually differentiates between setting bits and setting the whole
tp_status variable. Actually both occurs inside the kernel:
http://lingrok.org/xref/linux-net-next/net/packet/af_packet.c#798
http://lingrok.org/xref/linux-net-next/net/packet/af_packet.c#2300

I could prepare another patch for review, that clarifies the tp_status
usage for all references to TP_STATUS_xyz values.
Unless Stefan would like to do this himself, since it's actually his
"discovery" ;)

Cheers,
Carsten

Daniel Borkmann schrieb:
> On 04/24/2014 10:04 AM, Michael Kerrisk (man-pages) wrote:
>>> Now to your question. It can easily be seen from the if_packet.h header
>>> file http://lingrok.org/xref/linux-net-next/include/uapi/linux/if_packet.h#93
>>> that TP_STATUS_* are individual bits that are set in tp_status field.
>>
>> So, I take it that that's an Ack-for the patch by Stefan?
> 
> Feel free to add my Ack. One nit below:
> 
> diff --git a/man7/packet.7 b/man7/packet.7
> index 1d3f222..6bac465 100644
> --- a/man7/packet.7
> +++ b/man7/packet.7
> @@ -353,9 +353,9 @@ The packet socket owns all slots with status
>  .BR TP_STATUS_KERNEL .
>  After filling a slot, it changes the status of the slot to transfer
>  ownership to the application.
> -During normal operation, the new status is
> -.BR TP_STATUS_USER ,
> -to signal that a correctly received packet has been stored.
> +During normal operation, the new status has the
> +.BR TP_STATUS_USER
> +bit set to signal that a correctly received packet has been stored.
> 
> ^--- I would drop the 'correctly' here as it rather raises questions
>      what 'incorrectly' would mean in this context.
> 
>  When the application has finished processing a packet, it transfers
>  ownership of the slot back to the socket by setting the status to
> 
> 
>>
>> Cheers,
>>
>> Michael
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2014-04-24  9:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CACKs7VD6a0hzMd_Go6yUAxJ=tbYdJ9Qbq=FXQezVvaUUE9egUw@mail.gmail.com>
     [not found] ` <CACKs7VD6a0hzMd_Go6yUAxJ=tbYdJ9Qbq=FXQezVvaUUE9egUw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-02-25 21:24   ` Fwd: status in PACKET_RX_RING is actually a bit mask Stefan Puiu
     [not found]     ` <CACKs7VAUE89+NyuywOwpZ2GzeVQLtV_A-+4PicErmCzLNNNd6Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-04-24  7:42       ` Michael Kerrisk (man-pages)
     [not found]         ` <CAKgNAkhJO2ZMpMf6BiaW7TRUd6jvLFU7yPN7xKH9fDoVPPr=tA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-04-24  8:02           ` Daniel Borkmann
     [not found]             ` <5358C4FC.80204-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2014-04-24  8:04               ` Michael Kerrisk (man-pages)
     [not found]                 ` <CAKgNAkjwHkba6NktfNK9nRUUAwcYeFK5aGBgov1tcuipy4mPdw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-04-24  8:14                   ` Daniel Borkmann
     [not found]                     ` <5358C7E0.9040704-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2014-04-24  9:17                       ` Carsten Andrich [this message]
2014-04-24 10:16                         ` Michael Kerrisk (man-pages)
2014-04-25  6:39                         ` Stefan Puiu
2014-04-24  9:52                       ` Michael Kerrisk (man-pages)
2014-04-24  9:48               ` Michael Kerrisk (man-pages)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a8bac93e5e046516a01b2b1952c8cae5@localhost \
    --to=carsten.andrich-hs6bpbdvsezfm0aumx9v0g@public.gmane.org \
    --cc=dborkman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org \
    --cc=stefan.puiu-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=willemb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.