netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2] net/packet: Add getsockopt support for PACKET_COPY_THRESH
@ 2024-03-08 12:42 Juntong Deng
  2024-03-08 12:56 ` Eric Dumazet
  0 siblings, 1 reply; 5+ messages in thread
From: Juntong Deng @ 2024-03-08 12:42 UTC (permalink / raw)
  To: willemdebruijn.kernel, davem, edumazet, kuba, pabeni; +Cc: netdev, linux-kernel

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@outlook.com>
---
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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v2] net/packet: Add getsockopt support for PACKET_COPY_THRESH
  2024-03-08 12:42 [PATCH net-next v2] net/packet: Add getsockopt support for PACKET_COPY_THRESH Juntong Deng
@ 2024-03-08 12:56 ` Eric Dumazet
  2024-03-08 13:11   ` Jason Xing
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2024-03-08 12:56 UTC (permalink / raw)
  To: Juntong Deng
  Cc: willemdebruijn.kernel, davem, kuba, pabeni, netdev, linux-kernel

On Fri, Mar 8, 2024 at 1:43 PM Juntong Deng <juntong.deng@outlook.com> 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@outlook.com>
> ---
> 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()

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v2] net/packet: Add getsockopt support for PACKET_COPY_THRESH
  2024-03-08 12:56 ` Eric Dumazet
@ 2024-03-08 13:11   ` Jason Xing
  2024-03-08 13:19     ` Juntong Deng
  0 siblings, 1 reply; 5+ messages in thread
From: Jason Xing @ 2024-03-08 13:11 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Juntong Deng, willemdebruijn.kernel, davem, kuba, pabeni, netdev,
	linux-kernel

On Fri, Mar 8, 2024 at 8:56 PM Eric Dumazet <edumazet@google.com> wrote:
>
> On Fri, Mar 8, 2024 at 1:43 PM Juntong Deng <juntong.deng@outlook.com> 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@outlook.com>
> > ---
> > 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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v2] net/packet: Add getsockopt support for PACKET_COPY_THRESH
  2024-03-08 13:11   ` Jason Xing
@ 2024-03-08 13:19     ` Juntong Deng
  2024-03-08 13:27       ` Jason Xing
  0 siblings, 1 reply; 5+ messages in thread
From: Juntong Deng @ 2024-03-08 13:19 UTC (permalink / raw)
  To: Jason Xing, Eric Dumazet
  Cc: willemdebruijn.kernel, davem, kuba, pabeni, netdev, linux-kernel

On 2024/3/8 21:11, Jason Xing wrote:
> On Fri, Mar 8, 2024 at 8:56 PM Eric Dumazet <edumazet@google.com> wrote:
>>
>> On Fri, Mar 8, 2024 at 1:43 PM Juntong Deng <juntong.deng@outlook.com> 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@outlook.com>
>>> ---
>>> 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.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v2] net/packet: Add getsockopt support for PACKET_COPY_THRESH
  2024-03-08 13:19     ` Juntong Deng
@ 2024-03-08 13:27       ` Jason Xing
  0 siblings, 0 replies; 5+ messages in thread
From: Jason Xing @ 2024-03-08 13:27 UTC (permalink / raw)
  To: Juntong Deng
  Cc: Eric Dumazet, willemdebruijn.kernel, davem, kuba, pabeni, netdev,
	linux-kernel

On Fri, Mar 8, 2024 at 9:19 PM Juntong Deng <juntong.deng@outlook.com> wrote:
>
> On 2024/3/8 21:11, Jason Xing wrote:
> > On Fri, Mar 8, 2024 at 8:56 PM Eric Dumazet <edumazet@google.com> wrote:
> >>
> >> On Fri, Mar 8, 2024 at 1:43 PM Juntong Deng <juntong.deng@outlook.com> 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@outlook.com>
> >>> ---
> >>> 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.

The patch itself looks good to me.

But next time please do not post the patch too often, you have to wait
around 24 hour at least. We need to obey the rule. Then you have
plenty of time to collect/rethink about those suggestions...

Thanks,
Jason

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-03-08 13:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-08 12:42 [PATCH net-next v2] net/packet: Add getsockopt support for PACKET_COPY_THRESH Juntong Deng
2024-03-08 12:56 ` Eric Dumazet
2024-03-08 13:11   ` Jason Xing
2024-03-08 13:19     ` Juntong Deng
2024-03-08 13:27       ` Jason Xing

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).