v1 = replace if()/BUG with BUG_ON() for tx_ring.
v2 = remove handling of tx_ring in prb_setup_retire_blk_timer
for TPACKET_V3 because init_prb_bdqc is called only for NULL tx_ring
and thus prb_setup_retire_blk_timer for NULL tx_ring only.
And also in funciton init_prb_bdqc there is no usage of tx_ring.
Thus removing tx_ring from init_prb_bdqc.
Signed-off-by: Maninder Singh <[email protected]>
Suggested-by: Frans Klaver <[email protected]>
---
net/packet/af_packet.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index fd51641..aeafcf0 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -543,15 +543,11 @@ static void prb_init_blk_timer(struct packet_sock *po,
pkc->retire_blk_timer.expires = jiffies;
}
-static void prb_setup_retire_blk_timer(struct packet_sock *po, int tx_ring)
+static void prb_setup_retire_blk_timer(struct packet_sock *po)
{
struct tpacket_kbdq_core *pkc;
- if (tx_ring)
- BUG();
-
- pkc = tx_ring ? GET_PBDQC_FROM_RB(&po->tx_ring) :
- GET_PBDQC_FROM_RB(&po->rx_ring);
+ pkc = GET_PBDQC_FROM_RB(&po->rx_ring);
prb_init_blk_timer(po, pkc, prb_retire_rx_blk_timer_expired);
}
@@ -607,7 +603,7 @@ static void prb_init_ft_ops(struct tpacket_kbdq_core *p1,
static void init_prb_bdqc(struct packet_sock *po,
struct packet_ring_buffer *rb,
struct pgv *pg_vec,
- union tpacket_req_u *req_u, int tx_ring)
+ union tpacket_req_u *req_u)
{
struct tpacket_kbdq_core *p1 = GET_PBDQC_FROM_RB(rb);
struct tpacket_block_desc *pbd;
@@ -634,7 +630,7 @@ static void init_prb_bdqc(struct packet_sock *po,
p1->max_frame_len = p1->kblk_size - BLK_PLUS_PRIV(p1->blk_sizeof_priv);
prb_init_ft_ops(p1, req_u);
- prb_setup_retire_blk_timer(po, tx_ring);
+ prb_setup_retire_blk_timer(po);
prb_open_block(p1, pbd);
}
@@ -4001,7 +3997,7 @@ static int packet_set_ring(struct sock *sk, union tpacket_req_u *req_u,
* it above but just being paranoid
*/
if (!tx_ring)
- init_prb_bdqc(po, rb, pg_vec, req_u, tx_ring);
+ init_prb_bdqc(po, rb, pg_vec, req_u);
break;
default:
break;
--
1.7.9.5
On Mon, Jun 22, 2015 at 7:54 AM, Maninder Singh <[email protected]> wrote:
> v1 = replace if()/BUG with BUG_ON() for tx_ring.
>
> v2 =
I would keep this below the ---. There's little historical use for
this version information when it gets merged.
> remove handling of tx_ring in prb_setup_retire_blk_timer
> for TPACKET_V3 because init_prb_bdqc is called only for NULL tx_ring
> and thus prb_setup_retire_blk_timer for NULL tx_ring only.
I'd say tx_ring is false, rather than NULL. It's not a pointer (here).
> And also in funciton init_prb_bdqc there is no usage of tx_ring.
s,funciton,function,
> Thus removing tx_ring from init_prb_bdqc.
>
> Signed-off-by: Maninder Singh <[email protected]>
> Suggested-by: Frans Klaver <[email protected]>
> ---
> net/packet/af_packet.c | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index fd51641..aeafcf0 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -543,15 +543,11 @@ static void prb_init_blk_timer(struct packet_sock *po,
> pkc->retire_blk_timer.expires = jiffies;
> }
>
> -static void prb_setup_retire_blk_timer(struct packet_sock *po, int tx_ring)
> +static void prb_setup_retire_blk_timer(struct packet_sock *po)
> {
> struct tpacket_kbdq_core *pkc;
>
> - if (tx_ring)
> - BUG();
> -
> - pkc = tx_ring ? GET_PBDQC_FROM_RB(&po->tx_ring) :
> - GET_PBDQC_FROM_RB(&po->rx_ring);
> + pkc = GET_PBDQC_FROM_RB(&po->rx_ring);
> prb_init_blk_timer(po, pkc, prb_retire_rx_blk_timer_expired);
> }
>
> @@ -607,7 +603,7 @@ static void prb_init_ft_ops(struct tpacket_kbdq_core *p1,
> static void init_prb_bdqc(struct packet_sock *po,
> struct packet_ring_buffer *rb,
> struct pgv *pg_vec,
> - union tpacket_req_u *req_u, int tx_ring)
> + union tpacket_req_u *req_u)
> {
> struct tpacket_kbdq_core *p1 = GET_PBDQC_FROM_RB(rb);
> struct tpacket_block_desc *pbd;
> @@ -634,7 +630,7 @@ static void init_prb_bdqc(struct packet_sock *po,
>
> p1->max_frame_len = p1->kblk_size - BLK_PLUS_PRIV(p1->blk_sizeof_priv);
> prb_init_ft_ops(p1, req_u);
> - prb_setup_retire_blk_timer(po, tx_ring);
> + prb_setup_retire_blk_timer(po);
> prb_open_block(p1, pbd);
> }
>
> @@ -4001,7 +3997,7 @@ static int packet_set_ring(struct sock *sk, union tpacket_req_u *req_u,
> * it above but just being paranoid
> */
> if (!tx_ring)
> - init_prb_bdqc(po, rb, pg_vec, req_u, tx_ring);
> + init_prb_bdqc(po, rb, pg_vec, req_u);
> break;
> default:
> break;
> --
> 1.7.9.5
Looks like what I made myself yesterday evening.
Thanks,
Frans
Hi Frans,
>> v1 = replace if()/BUG with BUG_ON() for tx_ring.
>>
>> v2 =
>
>I would keep this below the ---. There's little historical use for
>this version information when it gets merged.
>
>> remove handling of tx_ring in prb_setup_retire_blk_timer
>> for TPACKET_V3 because init_prb_bdqc is called only for NULL tx_ring
>> and thus prb_setup_retire_blk_timer for NULL tx_ring only.
>
>I'd say tx_ring is false, rather than NULL. It's not a pointer (here).
>
>
>> And also in funciton init_prb_bdqc there is no usage of tx_ring.
>
>s,funciton,function,
Thanks for feedback , please check below changelog if it looks ok,
Then i will share updated patch:-
v1 = replace if()/BUG with BUG_ON() for tx_ring.
Signed-off-by: Maninder Singh <[email protected]>
Signed-off-by: Frans Klaver <[email protected]>
---
Changes in v2:
Remove handling of tx_ring in prb_setup_retire_blk_timer
for TPACKET_V3 because init_prb_bdqc is called only for zero tx_ring
and thus prb_setup_retire_blk_timer for zero tx_ring only.
And also in functon init_prb_bdqc there is no usage of tx_ring.
Thus removing tx_ring from init_prb_bdqc.
net/packet/af_packet.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
Thanks
Maninder????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????s?y? ?杶????i?????? ????i
Hi,
On Mon, Jun 22, 2015 at 8:53 AM, Maninder Singh <[email protected]> wrote:
> Hi Frans,
>
>>> v1 = replace if()/BUG with BUG_ON() for tx_ring.
>>>
>>> v2 =
>>
>>I would keep this below the ---. There's little historical use for
>>this version information when it gets merged.
>>
>>> remove handling of tx_ring in prb_setup_retire_blk_timer
>>> for TPACKET_V3 because init_prb_bdqc is called only for NULL tx_ring
>>> and thus prb_setup_retire_blk_timer for NULL tx_ring only.
>>
>>I'd say tx_ring is false, rather than NULL. It's not a pointer (here).
>>
>>
>>> And also in funciton init_prb_bdqc there is no usage of tx_ring.
>>
>>s,funciton,function,
>
> Thanks for feedback , please check below changelog if it looks ok,
> Then i will share updated patch:-
>
> v1 = replace if()/BUG with BUG_ON() for tx_ring.
>
> Signed-off-by: Maninder Singh <[email protected]>
> Signed-off-by: Frans Klaver <[email protected]>
No, the Suggested-by: was better for me. You can't go about and add
Signed-off-by lines for someone else without permission ;-).
> ---
> Changes in v2:
>
> Remove handling of tx_ring in prb_setup_retire_blk_timer
> for TPACKET_V3 because init_prb_bdqc is called only for zero tx_ring
> and thus prb_setup_retire_blk_timer for zero tx_ring only.
>
> And also in functon init_prb_bdqc there is no usage of tx_ring.
> Thus removing tx_ring from init_prb_bdqc.
>
> net/packet/af_packet.c | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> Thanks
> Maninder
No, I didn't make myself clear enough, I'm afraid. The info about the
different incarnations of your patch should go below the dashes. The
whole "Remove handling ...." text should be your commit message,
because that is what you want to see in the commit log. Here's an
example:
Subject: [RFC PATCH v2] packet: remove handling of tx_ring
Remove handling of tx_ring in prb_setup_retire_blk_timer for
TPACKET_V3 because init_prb_bdqc is called only for zero tx_ring and
thus prb_setup_retire_blk_timer for zero tx_ring only.
And also in functon init_prb_bdqc there is no usage of tx_ring. Thus
removing tx_ring from init_prb_bdqc.
Signed-off-by: Maninder Singh <[email protected]>
Suggested-by: Frans Klaver <[email protected]>
---
v1..v2: remove BUG() by removing tx_path
diffstat & patch