netdev - Re: [PATCH net-next v2 04/14] net: ipv4: Add ip_mr

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]

Message-ID: <20250613095143.37b5500b@kernel.org>
Date: Fri, 13 Jun 2025 09:51:43 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Petr Machata <petrm@...dia.com>
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, David Ahern
 <dsahern@...il.com>, <netdev@...r.kernel.org>, Simon Horman
 <horms@...nel.org>, Nikolay Aleksandrov <razor@...ckwall.org>, Ido Schimmel
 <idosch@...dia.com>, <mlxsw@...dia.com>, Roopa Prabhu <roopa@...dia.com>,
 Benjamin Poirier <bpoirier@...dia.com>
Subject: Re: [PATCH net-next v2 04/14] net: ipv4: Add ip_mr_output()

On Thu, 12 Jun 2025 22:10:38 +0200 Petr Machata wrote:
> +	/* Forward the frame */
> +	if (c->mfc_origin == htonl(INADDR_ANY) &&
> +	    c->mfc_mcastgrp == htonl(INADDR_ANY)) {
> +		if (ip_hdr(skb)->ttl >
> +				c->_c.mfc_un.res.ttls[c->_c.mfc_parent]) {

weird indent?

> +			/* It's an (*,*) entry and the packet is not coming from
> +			 * the upstream: forward the packet to the upstream
> +			 * only.
> +			 */
> +			psend = c->_c.mfc_parent;
> +			goto last_xmit;
> +		}
> +		goto dont_xmit;
> +	}
> +
> +	for (ct = c->_c.mfc_un.res.maxvif - 1;
> +	     ct >= c->_c.mfc_un.res.minvif; ct--) {
> +		if (ip_hdr(skb)->ttl > c->_c.mfc_un.res.ttls[ct]) {

I'd be tempted to invert condition, continue, save a level of indent.
Presumably we expect TTL to actually be large enough so that'd also
make the expected path not under the if ?

> +			if (psend != -1) {
> +				struct sk_buff *skb2 = skb_clone(skb,
> +								 GFP_ATOMIC);
> +
> +				if (skb2)

maybe this is some local custom in this code but:

				struct sk_buff *skb2;

				skb2 = skb_clone(skb, GFP_ATOMIC);
				if (skb2)

same LoC, less ugly.

> +					ipmr_queue_output_xmit(net, mrt,
> +							       skb2, psend);