netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Horatiu Vultur <horatiu.vultur@microchip.com>
Cc: Jakub Kicinski <kuba@kernel.org>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	UNGLinuxDriver@microchip.com, davem@davemloft.net,
	kernel test robot <lkp@intel.com>
Subject: Re: [PATCH net-next] net: lan966x: Fix when CONFIG_IPV6 is not set
Date: Thu, 10 Feb 2022 14:14:27 +0100	[thread overview]
Message-ID: <YgUPswHG60Fi9rjA@lunn.ch> (raw)
In-Reply-To: <20220210083612.4mszzwgcrvmn67rn@soft-dev3-1.localhost>

> What do you think if I do something like this in the lan966x_main.h
> 
> ---
> #if IS_ENABLED(CONFIG_IPV6)
> static inline bool lan966x_hw_offload_ipv6(struct sk_buff *skb)
> {
> 	if (skb->protocol == htons(ETH_P_IPV6) &&
> 	    ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr) &&
> 	    !ipv6_mc_check_mld(skb))
> 		return false;
> 
> 	return true;
> }
> #else
> static inline bool lan966x_hw_offload_ipv6(struct sk_buff *skb)
> {
> 	return false;
> }
> #endif
> ---

The reason we prefer not to use #if is that it reduced compile testing
coverage. The block of code inside gets compiled a lot less.

> And then in lan966x_main.c just call this function.
> 
> > 
> > If it's linking we can do:
> > 
> >         if (IS_ENABLED(CONFIG_IPV6) &&
> >             skb->protocol == htons(ETH_P_IPV6) &&
> >             ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr) &&
> >             !ipv6_mc_check_mld(skb))
> >                 return false;

Jakub solution results in the code always being compiled, but the
IS_ENABLED(CONFIG_IPV6) gets turned into a constant 0 or 1. The
optimizer can then remove the whole block of code in the 0 case.

> I was also looking at other drivers on how they use 'ipv6_mc_check_mld'.
> Then I have seen that drivers/net/amt.c and net/bridge/br_multicast.c
> they wrap this function with #if.
> But then there is net/batman-adv/multicast.c which doesn't do that and
> it can compile and link without CONFIG_IPV6 and I just don't see how
> that is working.

Maybe it is to do with this at the end of net/ip6/Makefile

ifneq ($(CONFIG_IPV6),)
obj-$(CONFIG_NET_UDP_TUNNEL) += ip6_udp_tunnel.o
obj-y += mcast_snoop.o
endif


      reply	other threads:[~2022-02-10 13:14 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-09 10:18 [PATCH net-next] net: lan966x: Fix when CONFIG_IPV6 is not set Horatiu Vultur
2022-02-09 13:54 ` Andrew Lunn
2022-02-10  2:06   ` Jakub Kicinski
2022-02-10  8:36     ` Horatiu Vultur
2022-02-10 13:14       ` Andrew Lunn [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YgUPswHG60Fi9rjA@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=davem@davemloft.net \
    --cc=horatiu.vultur@microchip.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkp@intel.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).