[PATCH net-next 0/8] Use max/min to simplify the code

[PATCH net-next 0/8] Use max/min to simplify the code

[Hongbo Li]

Many Coccinelle/coccicheck warning reported by minmax.cocci
in net module, such as:
        WARNING opportunity for max()
        WARNING opportunity for min()

Let's use max/min to simplify the code and fix these warnings.
These patch have passed compilation test.

Hongbo Li (8):
  net/mac80211: use max to simplify the code
  net/rds: Use max() to simplify the code
  net/ipv4: Use min() to simplify the code
  net/core: Use min()/max() to simplify the code
  net/dccp: Use min()/max() to simplify the code
  net/openvswitch: Use max() to simplify the code
  net/rxrpc: Use min() to simplify the code
  net/ceph: Use min() to simplify the code

 net/ceph/osd_client.c     | 2 +-
 net/core/pktgen.c         | 6 ++----
 net/core/sock.c           | 2 +-
 net/dccp/ackvec.c         | 2 +-
 net/dccp/dccp.h           | 2 +-
 net/ipv4/ip_sockglue.c    | 2 +-
 net/mac80211/driver-ops.h | 2 +-
 net/mac80211/mlme.c       | 2 +-
 net/mac80211/scan.c       | 6 ++----
 net/mac80211/tdls.c       | 2 +-
 net/openvswitch/vport.c   | 2 +-
 net/rds/info.c            | 5 +----
 net/rxrpc/input.c         | 3 +--
 13 files changed, 15 insertions(+), 23 deletions(-)

-- 
2.34.1

[PATCH net-next 1/8] net/mac80211: use max to simplify the code

[Hongbo Li]

The following Coccinelle/coccicheck warning reported by
minmax.cocci:
    WARNING opportunity for max()
Let's use max() to simplify the code and fix the warning.

Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
---
 net/mac80211/driver-ops.h | 2 +-
 net/mac80211/mlme.c       | 2 +-
 net/mac80211/scan.c       | 6 ++----
 net/mac80211/tdls.c       | 2 +-
 4 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index d382d9729e85..6b75c7eeff25 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -971,7 +971,7 @@ drv_mgd_protect_tdls_discover(struct ieee80211_local *local,
 		return;
 	WARN_ON_ONCE(sdata->vif.type != NL80211_IFTYPE_STATION);
 
-	link_id = link_id > 0 ? link_id : 0;
+	link_id = max(link_id, 0);
 
 	trace_drv_mgd_protect_tdls_discover(local, sdata);
 	if (local->ops->mgd_protect_tdls_discover)
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 4779a18ab75d..60a7631f0457 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -5375,7 +5375,7 @@ ieee80211_determine_our_sta_mode_auth(struct ieee80211_sub_if_data *sdata,
 				      struct ieee80211_conn_settings *conn)
 {
 	ieee80211_determine_our_sta_mode(sdata, sband, NULL, wmm_used,
-					 req->link_id > 0 ? req->link_id : 0,
+					 max(req->link_id, 0),
 					 conn);
 }
 
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index b5f2df61c7f6..e77c9f07b046 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -1013,10 +1013,8 @@ static void ieee80211_scan_state_set_channel(struct ieee80211_local *local,
 	 */
 	if ((chan->flags & (IEEE80211_CHAN_NO_IR | IEEE80211_CHAN_RADAR)) ||
 	    !scan_req->n_ssids) {
-		*next_delay = msecs_to_jiffies(scan_req->duration) >
-			      IEEE80211_PASSIVE_CHANNEL_TIME ?
-			      msecs_to_jiffies(scan_req->duration) :
-			      IEEE80211_PASSIVE_CHANNEL_TIME;
+		*next_delay = max(msecs_to_jiffies(scan_req->duration),
+				  IEEE80211_PASSIVE_CHANNEL_TIME);
 		local->next_scan_state = SCAN_DECISION;
 		if (scan_req->n_ssids)
 			set_bit(SCAN_BEACON_WAIT, &local->scanning);
diff --git a/net/mac80211/tdls.c b/net/mac80211/tdls.c
index f07b40916485..719739def96c 100644
--- a/net/mac80211/tdls.c
+++ b/net/mac80211/tdls.c
@@ -919,7 +919,7 @@ ieee80211_tdls_build_mgmt_packet_data(struct ieee80211_sub_if_data *sdata,
 	int ret;
 	struct ieee80211_link_data *link;
 
-	link_id = link_id >= 0 ? link_id : 0;
+	link_id = max(link_id, 0);
 	rcu_read_lock();
 	link = rcu_dereference(sdata->link[link_id]);
 	if (WARN_ON(!link))
-- 
2.34.1

[PATCH net-next 2/8] net/rds: Use max() to simplify the code

[Hongbo Li]

The target if-else can be replaced with max().

Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
---
 net/rds/info.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/net/rds/info.c b/net/rds/info.c
index b6b46a8214a0..8558b0a466b4 100644
--- a/net/rds/info.c
+++ b/net/rds/info.c
@@ -194,10 +194,7 @@ int rds_info_getsockopt(struct socket *sock, int optname, char __user *optval,
 	}
 	ret = pin_user_pages_fast(start, nr_pages, FOLL_WRITE, pages);
 	if (ret != nr_pages) {
-		if (ret > 0)
-			nr_pages = ret;
-		else
-			nr_pages = 0;
+		nr_pages = max(ret, 0);
 		ret = -EAGAIN; /* XXX ? */
 		goto out;
 	}
-- 
2.34.1

[PATCH net-next 3/8] net/ipv4: Use min() to simplify the code

[Hongbo Li]

The following Coccinelle/coccicheck warning reported by
minmax.cocci:
    WARNING opportunity for min()
Let's use min() to simplify the code and fix the warning.

Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
---
 net/ipv4/ip_sockglue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index cf377377b52d..f35aba4c3b0e 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -282,7 +282,7 @@ int ip_cmsg_send(struct sock *sk, struct msghdr *msg, struct ipcm_cookie *ipc,
 			/* Our caller is responsible for freeing ipc->opt */
 			err = ip_options_get(net, &ipc->opt,
 					     KERNEL_SOCKPTR(CMSG_DATA(cmsg)),
-					     err < 40 ? err : 40);
+					     min(40, err));
 			if (err)
 				return err;
 			break;
-- 
2.34.1

[PATCH net-next 4/8] net/core: Use min()/max() to simplify the code

[Hongbo Li]

Let's use min()/max() to simplify the code and fix the
Coccinelle/coccicheck warning.

Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
---
 net/core/pktgen.c | 6 ++----
 net/core/sock.c   | 2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index ea55a758a475..5d4d5ec4a126 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -2793,8 +2793,7 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
 		}
 
 		i = 0;
-		frag_len = (datalen/frags) < PAGE_SIZE ?
-			   (datalen/frags) : PAGE_SIZE;
+		frag_len = min(datalen/frags, PAGE_SIZE);
 		while (datalen > 0) {
 			if (unlikely(!pkt_dev->page)) {
 				int node = numa_node_id();
@@ -2811,8 +2810,7 @@ static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
 			if (i == (frags - 1))
 				skb_frag_fill_page_desc(&skb_shinfo(skb)->frags[i],
 							pkt_dev->page, 0,
-							(datalen < PAGE_SIZE ?
-							 datalen : PAGE_SIZE));
+							min(datalen, PAGE_SIZE));
 			else
 				skb_frag_fill_page_desc(&skb_shinfo(skb)->frags[i],
 							pkt_dev->page, 0, frag_len);
diff --git a/net/core/sock.c b/net/core/sock.c
index bbe4c58470c3..c9910f48903f 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -3800,7 +3800,7 @@ int sock_prot_inuse_get(struct net *net, struct proto *prot)
 	for_each_possible_cpu(cpu)
 		res += per_cpu_ptr(net->core.prot_inuse, cpu)->val[idx];
 
-	return res >= 0 ? res : 0;
+	return max(res, 0);
 }
 EXPORT_SYMBOL_GPL(sock_prot_inuse_get);
 
-- 
2.34.1

[PATCH net-next 5/8] net/dccp: Use min()/max() to simplify the code

[Hongbo Li]

Let's use min()/max() to simplify the code and fix the
Coccinelle/coccicheck warning reported by minmax.cocci.

Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
---
 net/dccp/ackvec.c | 2 +-
 net/dccp/dccp.h   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/dccp/ackvec.c b/net/dccp/ackvec.c
index 1cba001bb4c8..faadd0190107 100644
--- a/net/dccp/ackvec.c
+++ b/net/dccp/ackvec.c
@@ -305,7 +305,7 @@ void dccp_ackvec_clear_state(struct dccp_ackvec *av, const u64 ackno)
 	 * Deal with overlapping Ack Vectors: don't subtract more than the
 	 * number of packets between tail_ackno and ack_ackno.
 	 */
-	eff_runlen = delta < avr->avr_ack_runlen ? delta : avr->avr_ack_runlen;
+	eff_runlen = min(delta, avr->avr_ack_runlen);
 
 	runlen_now = dccp_ackvec_runlen(av->av_buf + avr->avr_ack_ptr);
 	/*
diff --git a/net/dccp/dccp.h b/net/dccp/dccp.h
index 1f748ed1279d..872d17fb85b5 100644
--- a/net/dccp/dccp.h
+++ b/net/dccp/dccp.h
@@ -149,7 +149,7 @@ static inline u64 dccp_loss_count(const u64 s1, const u64 s2, const u64 ndp)
 	WARN_ON(delta < 0);
 	delta -= ndp + 1;
 
-	return delta > 0 ? delta : 0;
+	return max(delta, 0);
 }
 
 /**
-- 
2.34.1

[PATCH net-next 6/8] net/openvswitch: Use max() to simplify the code

[Hongbo Li]

Let's use max() to simplify the code and fix the
Coccinelle/coccicheck warning reported by minmax.cocci.

Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
---
 net/openvswitch/vport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index 8732f6e51ae5..859208df65ea 100644
--- a/net/openvswitch/vport.c
+++ b/net/openvswitch/vport.c
@@ -534,7 +534,7 @@ static int packet_length(const struct sk_buff *skb,
 	 * account for 802.1ad. e.g. is_skb_forwardable().
 	 */
 
-	return length > 0 ? length : 0;
+	return max(length, 0);
 }
 
 void ovs_vport_send(struct vport *vport, struct sk_buff *skb, u8 mac_proto)
-- 
2.34.1

[PATCH net-next 7/8] net/rxrpc: Use min() to simplify the code

[Hongbo Li]

Let's use min() to simplify the code and fix the
Coccinelle/coccicheck warning reported by minmax.cocci.

Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
---
 net/rxrpc/input.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 16d49a861dbb..455aa0189b28 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -868,8 +868,7 @@ static void rxrpc_input_ack(struct rxrpc_call *call, struct sk_buff *skb)
 	prev_pkt	= sp->ack.prev_ack;
 	nr_acks		= sp->ack.nr_acks;
 	hard_ack	= first_soft_ack - 1;
-	summary.ack_reason = (sp->ack.reason < RXRPC_ACK__INVALID ?
-			      sp->ack.reason : RXRPC_ACK__INVALID);
+	summary.ack_reason = min(sp->ack.reason, RXRPC_ACK__INVALID);
 
 	trace_rxrpc_rx_ack(call, ack_serial, acked_serial,
 			   first_soft_ack, prev_pkt,
-- 
2.34.1

[PATCH net-next 8/8] net/ceph: Use min() to simplify the code

[Hongbo Li]

Let's use min() to simplify the code and fix the
Coccinelle/coccicheck warning reported by minmax.cocci.

Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
---
 net/ceph/osd_client.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
index 9d078b37fe0b..450eb3be48b0 100644
--- a/net/ceph/osd_client.c
+++ b/net/ceph/osd_client.c
@@ -3058,7 +3058,7 @@ static void linger_reg_commit_complete(struct ceph_osd_linger_request *lreq,
 				       int result)
 {
 	if (!completion_done(&lreq->reg_commit_wait)) {
-		lreq->reg_commit_error = (result <= 0 ? result : 0);
+		lreq->reg_commit_error = min(result, 0);
 		complete_all(&lreq->reg_commit_wait);
 	}
 }
-- 
2.34.1

Re: [PATCH net-next 7/8] net/rxrpc: Use min() to simplify the code

[David Howells]

Hongbo Li <lihongbo22@huawei.com> wrote:

> -	summary.ack_reason = (sp->ack.reason < RXRPC_ACK__INVALID ?
> -			      sp->ack.reason : RXRPC_ACK__INVALID);
> +	summary.ack_reason = min(sp->ack.reason, RXRPC_ACK__INVALID);

Can you use umin() rather than min(), please?

Thanks,
David

Re: [PATCH net-next 7/8] net/rxrpc: Use min() to simplify the code

[Hongbo Li]

On 2024/8/24 20:06, David Howells wrote:
> Hongbo Li <lihongbo22@huawei.com> wrote:
> 
>> -	summary.ack_reason = (sp->ack.reason < RXRPC_ACK__INVALID ?
>> -			      sp->ack.reason : RXRPC_ACK__INVALID);
>> +	summary.ack_reason = min(sp->ack.reason, RXRPC_ACK__INVALID);
> 
> Can you use umin() rather than min(), please?
> I see reason is u8, may I use min_t(u8, sp->ack.reason, RXRPC_ACK__INVALID);

Thanks,
Hongbo

> Thanks,
> David
> 
> 

Re: [PATCH net-next 7/8] net/rxrpc: Use min() to simplify the code

[Hongbo Li]

On 2024/8/24 20:06, David Howells wrote:
> Hongbo Li <lihongbo22@huawei.com> wrote:
> 
>> -	summary.ack_reason = (sp->ack.reason < RXRPC_ACK__INVALID ?
>> -			      sp->ack.reason : RXRPC_ACK__INVALID);
>> +	summary.ack_reason = min(sp->ack.reason, RXRPC_ACK__INVALID);
> 
> Can you use umin() rather than min(), please?
> 

I see reason is u8, so may I use min_t(u8, sp->ack.reason, 
RXRPC_ACK__INVALID)?

Thanks,
Hongbo


> Thanks,
> David
> 
> 

Re: [PATCH net-next 6/8] net/openvswitch: Use max() to simplify the code

[Eelco Chaudron]

On 24 Aug 2024, at 9:40, Hongbo Li wrote:

> Let's use max() to simplify the code and fix the
> Coccinelle/coccicheck warning reported by minmax.cocci.
>
> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>

The change looks good to me.

Acked-by: Eelco Chaudron <echaudro@redhat.com>
> ---
>  net/openvswitch/vport.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
> index 8732f6e51ae5..859208df65ea 100644
> --- a/net/openvswitch/vport.c
> +++ b/net/openvswitch/vport.c
> @@ -534,7 +534,7 @@ static int packet_length(const struct sk_buff *skb,
>  	 * account for 802.1ad. e.g. is_skb_forwardable().
>  	 */
>
> -	return length > 0 ? length : 0;
> +	return max(length, 0);
>  }
>
>  void ovs_vport_send(struct vport *vport, struct sk_buff *skb, u8 mac_proto)
> -- 
> 2.34.1

Re: [ovs-dev] [PATCH net-next 6/8] net/openvswitch: Use max() to simplify the code

[Aaron Conole]

Hongbo Li via dev <ovs-dev@openvswitch.org> writes:

> Let's use max() to simplify the code and fix the
> Coccinelle/coccicheck warning reported by minmax.cocci.
>
> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
> ---

Reviewed-by: Aaron Conole <aconole@redhat.com>

>  net/openvswitch/vport.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
> index 8732f6e51ae5..859208df65ea 100644
> --- a/net/openvswitch/vport.c
> +++ b/net/openvswitch/vport.c
> @@ -534,7 +534,7 @@ static int packet_length(const struct sk_buff *skb,
>  	 * account for 802.1ad. e.g. is_skb_forwardable().
>  	 */
>  
> -	return length > 0 ? length : 0;
> +	return max(length, 0);
>  }
>  
>  void ovs_vport_send(struct vport *vport, struct sk_buff *skb, u8 mac_proto)

Re: [PATCH net-next 1/8] net/mac80211: use max to simplify the code

[Kalle Valo]

Hongbo Li <lihongbo22@huawei.com> writes:

> The following Coccinelle/coccicheck warning reported by
> minmax.cocci:
>     WARNING opportunity for max()
> Let's use max() to simplify the code and fix the warning.
>
> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
> ---
>  net/mac80211/driver-ops.h | 2 +-
>  net/mac80211/mlme.c       | 2 +-
>  net/mac80211/scan.c       | 6 ++----
>  net/mac80211/tdls.c       | 2 +-
>  4 files changed, 5 insertions(+), 7 deletions(-)

mac80211 patches go to wireless-next, please submit this separately. And
the title should begin with 'wifi: mac80211:'.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH net-next 0/8] Use max/min to simplify the code

[Jakub Kicinski]

On Sat, 24 Aug 2024 15:40:25 +0800 Hongbo Li wrote:
> Many Coccinelle/coccicheck warning reported by minmax.cocci
> in net module, such as:
>         WARNING opportunity for max()
>         WARNING opportunity for min()
> 
> Let's use max/min to simplify the code and fix these warnings.
> These patch have passed compilation test.

This set does not build.

Re: [PATCH net-next 0/8] Use max/min to simplify the code

[Hongbo Li]

On 2024/8/27 5:44, Jakub Kicinski wrote:
> On Sat, 24 Aug 2024 15:40:25 +0800 Hongbo Li wrote:
>> Many Coccinelle/coccicheck warning reported by minmax.cocci
>> in net module, such as:
>>          WARNING opportunity for max()
>>          WARNING opportunity for min()
>>
>> Let's use max/min to simplify the code and fix these warnings.
>> These patch have passed compilation test.
> 
> This set does not build.
> 
Do you mean some patches will go to other branches (such as mac80211)?

Thanks,
Hongbo
> 

Re: [PATCH net-next 0/8] Use max/min to simplify the code

[Kalle Valo]

Hongbo Li <lihongbo22@huawei.com> writes:

> On 2024/8/27 5:44, Jakub Kicinski wrote:
>> On Sat, 24 Aug 2024 15:40:25 +0800 Hongbo Li wrote:
>>> Many Coccinelle/coccicheck warning reported by minmax.cocci
>>> in net module, such as:
>>>          WARNING opportunity for max()
>>>          WARNING opportunity for min()
>>>
>>> Let's use max/min to simplify the code and fix these warnings.
>>> These patch have passed compilation test.
>> This set does not build.
>> 
> Do you mean some patches will go to other branches (such as mac80211)?

Jakub means that your patchset had compilation errors, see the red on
patchwork:

https://patchwork.kernel.org/project/netdevbpf/list/?series=882901&state=*&order=date

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH net-next 2/8] net/rds: Use max() to simplify the code

[Allison Henderson]

On Sat, 2024-08-24 at 15:40 +0800, Hongbo Li wrote:
> The target if-else can be replaced with max().
> 
> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>

Looks simple enough to me.  Thanks Hongbo!

Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>  net/rds/info.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/net/rds/info.c b/net/rds/info.c
> index b6b46a8214a0..8558b0a466b4 100644
> --- a/net/rds/info.c
> +++ b/net/rds/info.c
> @@ -194,10 +194,7 @@ int rds_info_getsockopt(struct socket *sock, int
> optname, char __user *optval,
>         }
>         ret = pin_user_pages_fast(start, nr_pages, FOLL_WRITE,
> pages);
>         if (ret != nr_pages) {
> -               if (ret > 0)
> -                       nr_pages = ret;
> -               else
> -                       nr_pages = 0;
> +               nr_pages = max(ret, 0);
>                 ret = -EAGAIN; /* XXX ? */
>                 goto out;
>         }

Re: [PATCH net-next 0/8] Use max/min to simplify the code

[Jakub Kicinski]

On Tue, 27 Aug 2024 07:45:02 +0300 Kalle Valo wrote:
> > Do you mean some patches will go to other branches (such as mac80211)?  
> 
> Jakub means that your patchset had compilation errors, see the red on
> patchwork:
> 
> https://patchwork.kernel.org/project/netdevbpf/list/?series=882901&state=*&order=date

FWIW I prefer not to point noobs to the patchwork checks, lest they
think it's a public CI and they can fling broken code at the list :(
But yes, in case "code doesn't build" needs a further explanation:

net/core/pktgen.c: In function ‘pktgen_finalize_skb’:
./../include/linux/compiler_types.h:510:45: error: call to ‘__compiletime_assert_928’ declared with attribute error: min(datalen/frags, ((1UL) << 12)) signedness error
  510 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
      |                                             ^
./../include/linux/compiler_types.h:491:25: note: in definition of macro ‘__compiletime_assert’
  491 |                         prefix ## suffix();                             \
      |                         ^~~~~~
./../include/linux/compiler_types.h:510:9: note: in expansion of macro ‘_compiletime_assert’
  510 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
      |         ^~~~~~~~~~~~~~~~~~~
../include/linux/build_bug.h:39:37: note: in expansion of macro ‘compiletime_assert’
   39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
      |                                     ^~~~~~~~~~~~~~~~~~
../include/linux/minmax.h:100:9: note: in expansion of macro ‘BUILD_BUG_ON_MSG’
  100 |         BUILD_BUG_ON_MSG(!__types_ok(x,y,ux,uy),        \
      |         ^~~~~~~~~~~~~~~~
../include/linux/minmax.h:105:9: note: in expansion of macro ‘__careful_cmp_once’
  105 |         __careful_cmp_once(op, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
      |         ^~~~~~~~~~~~~~~~~~
../include/linux/minmax.h:129:25: note: in expansion of macro ‘__careful_cmp’
  129 | #define min(x, y)       __careful_cmp(min, x, y)
      |                         ^~~~~~~~~~~~~
../net/core/pktgen.c:2796:28: note: in expansion of macro ‘min’
 2796 |                 frag_len = min(datalen/frags, PAGE_SIZE);
      |                            ^~~
make[5]: *** [../scripts/Makefile.build:244: net/core/pktgen.o] Error 1

Re: [PATCH net-next 0/8] Use max/min to simplify the code

[Kalle Valo]

Jakub Kicinski <kuba@kernel.org> writes:

> On Tue, 27 Aug 2024 07:45:02 +0300 Kalle Valo wrote:
>> > Do you mean some patches will go to other branches (such as mac80211)?  
>> 
>> Jakub means that your patchset had compilation errors, see the red on
>> patchwork:
>> 
>> https://patchwork.kernel.org/project/netdevbpf/list/?series=882901&state=*&order=date
>
> FWIW I prefer not to point noobs to the patchwork checks, lest they
> think it's a public CI and they can fling broken code at the list :(

Good point, that's definitely what we do not want. I'll keep this in
mind.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
https://docs.kernel.org/process/submitting-patches.html

Re: [PATCH net-next 7/8] net/rxrpc: Use min() to simplify the code

[Simon Horman]

On Mon, Aug 26, 2024 at 10:50:03AM +0800, Hongbo Li wrote:
> 
> 
> On 2024/8/24 20:06, David Howells wrote:
> > Hongbo Li <lihongbo22@huawei.com> wrote:
> > 
> > > -	summary.ack_reason = (sp->ack.reason < RXRPC_ACK__INVALID ?
> > > -			      sp->ack.reason : RXRPC_ACK__INVALID);
> > > +	summary.ack_reason = min(sp->ack.reason, RXRPC_ACK__INVALID);
> > 
> > Can you use umin() rather than min(), please?
> > 
> 
> I see reason is u8, so may I use min_t(u8, sp->ack.reason,
> RXRPC_ACK__INVALID)?

I believe that umin was added precisely to avoid such constructions.

See: 80fcac55385c ("minmax: add umin(a, b) and umax(a, b)")
     https://git.kernel.org/torvalds/c/80fcac55385c

Re: [PATCH net-next 4/8] net/core: Use min()/max() to simplify the code

[kernel test robot]

Hi Hongbo,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Hongbo-Li/net-mac80211-use-max-to-simplify-the-code/20240826-154029
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240824074033.2134514-5-lihongbo22%40huawei.com
patch subject: [PATCH net-next 4/8] net/core: Use min()/max() to simplify the code
config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20240828/202408281516.thVNxpEo-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 08e5a1de8227512d4774a534b91cb2353cef6284)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240828/202408281516.thVNxpEo-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408281516.thVNxpEo-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from net/core/pktgen.c:126:
   In file included from include/linux/ptrace.h:10:
   In file included from include/linux/pid_namespace.h:7:
   In file included from include/linux/mm.h:2228:
   include/linux/vmstat.h:517:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     517 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   In file included from net/core/pktgen.c:129:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     548 |         val = __raw_readb(PCI_IOBASE + addr);
         |                           ~~~~~~~~~~ ^
   include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     561 |         val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
      37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
         |                                                   ^
   In file included from net/core/pktgen.c:129:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     574 |         val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
         |                                                         ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
      35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
         |                                                   ^
   In file included from net/core/pktgen.c:129:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:14:
   In file included from arch/hexagon/include/asm/io.h:328:
   include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     585 |         __raw_writeb(value, PCI_IOBASE + addr);
         |                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     595 |         __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
   include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     605 |         __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
         |                                                       ~~~~~~~~~~ ^
>> net/core/pktgen.c:2796:14: error: call to '__compiletime_assert_743' declared with 'error' attribute: min(datalen/frags, (1UL << 12)) signedness error
    2796 |                 frag_len = min(datalen/frags, PAGE_SIZE);
         |                            ^
   include/linux/minmax.h:129:19: note: expanded from macro 'min'
     129 | #define min(x, y)       __careful_cmp(min, x, y)
         |                         ^
   include/linux/minmax.h:105:2: note: expanded from macro '__careful_cmp'
     105 |         __careful_cmp_once(op, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
         |         ^
   include/linux/minmax.h:100:2: note: expanded from macro '__careful_cmp_once'
     100 |         BUILD_BUG_ON_MSG(!__types_ok(x,y,ux,uy),        \
         |         ^
   note: (skipping 2 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
   include/linux/compiler_types.h:498:2: note: expanded from macro '_compiletime_assert'
     498 |         __compiletime_assert(condition, msg, prefix, suffix)
         |         ^
   include/linux/compiler_types.h:491:4: note: expanded from macro '__compiletime_assert'
     491 |                         prefix ## suffix();                             \
         |                         ^
   <scratch space>:6:1: note: expanded from here
       6 | __compiletime_assert_743
         | ^
   7 warnings and 1 error generated.


vim +2796 net/core/pktgen.c

  2769	
  2770	static void pktgen_finalize_skb(struct pktgen_dev *pkt_dev, struct sk_buff *skb,
  2771					int datalen)
  2772	{
  2773		struct timespec64 timestamp;
  2774		struct pktgen_hdr *pgh;
  2775	
  2776		pgh = skb_put(skb, sizeof(*pgh));
  2777		datalen -= sizeof(*pgh);
  2778	
  2779		if (pkt_dev->nfrags <= 0) {
  2780			skb_put_zero(skb, datalen);
  2781		} else {
  2782			int frags = pkt_dev->nfrags;
  2783			int i, len;
  2784			int frag_len;
  2785	
  2786	
  2787			if (frags > MAX_SKB_FRAGS)
  2788				frags = MAX_SKB_FRAGS;
  2789			len = datalen - frags * PAGE_SIZE;
  2790			if (len > 0) {
  2791				skb_put_zero(skb, len);
  2792				datalen = frags * PAGE_SIZE;
  2793			}
  2794	
  2795			i = 0;
> 2796			frag_len = min(datalen/frags, PAGE_SIZE);
  2797			while (datalen > 0) {
  2798				if (unlikely(!pkt_dev->page)) {
  2799					int node = numa_node_id();
  2800	
  2801					if (pkt_dev->node >= 0 && (pkt_dev->flags & F_NODE))
  2802						node = pkt_dev->node;
  2803					pkt_dev->page = alloc_pages_node(node, GFP_KERNEL | __GFP_ZERO, 0);
  2804					if (!pkt_dev->page)
  2805						break;
  2806				}
  2807				get_page(pkt_dev->page);
  2808	
  2809				/*last fragment, fill rest of data*/
  2810				if (i == (frags - 1))
  2811					skb_frag_fill_page_desc(&skb_shinfo(skb)->frags[i],
  2812								pkt_dev->page, 0,
  2813								min(datalen, PAGE_SIZE));
  2814				else
  2815					skb_frag_fill_page_desc(&skb_shinfo(skb)->frags[i],
  2816								pkt_dev->page, 0, frag_len);
  2817	
  2818				datalen -= skb_frag_size(&skb_shinfo(skb)->frags[i]);
  2819				skb->len += skb_frag_size(&skb_shinfo(skb)->frags[i]);
  2820				skb->data_len += skb_frag_size(&skb_shinfo(skb)->frags[i]);
  2821				i++;
  2822				skb_shinfo(skb)->nr_frags = i;
  2823			}
  2824		}
  2825	
  2826		/* Stamp the time, and sequence number,
  2827		 * convert them to network byte order
  2828		 */
  2829		pgh->pgh_magic = htonl(PKTGEN_MAGIC);
  2830		pgh->seq_num = htonl(pkt_dev->seq_num);
  2831	
  2832		if (pkt_dev->flags & F_NO_TIMESTAMP) {
  2833			pgh->tv_sec = 0;
  2834			pgh->tv_usec = 0;
  2835		} else {
  2836			/*
  2837			 * pgh->tv_sec wraps in y2106 when interpreted as unsigned
  2838			 * as done by wireshark, or y2038 when interpreted as signed.
  2839			 * This is probably harmless, but if anyone wants to improve
  2840			 * it, we could introduce a variant that puts 64-bit nanoseconds
  2841			 * into the respective header bytes.
  2842			 * This would also be slightly faster to read.
  2843			 */
  2844			ktime_get_real_ts64(&timestamp);
  2845			pgh->tv_sec = htonl(timestamp.tv_sec);
  2846			pgh->tv_usec = htonl(timestamp.tv_nsec / NSEC_PER_USEC);
  2847		}
  2848	}
  2849	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next 4/8] net/core: Use min()/max() to simplify the code

[kernel test robot]

Hi Hongbo,

kernel test robot noticed the following build errors:

[auto build test ERROR on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Hongbo-Li/net-mac80211-use-max-to-simplify-the-code/20240826-154029
base:   net-next/main
patch link:    https://lore.kernel.org/r/20240824074033.2134514-5-lihongbo22%40huawei.com
patch subject: [PATCH net-next 4/8] net/core: Use min()/max() to simplify the code
config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20240828/202408281628.FtNGguag-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240828/202408281628.FtNGguag-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408281628.FtNGguag-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from <command-line>:
   net/core/pktgen.c: In function 'pktgen_finalize_skb':
>> include/linux/compiler_types.h:510:45: error: call to '__compiletime_assert_991' declared with attribute error: min(datalen/frags, (1UL << 16)) signedness error
     510 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |                                             ^
   include/linux/compiler_types.h:491:25: note: in definition of macro '__compiletime_assert'
     491 |                         prefix ## suffix();                             \
         |                         ^~~~~~
   include/linux/compiler_types.h:510:9: note: in expansion of macro '_compiletime_assert'
     510 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
      39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
         |                                     ^~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:100:9: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     100 |         BUILD_BUG_ON_MSG(!__types_ok(x,y,ux,uy),        \
         |         ^~~~~~~~~~~~~~~~
   include/linux/minmax.h:105:9: note: in expansion of macro '__careful_cmp_once'
     105 |         __careful_cmp_once(op, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
         |         ^~~~~~~~~~~~~~~~~~
   include/linux/minmax.h:129:25: note: in expansion of macro '__careful_cmp'
     129 | #define min(x, y)       __careful_cmp(min, x, y)
         |                         ^~~~~~~~~~~~~
   net/core/pktgen.c:2796:28: note: in expansion of macro 'min'
    2796 |                 frag_len = min(datalen/frags, PAGE_SIZE);
         |                            ^~~


vim +/__compiletime_assert_991 +510 include/linux/compiler_types.h

eb5c2d4b45e3d2 Will Deacon 2020-07-21  496  
eb5c2d4b45e3d2 Will Deacon 2020-07-21  497  #define _compiletime_assert(condition, msg, prefix, suffix) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21  498  	__compiletime_assert(condition, msg, prefix, suffix)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  499  
eb5c2d4b45e3d2 Will Deacon 2020-07-21  500  /**
eb5c2d4b45e3d2 Will Deacon 2020-07-21  501   * compiletime_assert - break build and emit msg if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  502   * @condition: a compile-time constant condition to check
eb5c2d4b45e3d2 Will Deacon 2020-07-21  503   * @msg:       a message to emit if condition is false
eb5c2d4b45e3d2 Will Deacon 2020-07-21  504   *
eb5c2d4b45e3d2 Will Deacon 2020-07-21  505   * In tradition of POSIX assert, this macro will break the build if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  506   * supplied condition is *false*, emitting the supplied error message if the
eb5c2d4b45e3d2 Will Deacon 2020-07-21  507   * compiler has support to do so.
eb5c2d4b45e3d2 Will Deacon 2020-07-21  508   */
eb5c2d4b45e3d2 Will Deacon 2020-07-21  509  #define compiletime_assert(condition, msg) \
eb5c2d4b45e3d2 Will Deacon 2020-07-21 @510  	_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
eb5c2d4b45e3d2 Will Deacon 2020-07-21  511  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH net-next 7/8] net/rxrpc: Use min() to simplify the code

[David Howells]

Hongbo Li <lihongbo22@huawei.com> wrote:

> I see reason is u8, so may I use min_t(u8, sp->ack.reason,
> RXRPC_ACK__INVALID)?

No, please don't use min_t(<unsigned type>, ...) if umin() will do.  IIRC,
some arches can't do byte-level arithmetic.

Thanks,
David

Re: [PATCH net-next 2/8] net/rds: Use max() to simplify the code

[Simon Horman]

On Sat, Aug 24, 2024 at 03:40:27PM +0800, Hongbo Li wrote:
> The target if-else can be replaced with max().
> 
> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
> ---
>  net/rds/info.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/net/rds/info.c b/net/rds/info.c
> index b6b46a8214a0..8558b0a466b4 100644
> --- a/net/rds/info.c
> +++ b/net/rds/info.c
> @@ -194,10 +194,7 @@ int rds_info_getsockopt(struct socket *sock, int optname, char __user *optval,
>  	}
>  	ret = pin_user_pages_fast(start, nr_pages, FOLL_WRITE, pages);
>  	if (ret != nr_pages) {
> -		if (ret > 0)
> -			nr_pages = ret;
> -		else
> -			nr_pages = 0;
> +		nr_pages = max(ret, 0);

Along the same lines as Johannes Berg's comment on a different patch [1]
I think that there is a subtle but important difference, semantically,
between max() and that the existing code does, for which the best
description I can think of is setting a floor on the value.

Other than Johannes's comment, and now mine here, I think you will find
that, if you search the netdev ML, you will find this point being made
consistently, at least over the past year.

And yes, we understand that mathematically max() is doing the right thing.
But that is not the point that is being made here.

I suggest dropping this patch.
And any others like it.

[1] https://lore.kernel.org/all/d5f495b67fe6bf128e7a51b9fcfe11f70c9b66ae.camel@sipsolutions.net/

>  		ret = -EAGAIN; /* XXX ? */
>  		goto out;
>  	}
> -- 
> 2.34.1
> 
> 

Re: [PATCH net-next 4/8] net/core: Use min()/max() to simplify the code

[Simon Horman]

On Sat, Aug 24, 2024 at 03:40:29PM +0800, Hongbo Li wrote:
> Let's use min()/max() to simplify the code and fix the
> Coccinelle/coccicheck warning.
> 
> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>

...

> diff --git a/net/core/sock.c b/net/core/sock.c
> index bbe4c58470c3..c9910f48903f 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -3800,7 +3800,7 @@ int sock_prot_inuse_get(struct net *net, struct proto *prot)
>  	for_each_possible_cpu(cpu)
>  		res += per_cpu_ptr(net->core.prot_inuse, cpu)->val[idx];
>  
> -	return res >= 0 ? res : 0;
> +	return max(res, 0);
>  }
>  EXPORT_SYMBOL_GPL(sock_prot_inuse_get);

As per my comment on 2/8 [*], I think you should drop this hunk.

[*] https://lore.kernel.org/all/20240828135310.GC1368797@kernel.org/

Re: [PATCH net-next 5/8] net/dccp: Use min()/max() to simplify the code

[Simon Horman]

On Sat, Aug 24, 2024 at 03:40:30PM +0800, Hongbo Li wrote:
> Let's use min()/max() to simplify the code and fix the
> Coccinelle/coccicheck warning reported by minmax.cocci.
> 
> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
> ---
>  net/dccp/ackvec.c | 2 +-
>  net/dccp/dccp.h   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/dccp/ackvec.c b/net/dccp/ackvec.c
> index 1cba001bb4c8..faadd0190107 100644
> --- a/net/dccp/ackvec.c
> +++ b/net/dccp/ackvec.c
> @@ -305,7 +305,7 @@ void dccp_ackvec_clear_state(struct dccp_ackvec *av, const u64 ackno)
>  	 * Deal with overlapping Ack Vectors: don't subtract more than the
>  	 * number of packets between tail_ackno and ack_ackno.
>  	 */
> -	eff_runlen = delta < avr->avr_ack_runlen ? delta : avr->avr_ack_runlen;
> +	eff_runlen = min(delta, avr->avr_ack_runlen);

delta is s64, but known to be non-negative
avr->avr_ack_runlen is u8

I _think_ this is a candidate for umin().

>  
>  	runlen_now = dccp_ackvec_runlen(av->av_buf + avr->avr_ack_ptr);
>  	/*
> diff --git a/net/dccp/dccp.h b/net/dccp/dccp.h
> index 1f748ed1279d..872d17fb85b5 100644
> --- a/net/dccp/dccp.h
> +++ b/net/dccp/dccp.h
> @@ -149,7 +149,7 @@ static inline u64 dccp_loss_count(const u64 s1, const u64 s2, const u64 ndp)
>  	WARN_ON(delta < 0);
>  	delta -= ndp + 1;
>  
> -	return delta > 0 ? delta : 0;
> +	return max(delta, 0);
>  }

As per my comment on 2/8 [*], I think you should drop this hunk.

[*] https://lore.kernel.org/all/20240828135310.GC1368797@kernel.org/

Re: [PATCH net-next 6/8] net/openvswitch: Use max() to simplify the code

[Simon Horman]

+ Aaron, Eelco

On Sat, Aug 24, 2024 at 03:40:31PM +0800, Hongbo Li wrote:
> Let's use max() to simplify the code and fix the
> Coccinelle/coccicheck warning reported by minmax.cocci.
> 
> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
> ---
>  net/openvswitch/vport.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
> index 8732f6e51ae5..859208df65ea 100644
> --- a/net/openvswitch/vport.c
> +++ b/net/openvswitch/vport.c
> @@ -534,7 +534,7 @@ static int packet_length(const struct sk_buff *skb,
>  	 * account for 802.1ad. e.g. is_skb_forwardable().
>  	 */
>  
> -	return length > 0 ? length : 0;
> +	return max(length, 0);

As per my comment on 2/8 [*], I think it would be best to drop this patch.

[*] https://lore.kernel.org/all/20240828135310.GC1368797@kernel.org/

Re: [PATCH net-next 8/8] net/ceph: Use min() to simplify the code

[Simon Horman]

On Sat, Aug 24, 2024 at 03:40:33PM +0800, Hongbo Li wrote:
> Let's use min() to simplify the code and fix the
> Coccinelle/coccicheck warning reported by minmax.cocci.
> 
> Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
> ---
>  net/ceph/osd_client.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/ceph/osd_client.c b/net/ceph/osd_client.c
> index 9d078b37fe0b..450eb3be48b0 100644
> --- a/net/ceph/osd_client.c
> +++ b/net/ceph/osd_client.c
> @@ -3058,7 +3058,7 @@ static void linger_reg_commit_complete(struct ceph_osd_linger_request *lreq,
>  				       int result)
>  {
>  	if (!completion_done(&lreq->reg_commit_wait)) {
> -		lreq->reg_commit_error = (result <= 0 ? result : 0);
> +		lreq->reg_commit_error = min(result, 0);
>  		complete_all(&lreq->reg_commit_wait);
>  	}
>  }

As per my comment on 2/8 [*], I think it would be best to drop this patch.

[*] https://lore.kernel.org/all/20240828135310.GC1368797@kernel.org/

RE: [PATCH net-next 7/8] net/rxrpc: Use min() to simplify the code

[David Laight]

From: David Howells <dhowells@redhat.com>
> Sent: 28 August 2024 09:18
> 
> Hongbo Li <lihongbo22@huawei.com> wrote:
> 
> > I see reason is u8, so may I use min_t(u8, sp->ack.reason,
> > RXRPC_ACK__INVALID)?
> 
> No, please don't use min_t(<unsigned type>, ...) if umin() will do.  IIRC,
> some arches can't do byte-level arithmetic.

Not to mention all the places where the wrong type has been used and
significant bits masked off before the comparison.

Is there even a problem with min() here?
It should be fine unless sp->ack.reason is a signed type.
In which case things are probably horribly wrong if it is negative.

Looking at the code I'm not even sure that min() is right.
It really ought to be used for counters/sizes.
This is a bit like the (broken) suggestion of replacing:
	return rval < 0 ? rval : 0;
with
	return min(rval, 0);

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

RE: [PATCH net-next 0/8] Use max/min to simplify the code

[David Laight]

From: Jakub Kicinski
> Sent: 27 August 2024 15:04
> 
> On Tue, 27 Aug 2024 07:45:02 +0300 Kalle Valo wrote:
> > > Do you mean some patches will go to other branches (such as mac80211)?
> >
> > Jakub means that your patchset had compilation errors, see the red on
> > patchwork:
> >
> > https://patchwork.kernel.org/project/netdevbpf/list/?series=882901&state=*&order=date
> 
> FWIW I prefer not to point noobs to the patchwork checks, lest they
> think it's a public CI and they can fling broken code at the list :(
> But yes, in case "code doesn't build" needs a further explanation:
> 
> net/core/pktgen.c: In function ‘pktgen_finalize_skb’:
> ./../include/linux/compiler_types.h:510:45: error: call to ‘__compiletime_assert_928’ declared with
> attribute error: min(datalen/frags, ((1UL) << 12)) signedness error
...
> ../net/core/pktgen.c:2796:28: note: in expansion of macro ‘min’
>  2796 |                 frag_len = min(datalen/frags, PAGE_SIZE);
>       |                            ^~~

I can't help feeling that a signed divide isn't intended here.
Which rather implies that both datalen and frags are signed types.
Whereas neither can be sensibly negative.
Perhaps that is the real bug?

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)