* [PATCH -next] packet: remove unused variable 'status' in __packet_lookup_frame_in_block
@ 2019-06-10 11:58 Mao Wenan
2019-06-10 13:05 ` Willem de Bruijn
0 siblings, 1 reply; 6+ messages in thread
From: Mao Wenan @ 2019-06-10 11:58 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, kernel-janitors, Mao Wenan
The variable 'status' in __packet_lookup_frame_in_block() is never used since
introduction in commit f6fb8f100b80 ("af-packet: TPACKET_V3 flexible buffer
implementation."), we can remove it.
And when __packet_lookup_frame_in_block() calls prb_retire_current_block(),
it can pass macro TP_STATUS_KERNEL instead of 0.
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
net/packet/af_packet.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index a29d66d..fb1a79c 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1003,7 +1003,6 @@ static void prb_fill_curr_block(char *curr,
/* Assumes caller has the sk->rx_queue.lock */
static void *__packet_lookup_frame_in_block(struct packet_sock *po,
struct sk_buff *skb,
- int status,
unsigned int len
)
{
@@ -1046,7 +1045,7 @@ static void *__packet_lookup_frame_in_block(struct packet_sock *po,
}
/* Ok, close the current block */
- prb_retire_current_block(pkc, po, 0);
+ prb_retire_current_block(pkc, po, TP_STATUS_KERNEL);
/* Now, try to dispatch the next block */
curr = (char *)prb_dispatch_next_block(pkc, po);
@@ -1075,7 +1074,7 @@ static void *packet_current_rx_frame(struct packet_sock *po,
po->rx_ring.head, status);
return curr;
case TPACKET_V3:
- return __packet_lookup_frame_in_block(po, skb, status, len);
+ return __packet_lookup_frame_in_block(po, skb, len);
default:
WARN(1, "TPACKET version not supported\n");
BUG();
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH -next] packet: remove unused variable 'status' in __packet_lookup_frame_in_block
2019-06-10 11:58 [PATCH -next] packet: remove unused variable 'status' in __packet_lookup_frame_in_block Mao Wenan
@ 2019-06-10 13:05 ` Willem de Bruijn
2019-06-10 14:02 ` maowenan
0 siblings, 1 reply; 6+ messages in thread
From: Willem de Bruijn @ 2019-06-10 13:05 UTC (permalink / raw)
To: Mao Wenan; +Cc: David Miller, Network Development, LKML, kernel-janitors
On Mon, Jun 10, 2019 at 8:17 AM Mao Wenan <maowenan@huawei.com> wrote:
>
> The variable 'status' in __packet_lookup_frame_in_block() is never used since
> introduction in commit f6fb8f100b80 ("af-packet: TPACKET_V3 flexible buffer
> implementation."), we can remove it.
> And when __packet_lookup_frame_in_block() calls prb_retire_current_block(),
> it can pass macro TP_STATUS_KERNEL instead of 0.
>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
> net/packet/af_packet.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index a29d66d..fb1a79c 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1003,7 +1003,6 @@ static void prb_fill_curr_block(char *curr,
> /* Assumes caller has the sk->rx_queue.lock */
> static void *__packet_lookup_frame_in_block(struct packet_sock *po,
> struct sk_buff *skb,
> - int status,
> unsigned int len
> )
> {
> @@ -1046,7 +1045,7 @@ static void *__packet_lookup_frame_in_block(struct packet_sock *po,
> }
>
> /* Ok, close the current block */
> - prb_retire_current_block(pkc, po, 0);
> + prb_retire_current_block(pkc, po, TP_STATUS_KERNEL);
I don't think that 0 is intended to mean TP_STATUS_KERNEL here.
prb_retire_current_block calls prb_close_block which sets status to
TP_STATUS_USER | stat
where stat is 0 or TP_STATUS_BLK_TMO.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH -next] packet: remove unused variable 'status' in __packet_lookup_frame_in_block
2019-06-10 13:05 ` Willem de Bruijn
@ 2019-06-10 14:02 ` maowenan
2019-06-10 14:34 ` Willem de Bruijn
0 siblings, 1 reply; 6+ messages in thread
From: maowenan @ 2019-06-10 14:02 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: David Miller, Network Development, LKML, kernel-janitors
On 2019/6/10 21:05, Willem de Bruijn wrote:
> On Mon, Jun 10, 2019 at 8:17 AM Mao Wenan <maowenan@huawei.com> wrote:
>>
>> The variable 'status' in __packet_lookup_frame_in_block() is never used since
>> introduction in commit f6fb8f100b80 ("af-packet: TPACKET_V3 flexible buffer
>> implementation."), we can remove it.
>> And when __packet_lookup_frame_in_block() calls prb_retire_current_block(),
>> it can pass macro TP_STATUS_KERNEL instead of 0.
>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>> ---
>> net/packet/af_packet.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
>> index a29d66d..fb1a79c 100644
>> --- a/net/packet/af_packet.c
>> +++ b/net/packet/af_packet.c
>> @@ -1003,7 +1003,6 @@ static void prb_fill_curr_block(char *curr,
>> /* Assumes caller has the sk->rx_queue.lock */
>> static void *__packet_lookup_frame_in_block(struct packet_sock *po,
>> struct sk_buff *skb,
>> - int status,
>> unsigned int len
>> )
>> {
>> @@ -1046,7 +1045,7 @@ static void *__packet_lookup_frame_in_block(struct packet_sock *po,
>> }
>>
>> /* Ok, close the current block */
>> - prb_retire_current_block(pkc, po, 0);
>> + prb_retire_current_block(pkc, po, TP_STATUS_KERNEL);
>
> I don't think that 0 is intended to mean TP_STATUS_KERNEL here.
>
> prb_retire_current_block calls prb_close_block which sets status to
>
> TP_STATUS_USER | stat
>
> where stat is 0 or TP_STATUS_BLK_TMO.
#define TP_STATUS_KERNEL 0
#define TP_STATUS_BLK_TMO (1 << 5)
Actually, packet_current_rx_frame calls __packet_lookup_frame_in_block with status=TP_STATUS_KERNEL
in original code.
__packet_lookup_frame_in_block in this function, first is to check whether the currently active block
has enough space for the packet, which means status of block should be TP_STATUS_KERNEL, then it calls
prb_retire_current_block to retire this block.
Since there needs some discussion about means of status, I can send v2 only removing the parameter status of
__packet_lookup_frame_in_block?
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH -next] packet: remove unused variable 'status' in __packet_lookup_frame_in_block
2019-06-10 14:02 ` maowenan
@ 2019-06-10 14:34 ` Willem de Bruijn
2019-06-11 1:32 ` [PATCH -next v2] " Mao Wenan
0 siblings, 1 reply; 6+ messages in thread
From: Willem de Bruijn @ 2019-06-10 14:34 UTC (permalink / raw)
To: maowenan; +Cc: David Miller, Network Development, LKML, kernel-janitors
On Mon, Jun 10, 2019 at 10:03 AM maowenan <maowenan@huawei.com> wrote:
>
>
>
> On 2019/6/10 21:05, Willem de Bruijn wrote:
> > On Mon, Jun 10, 2019 at 8:17 AM Mao Wenan <maowenan@huawei.com> wrote:
> >>
> >> The variable 'status' in __packet_lookup_frame_in_block() is never used since
> >> introduction in commit f6fb8f100b80 ("af-packet: TPACKET_V3 flexible buffer
> >> implementation."), we can remove it.
> >> And when __packet_lookup_frame_in_block() calls prb_retire_current_block(),
> >> it can pass macro TP_STATUS_KERNEL instead of 0.
> >>
> >> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> >> ---
> >> /* Ok, close the current block */
> >> - prb_retire_current_block(pkc, po, 0);
> >> + prb_retire_current_block(pkc, po, TP_STATUS_KERNEL);
> >
> > I don't think that 0 is intended to mean TP_STATUS_KERNEL here.
> >
> > prb_retire_current_block calls prb_close_block which sets status to
> >
> > TP_STATUS_USER | stat
> >
> > where stat is 0 or TP_STATUS_BLK_TMO.
>
>
> #define TP_STATUS_KERNEL 0
> #define TP_STATUS_BLK_TMO (1 << 5)
>
> Actually, packet_current_rx_frame calls __packet_lookup_frame_in_block with status=TP_STATUS_KERNEL
> in original code.
>
> __packet_lookup_frame_in_block in this function, first is to check whether the currently active block
> has enough space for the packet, which means status of block should be TP_STATUS_KERNEL, then it calls
> prb_retire_current_block to retire this block.
I know. I mean that the status here is what is passed to userspace on
block retire.
It is not intended to be TP_STATUS_USER | TP_STATUS_KERNEL. That makes no sense.
> Since there needs some discussion about means of status, I can send v2 only removing the parameter status of
> __packet_lookup_frame_in_block?
Sounds good.
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH -next v2] packet: remove unused variable 'status' in __packet_lookup_frame_in_block
2019-06-10 14:34 ` Willem de Bruijn
@ 2019-06-11 1:32 ` Mao Wenan
2019-06-11 19:13 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Mao Wenan @ 2019-06-11 1:32 UTC (permalink / raw)
To: davem
Cc: willemdebruijn.kernel, netdev, linux-kernel, kernel-janitors, Mao Wenan
The variable 'status' in __packet_lookup_frame_in_block() is never used since
introduction in commit f6fb8f100b80 ("af-packet: TPACKET_V3 flexible buffer
implementation."), we can remove it.
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
v2: don't change parameter from 0 to TP_STATUS_KERNEL when calls
prb_retire_current_block().
---
net/packet/af_packet.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index a29d66da7394..7fa847dcea30 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1003,7 +1003,6 @@ static void prb_fill_curr_block(char *curr,
/* Assumes caller has the sk->rx_queue.lock */
static void *__packet_lookup_frame_in_block(struct packet_sock *po,
struct sk_buff *skb,
- int status,
unsigned int len
)
{
@@ -1075,7 +1074,7 @@ static void *packet_current_rx_frame(struct packet_sock *po,
po->rx_ring.head, status);
return curr;
case TPACKET_V3:
- return __packet_lookup_frame_in_block(po, skb, status, len);
+ return __packet_lookup_frame_in_block(po, skb, len);
default:
WARN(1, "TPACKET version not supported\n");
BUG();
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH -next v2] packet: remove unused variable 'status' in __packet_lookup_frame_in_block
2019-06-11 1:32 ` [PATCH -next v2] " Mao Wenan
@ 2019-06-11 19:13 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2019-06-11 19:13 UTC (permalink / raw)
To: maowenan; +Cc: willemdebruijn.kernel, netdev, linux-kernel, kernel-janitors
From: Mao Wenan <maowenan@huawei.com>
Date: Tue, 11 Jun 2019 09:32:13 +0800
> The variable 'status' in __packet_lookup_frame_in_block() is never used since
> introduction in commit f6fb8f100b80 ("af-packet: TPACKET_V3 flexible buffer
> implementation."), we can remove it.
>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
> v2: don't change parameter from 0 to TP_STATUS_KERNEL when calls
> prb_retire_current_block().
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-06-11 19:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-10 11:58 [PATCH -next] packet: remove unused variable 'status' in __packet_lookup_frame_in_block Mao Wenan
2019-06-10 13:05 ` Willem de Bruijn
2019-06-10 14:02 ` maowenan
2019-06-10 14:34 ` Willem de Bruijn
2019-06-11 1:32 ` [PATCH -next v2] " Mao Wenan
2019-06-11 19:13 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).