Re: [dpdk-dev] [PATCH] kni: fix compile errors on Oracle Linux6.4 and RHEL6.5

[Thomas Monjalon <thomas.monjalon@6wind.com>]

Hi Helin,

Please answer below the question.

2014-06-10 14:59, Zhang, Helin:
> Were you talking about the "#ifndef ether_addr_equal "?

yes

> The compile errors I copied in that patch shows that the function
> has already been defined in include/linux/etherdevice.h on Oracle
> Linux6.4. In this case, it seems not always true as you said.

No, ether_addr_equal() is defined for the processor but not for the preprocessor.

> The method came from the standard Linux driver, so I think
> it should be useful and has been reviewed by Linux kernel community.

No, I think that this code come from the opaque Intel development
of the sourceforge driver.
On kernel.org, there is no need for kcompat glue as drivers are in-tree.

> Oops, it seems that there are useless code which I need to remove.
> I will update it later! Thank you very much for the pointing!

Thanks
-- 
Thomas

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] 
> Sent: Tuesday, June 10, 2014 6:03 PM
> To: Zhang, Helin
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] kni: fix compile errors on Oracle Linux6.4 and RHEL6.5
> 
> Hi Helin,
> 
> 2014-06-09 16:38, Helin Zhang:
> >  #if ( LINUX_VERSION_CODE < KERNEL_VERSION(3,5,0) )  #define 
> > skb_tx_timestamp(skb) do {} while (0) -#if !(RHEL_RELEASE_CODE && 
> > RHEL_RELEASE_CODE >= RHEL_RELEASE_VERSION(6,4)) -static inline bool 
> > ether_addr_equal(const u8 *addr1, const u8 *addr2)
> > +#ifndef ether_addr_equal
> 
> It should be always true as it is a function (not known by the preprocessor).
> 
> > +static inline bool __kc_ether_addr_equal(const u8 *addr1, const u8 
> > +*addr2)
> >  {
> >  	return !compare_ether_addr(addr1, addr2);  } -#endif
> > +#define ether_addr_equal(_addr1, _addr2) 
> > +__kc_ether_addr_equal((_addr1),(_addr2))
> > +#endif /* __kc_ether_addr_equal*/
> 
> So it is always replacing ether_addr_equal() by a kcompat equivalent for old kernels, even if ether_addr_equal() is already defined as a C function.
> Just to confirm: is it really what we want?
> 
> [...]
> > -#endif
> > +#define ethtool_adv_to_mmd_eee_adv_t(adv) \
> > +__kc_ethtool_adv_to_mmd_eee_adv_t(adv)
> 
> An indentation is missing here.
> 
> > +#endif /* ethtool_adv_to_mmd_eee_adv_t */ #endif /* */
> 
> Why an empty comment?
> 
> Thanks
> --
> Thomas