lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 20 Dec 2022 15:28:16 +0100
From:   Daniele Palmas <dnlplm@...il.com>
To:     Paolo Abeni <pabeni@...hat.com>
Cc:     David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Subash Abhinov Kasiviswanathan <quic_subashab@...cinc.com>,
        Sean Tranchetti <quic_stranche@...cinc.com>,
        Jonathan Corbet <corbet@....net>,
        Alexander Lobakin <alexandr.lobakin@...el.com>,
        Gal Pressman <gal@...dia.com>,
        Bjørn Mork <bjorn@...k.no>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Dave Taht <dave.taht@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v3 2/3] net: qualcomm: rmnet: add tx packets aggregation

Hello Paolo,

Il giorno ven 9 dic 2022 alle ore 08:38 Daniele Palmas
<dnlplm@...il.com> ha scritto:
>
> Il giorno mer 7 dic 2022 alle ore 13:46 Paolo Abeni
> <pabeni@...hat.com> ha scritto:
> > > +static void rmnet_map_flush_tx_packet_work(struct work_struct *work)
> > > +{
> > > +     struct sk_buff *skb = NULL;
> > > +     struct rmnet_port *port;
> > > +
> > > +     port = container_of(work, struct rmnet_port, agg_wq);
> > > +
> > > +     spin_lock_bh(&port->agg_lock);
> > > +     if (likely(port->agg_state == -EINPROGRESS)) {
> > > +             /* Buffer may have already been shipped out */
> > > +             if (likely(port->skbagg_head)) {
> > > +                     skb = port->skbagg_head;
> > > +                     reset_aggr_params(port);
> > > +             }
> > > +             port->agg_state = 0;
> > > +     }
> > > +
> > > +     spin_unlock_bh(&port->agg_lock);
> > > +     if (skb)
> > > +             rmnet_send_skb(port, skb);
> > > +}
> > > +
> > > +static enum hrtimer_restart rmnet_map_flush_tx_packet_queue(struct hrtimer *t)
> > > +{
> > > +     struct rmnet_port *port;
> > > +
> > > +     port = container_of(t, struct rmnet_port, hrtimer);
> > > +
> > > +     schedule_work(&port->agg_wq);
> >
> > Why you need to schedule a work and you can't instead call the core of
> > rmnet_map_flush_tx_packet_work() here? it looks like the latter does
> > not need process context...
> >
>
> Ack.
>

looks like removing the work is not as straightforward as I thought.

Now the timer cb has become:

static enum hrtimer_restart rmnet_map_flush_tx_packet_cb(struct hrtimer *t)
{
    struct sk_buff *skb = NULL;
    struct rmnet_port *port;

    port = container_of(t, struct rmnet_port, hrtimer);

    spin_lock_bh(&port->agg_lock);
    if (likely(port->agg_state == -EINPROGRESS)) {
        /* Buffer may have already been shipped out */
        if (likely(port->skbagg_head)) {
            skb = port->skbagg_head;
            reset_aggr_params(port);
        }
        port->agg_state = 0;
    }
    spin_unlock_bh(&port->agg_lock);

    if (skb)
        rmnet_send_skb(port, skb);

    return HRTIMER_NORESTART;
}

but this is causing the following warning:

[ 3106.701296] WARNING: CPU: 15 PID: 0 at kernel/softirq.c:375
__local_bh_enable_ip+0x54/0x70
...
[ 3106.701537] CPU: 15 PID: 0 Comm: swapper/15 Tainted: G           OE
     6.1.0-rc5-rmnet-v4-warn #1
[ 3106.701543] Hardware name: LENOVO 30DH00H2IX/1048, BIOS S08KT40A 08/23/2021
[ 3106.701546] RIP: 0010:__local_bh_enable_ip+0x54/0x70
[ 3106.701554] Code: a9 00 ff ff 00 74 27 65 ff 0d 08 bb 75 61 65 8b
05 01 bb 75 61 85 c0 74 06 5d c3 cc cc cc cc 0f 1f 44 00 00 5d c3 cc
cc cc cc <0f> 0b eb bf 65 66 8b 05 e0 ca 76 61 66 85 c0 74 cc e8 e6 fd
ff ff
[ 3106.701559] RSP: 0018:ffffb8aa80510ec8 EFLAGS: 00010006
[ 3106.701564] RAX: 0000000080010202 RBX: ffff932d7b687868 RCX: 0000000000000000
[ 3106.701569] RDX: 0000000000000001 RSI: 0000000000000201 RDI: ffffffffc0bd5f7c
[ 3106.701573] RBP: ffffb8aa80510ec8 R08: ffff933bdc3e31a0 R09: 000002d355c2f99d
[ 3106.701576] R10: 0000000000000000 R11: ffffb8aa80510ff8 R12: ffff932d7b687828
[ 3106.701580] R13: ffff932d7b687000 R14: ffff932cc1a76400 R15: ffff933bdc3e3180
[ 3106.701584] FS:  0000000000000000(0000) GS:ffff933bdc3c0000(0000)
knlGS:0000000000000000
[ 3106.701589] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 3106.701593] CR2: 00007ffc26dae080 CR3: 0000000209f04003 CR4: 00000000007706e0
[ 3106.701597] PKRU: 55555554
[ 3106.701599] Call Trace:
[ 3106.701602]  <IRQ>
[ 3106.701608]  _raw_spin_unlock_bh+0x1d/0x30
[ 3106.701623]  rmnet_map_flush_tx_packet_cb+0x4c/0x90 [rmnet]
[ 3106.701640]  ? rmnet_send_skb+0x90/0x90 [rmnet]
[ 3106.701655]  __hrtimer_run_queues+0x106/0x260
[ 3106.701664]  hrtimer_interrupt+0x101/0x220
[ 3106.701671]  __sysvec_apic_timer_interrupt+0x61/0x110
[ 3106.701677]  sysvec_apic_timer_interrupt+0x7b/0x90
[ 3106.701685]  </IRQ>
[ 3106.701687]  <TASK>
[ 3106.701689]  asm_sysvec_apic_timer_interrupt+0x1b/0x20
[ 3106.701694] RIP: 0010:cpuidle_enter_state+0xde/0x6e0
[ 3106.701704] Code: eb d6 60 e8 74 01 68 ff 8b 53 04 49 89 c7 0f 1f
44 00 00 31 ff e8 b2 23 67 ff 80 7d d0 00 0f 85 da 00 00 00 fb 0f 1f
44 00 00 <45> 85 f6 0f 88 01 02 00 00 4d 63 ee 49 83 fd 09 0f 87 b6 04
00 00
[ 3106.701709] RSP: 0018:ffffb8aa801dbe38 EFLAGS: 00000246
[ 3106.701713] RAX: ffff933bdc3f1380 RBX: ffffd8aa7fbc0700 RCX: 000000000000001f
[ 3106.701717] RDX: 000000000000000f RSI: 0000000000000002 RDI: 0000000000000000
[ 3106.701720] RBP: ffffb8aa801dbe88 R08: 000002d355d34146 R09: 000000000006e988
[ 3106.701723] R10: 0000000000000004 R11: 071c71c71c71c71c R12: ffffffffa04ba5c0
[ 3106.701727] R13: 0000000000000001 R14: 0000000000000001 R15: 000002d355d34146
[ 3106.701735]  ? cpuidle_enter_state+0xce/0x6e0
[ 3106.701744]  cpuidle_enter+0x2e/0x50
[ 3106.701751]  do_idle+0x204/0x290
[ 3106.701758]  cpu_startup_entry+0x20/0x30
[ 3106.701763]  start_secondary+0x122/0x160
[ 3106.701773]  secondary_startup_64_no_verify+0xe5/0xeb
[ 3106.701784]  </TASK>

The reason is not obvious to me, so I need to dig further...

Regards,
Daniele

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ