Received-SPF: pass (google.com: domain of linux-kernel+bounces-96998-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Fri, 08 Mar 2024 07:24:30 -0500
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: Juntong Deng <juntong.deng@outlook.com>, 
 Willem de Bruijn <willemdebruijn.kernel@gmail.com>, 
 davem@davemloft.net, 
 edumazet@google.com, 
 kuba@kernel.org, 
 pabeni@redhat.com
Cc: netdev@vger.kernel.org, 
 linux-kernel@vger.kernel.org
Message-ID: <65eb037ebbd4a_1356ef29497@willemb.c.googlers.com.notmuch>
In-Reply-To: <AM6PR03MB58489E6E890BA4A20D429DDC99272@AM6PR03MB5848.eurprd03.prod.outlook.com>
References: <AM6PR03MB5848595A20BB5D958C2D9DE299272@AM6PR03MB5848.eurprd03.prod.outlook.com>
 <65eaf9e336d07_133b25294db@willemb.c.googlers.com.notmuch>
 <AM6PR03MB58489E6E890BA4A20D429DDC99272@AM6PR03MB5848.eurprd03.prod.outlook.com>
Subject: Re: [PATCH net-next] net/packet: Add getsockopt support for
 PACKET_COPY_THRESH
Precedence: bulk
Mime-Version: 1.0
Content-Type: text/plain;
 charset=utf-8
Content-Transfer-Encoding: 7bit

Juntong Deng wrote:
> On 2024/3/8 19:43, Willem de Bruijn wrote:
> > Juntong Deng wrote:
> >> Currently getsockopt does not support PACKET_COPY_THRESH,
> >> and we are unable to get the value of PACKET_COPY_THRESH
> >> socket option through getsockopt.
> >>
> >> This patch adds getsockopt support for PACKET_COPY_THRESH.
> >>
> >> Signed-off-by: Juntong Deng <juntong.deng@outlook.com>
> >> ---
> >>   net/packet/af_packet.c | 3 +++
> >>   1 file changed, 3 insertions(+)
> >>
> >> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> >> index 0db31ca4982d..65042edd1a97 100644
> >> --- a/net/packet/af_packet.c
> >> +++ b/net/packet/af_packet.c
> >> @@ -4090,6 +4090,9 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,
> >>   	case PACKET_VNET_HDR_SZ:
> >>   		val = READ_ONCE(po->vnet_hdr_sz);
> >>   		break;
> >> +	case PACKET_COPY_THRESH:
> >> +		val = pkt_sk(sk)->copy_thresh;
> >> +		break;
> > 
> > This is probably a good opportunity to use READ_ONCE/WRITE_ONCE for
> > this variable that can be modified and read concurrently.
> > 
> > Alternatively I can fix up all three locations in a follow-on patch.
> > 
> > No concerns with adding the getsockopt itself.
> > 
> >>   	case PACKET_VERSION:
> >>   		val = po->tp_version;
> >>   		break;
> >> -- 
> >> 2.39.2
> >>
> > 
> > 
> 
> Thanks for reviewing.
> 
> So, do I need to send PATCH V2 to add READ_ONCE?
> 
> Or do you want you to use a follow-on patch to fix all three locations 
> at once?

Please use READ_ONCE and convert the existing accesses to copy_thresh
in the same patch to READ_ONCE/WRITE_ONCE. That's simplest.