Re: [PATCH net-next v2] net/packet: Add getsockopt support for PACKET_COPY_THRESH

[Juntong Deng]

On 2024/3/8 21:11, Jason Xing wrote:
> On Fri, Mar 8, 2024 at 8:56 PM Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
> > On Fri, Mar 8, 2024 at 1:43 PM Juntong Deng <juntong.deng@xxxxxxxxxxx> 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.
> > >
> > > In addition, this patch converts access to copy_thresh to
> > > READ_ONCE/WRITE_ONCE.
> > >
> > > Signed-off-by: Juntong Deng <juntong.deng@xxxxxxxxxxx>
> > > ---
> > > V1 -> V2: Use READ_ONCE/WRITE_ONCE.
> > >
> > >   net/packet/af_packet.c | 7 +++++--
> > >   1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> > > index 0db31ca4982d..61270826b9ac 100644
> > > --- a/net/packet/af_packet.c
> > > +++ b/net/packet/af_packet.c
> > > @@ -2318,7 +2318,7 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
> > >          }
> > >          if (po->tp_version <= TPACKET_V2) {
> > >                  if (macoff + snaplen > po->rx_ring.frame_size) {
> > > -                       if (po->copy_thresh &&
> > > +                       if (READ_ONCE(po->copy_thresh) &&
> > >                              atomic_read(&sk->sk_rmem_alloc) < sk->sk_rcvbuf) {
> > >                                  if (skb_shared(skb)) {
> > >                                          copy_skb = skb_clone(skb, GFP_ATOMIC);
> > > @@ -3836,7 +3836,7 @@ packet_setsockopt(struct socket *sock, int level, int optname, sockptr_t optval,
> > >                  if (copy_from_sockptr(&val, optval, sizeof(val)))
> > >                          return -EFAULT;
> > >
> > > -               pkt_sk(sk)->copy_thresh = val;
> > > +               WRITE_ONCE(pkt_sk(sk)->copy_thresh, val);
> > >                  return 0;
> > >          }
> > >          case PACKET_VERSION:
> > > @@ -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 = READ_ONCE(pkt_sk(sk)->copy_thresh);
> > > +               break;
> > >          case PACKET_VERSION:
> > >                  val = po->tp_version;
> > >                  break;
> > > --
> > > 2.39.2
> >
> > I think you forgot to change net/packet/diag.c pdiag_put_info()
>
> Ah, he updated his patch so soon even without taking a break.
>
> I just replied to the v1 thread with three changes made.
>
> Juntong, you could check your v1 patch thread and you will see the
> missing point.
>
> Thanks,
> Jason

Hi Jason,

Thanks, I have fixed this in the v3 patch.