netdev - Re: [PATCH net-next] net: Print real skb addresses for all net events

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:   Fri, 24 Jun 2022 02:48:24 -0600
From:   "Subash Abhinov Kasiviswanathan (KS)" <quic_subashab@...cinc.com>
To:     Eric Dumazet <edumazet@...gle.com>
CC:     David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        netdev <netdev@...r.kernel.org>, <quic_jzenner@...cinc.com>,
        Cong Wang <cong.wang@...edance.com>, <qitao.xu@...edance.com>,
        Sean Tranchetti <quic_stranche@...cinc.com>
Subject: Re: [PATCH net-next] net: Print real skb addresses for all net events



On 6/24/2022 12:27 AM, Eric Dumazet wrote:
> On Fri, Jun 24, 2022 at 8:09 AM Subash Abhinov Kasiviswanathan
> <quic_subashab@...cinc.com> wrote:
>>
>> Commit 65875073eddd ("net: use %px to print skb address in trace_netif_receive_skb")
>> added support for printing the real addresses for the events using
>> net_dev_template.
>>
> 
> It is not clear why the 'real address' is needed in trace events.
> 
> I would rather do the opposite.
> 

We don't need the real address. We just need the events to display in 
the same format - hashed address is fine.

> diff --git a/include/trace/events/net.h b/include/trace/events/net.h
> index 032b431b987b63b5fab1d3c763643d6192f2c325..da611a7aaf970f541949cdd87ac9203c4c7e81b1
> 100644
> --- a/include/trace/events/net.h
> +++ b/include/trace/events/net.h
> @@ -136,7 +136,7 @@ DECLARE_EVENT_CLASS(net_dev_template,
>                  __assign_str(name, skb->dev->name);
>          ),
> 
> -       TP_printk("dev=%s skbaddr=%px len=%u",
> +       TP_printk("dev=%s skbaddr=%p len=%u",
>                  __get_str(name), __entry->skbaddr, __entry->len)
>   )
> 

Qitao / Cong, do you have any concerns as it ends up reverting commit 
65875073eddd ("net: use %px to print skb address in 
trace_netif_receive_skb")

> diff --git a/include/trace/events/qdisc.h b/include/trace/events/qdisc.h
> index 59c945b66f9c7469bc071e2a27efb8bfa9eb19f7..a3995925cb057021dc779344d19f7e3724f6df3c
> 100644
> --- a/include/trace/events/qdisc.h
> +++ b/include/trace/events/qdisc.h
> @@ -41,7 +41,7 @@ TRACE_EVENT(qdisc_dequeue,
>                  __entry->txq_state      = txq->state;
>          ),
> 
> -       TP_printk("dequeue ifindex=%d qdisc handle=0x%X parent=0x%X
> txq_state=0x%lX packets=%d skbaddr=%px",
> +       TP_printk("dequeue ifindex=%d qdisc handle=0x%X parent=0x%X
> txq_state=0x%lX packets=%d skbaddr=%p",
>                    __entry->ifindex, __entry->handle, __entry->parent,
>                    __entry->txq_state, __entry->packets, __entry->skbaddr )
>   );
> @@ -70,7 +70,7 @@ TRACE_EVENT(qdisc_enqueue,
>                  __entry->parent  = qdisc->parent;
>          ),
> 
> -       TP_printk("enqueue ifindex=%d qdisc handle=0x%X parent=0x%X
> skbaddr=%px",
> +       TP_printk("enqueue ifindex=%d qdisc handle=0x%X parent=0x%X skbaddr=%p",
>                    __entry->ifindex, __entry->handle, __entry->parent,
> __entry->skbaddr)
>   );
> 
>