Netdev List
 help / color / mirror / Atom feed
* Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM
From: Harini Katakam @ 2016-11-18 10:32 UTC (permalink / raw)
  To: Andrei Pistirica
  Cc: Rafal Ozieblo, Nicolas Ferre, harini.katakam@xilinx.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <27516aeb-b8f9-dc38-4d01-d2a5c5cf44dd@microchip.com>

tHi Andrei,

>> Yes, Andre's version of Cadence does not ability to report have RX
>> timestamp.
>> The version I worked with did. This is the old series I sent:
>> https://lkml.org/lkml/2015/9/11/92
>> But right now, i'm working on building on top of Andre's changes.
>
>
> I'm addressing maintainer's feedback on both patches:
> https://patchwork.kernel.org/patch/9310989/
> https://patchwork.kernel.org/patch/9310991/
>
> I've done all suggested updates, except the numerous 64bit divisions in the
> frequency adjustment callback. I've implemented a different algorithm which
> uses 2 64bit division, but I couldn't find a way to use only 1.
>
> Also, I have a version with timecounter/cyclecounter which shows a much
> better accuracy (less than 100ns level). In my opinion this could be a
> better implementation. What is you opinion about this? Did you try it?
>

I did not try timecounter on Cadence IP versions later than r1p06.
Because with sub ns precision in HW timestamp, that works better
than SW cyclecounter.
On older Zynq version, yes timecounter is used and is better.

Regards,
Harini

^ permalink raw reply

* Re: [PATCH net-next] bridge: add igmpv3 and mldv2 query support
From: kbuild test robot @ 2016-11-18 10:16 UTC (permalink / raw)
  To: Hangbin Liu
  Cc: kbuild-all, netdev, Hannes Frederic Sowa, Nikolay Aleksandrov,
	linus.luessing, Hangbin Liu
In-Reply-To: <1479454321-31304-1-git-send-email-liuhangbin@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2634 bytes --]

Hi Hangbin,

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

url:    https://github.com/0day-ci/linux/commits/Hangbin-Liu/bridge-add-igmpv3-and-mldv2-query-support/20161118-155854
config: i386-randconfig-r0-201646 (attached as .config)
compiler: gcc-5 (Debian 5.4.1-2) 5.4.1 20160904
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All error/warnings (new ones prefixed by >>):

   net/bridge/br_multicast.c: In function 'mld_force_mld_version':
>> net/bridge/br_multicast.c:688:24: error: 'struct net' has no member named 'ipv6'
     if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version != 0)
                           ^
   net/bridge/br_multicast.c:689:28: error: 'struct net' has no member named 'ipv6'
      return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version;
                               ^
   net/bridge/br_multicast.c: In function 'br_multicast_alloc_query':
>> net/bridge/br_multicast.c:699:27: error: implicit declaration of function '__in6_dev_get' [-Werror=implicit-function-declaration]
     struct inet6_dev *idev = __in6_dev_get(br->dev);
                              ^
>> net/bridge/br_multicast.c:699:27: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
>> net/bridge/br_multicast.c:699:20: warning: unused variable 'idev' [-Wunused-variable]
     struct inet6_dev *idev = __in6_dev_get(br->dev);
                       ^
   net/bridge/br_multicast.c: At top level:
   net/bridge/br_multicast.c:686:12: warning: 'mld_force_mld_version' defined but not used [-Wunused-function]
    static int mld_force_mld_version(const struct inet6_dev *idev)
               ^
   cc1: some warnings being treated as errors

vim +688 net/bridge/br_multicast.c

   682		return skb;
   683	}
   684	#endif
   685	
   686	static int mld_force_mld_version(const struct inet6_dev *idev)
   687	{
 > 688		if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version != 0)
 > 689			return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version;
   690		else
   691			return idev->cnf.force_mld_version;
   692	}
   693	
   694	static struct sk_buff *br_multicast_alloc_query(struct net_bridge *br,
   695							struct br_ip *addr,
   696							u8 *igmp_type)
   697	{
   698		struct in_device *in_dev = __in_dev_get_rcu(br->dev);
 > 699		struct inet6_dev *idev = __in6_dev_get(br->dev);
   700		switch (addr->proto) {
   701		case htons(ETH_P_IP):
   702			if (IGMP_V3_SEEN(in_dev))

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28193 bytes --]

^ permalink raw reply

* Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM
From: Nicolas Ferre @ 2016-11-18 10:14 UTC (permalink / raw)
  To: Rafal Ozieblo, Harini Katakam, Andrei Pistirica
  Cc: harini.katakam@xilinx.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <BN3PR07MB25162D82CEF09B8C7586B799C9B00@BN3PR07MB2516.namprd07.prod.outlook.com>

Le 18/11/2016 à 10:59, Rafal Ozieblo a écrit :
>>                                                     
>> -----Original Message-----                           
>> From: Harini Katakam [mailto:harinikatakamlinux@gmail.com] 
>> Sent: 18 listopada 2016 10:44                              
>> To: Rafal Ozieblo                                          
>> Cc: Nicolas Ferre; Andrei Pistirica; harini.katakam@xilinx.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org                                                                                             
>> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM                           
>>                                                                                                        
>> Hi Rafal,                                                                                               
>>                                                                                                        
>> On Fri, Nov 18, 2016 at 3:00 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:                              
>>>> -----Original Message-----                                                                            
>>>> From: Nicolas Ferre [mailto:nicolas.ferre@atmel.com]                                                  
>>>> Sent: 18 listopada 2016 10:10                                                                         
>>>> To: Rafal Ozieblo; Harini Katakam; Andrei Pistirica                                                   
>>>> Cc: harini.katakam@xilinx.com; netdev@vger.kernel.org;                                                
>>>> linux-kernel@vger.kernel.org                                                                          
>>>> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support                                 
>>>> for GEM                                                                                               
>>>>                                                                                                      
>>>> Le 18/11/2016 à 09:59, Rafal Ozieblo a écrit :                                                        
>>>>> Hello,                                                                                              
>>>>>                                                                                                     
>>>>>> From: Harini Katakam [mailto:harinikatakamlinux@gmail.com]                                         
>>>>>> Sent: 18 listopada 2016 05:30                                                                      
>>>>>> To: Rafal Ozieblo                                                                                  
>>>>>> Cc: Nicolas Ferre; harini.katakam@xilinx.com;                                                      
>>>>>> netdev@vger.kernel.org; linux-kernel@vger.kernel.org                                               
>>>>>> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing                                      
>>>>>> support for GEM                                                                                    
>>>>>>                                                                                                    
>>>>>> Hi Rafal,                                                                                          
>>>>>>                                                                                                    
>>>>>> On Thu, Nov 17, 2016 at 7:05 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:                         
>>>>>>> -----Original Message-----                                                                        
>>>>>>> From: Nicolas Ferre [mailto:nicolas.ferre@atmel.com]                                              
>>>>>>> Sent: 17 listopada 2016 14:29                                                                     
>>>>>>> To: Harini Katakam; Rafal Ozieblo                                                                 
>>>>>>> Cc: harini.katakam@xilinx.com; netdev@vger.kernel.org;                                            
>>>>>>> linux-kernel@vger.kernel.org                                                                      
>>>>>>> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing                                     
>>>>>>> support for GEM                                                                                   
>>>>>>>                                                                                                   
>>>>>>>> Le 17/11/2016 à 13:21, Harini Katakam a écrit :                                                  
>>>>>>>>> Hi Rafal,                                                                                       
>>>>>>>>>                                                                                                 
>>>>>>>>> On Thu, Nov 17, 2016 at 5:20 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:                      
>>>>>>>>>> Hello,                                                                                         
>>>>>>>>>> I think, there could a bug in your patch.                                                      
>>>>>>>>>>                                                                                                
>>>>>>>>>>> +                                                                                             
>>>>>>>>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT                                                          
>>>>>>>>>>> +             dmacfg |= GEM_BIT(ADDR64); #endif                                               
>>>>>>>>>>                                                                                                
>>>>>>>>>> You enable 64 bit addressing (64b dma bus width) always when appropriate architecture config option is enabled.                                                                                         
>>>>>>>>>> But there are some legacy controllers which do not support that feature. According Cadence hardware team:                                                                                               
>>>>>>>>>> "64 bit addressing was added in July 2013. Earlier version do not have it.                     
>>>>>>>>>> This feature was enhanced in release August 2014 to have separate upper address values for transmit and receive."                                                                                       
>>>>>>>>>>                                                                                                
>>>>>>>>>>> /* Bitfields in NSR */                                                                        
>>>>>>>>>>> @@ -474,6 +479,10 @@                                                                          
>>>>>>>>>>>  struct macb_dma_desc {                                                                       
>>>>>>>>>>  >      u32     addr;                                                                          
>>>>>>>>>>>       u32     ctrl;                                                                           
>>>>>>>>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT                                                          
>>>>>>>>>>> +     u32     addrh;                                                                          
>>>>>>>>>>> +     u32     resvd;                                                                          
>>>>>>>>>>> +#endif                                                                                       
>>>>>>>>>>>  };                                                                                           
>>>>>>>>>>                                                                                                
>>>>>>>>>> It will not work for legacy hardware. Old descriptor is 2 words wide, the new one is 4 words wide.                                                                                                      
>>>>>>>>>> If you enable CONFIG_ARCH_DMA_ADDR_T_64BIT but hardware doesn't                                
>>>>>>>>>> support it at all, you will miss every second descriptor.                                      
>>>>>>>>>>                                                                                                
>>>>>>>>>                                                                                                 
>>>>>>>>> True, this feature is not available in all of Cadence IP versions.                              
>>>>>>>>> In fact, the IP version Zynq does not support this. But the one in ZynqMP does.                 
>>>>>>>>> So, we enable kernel config for 64 bit DMA addressing for this                                  
>>>>>>>>> SoC and hence the driver picks it up. My assumption was that if                                 
>>>>>>>>> the legacy IP does not support                                                                  
>>>>>>>>> 64 bit addressing, then this DMA option wouldn't be enabled.                                    
>>>>>>>>>                                                                                                 
>>>>>>>>> There is a design config register in Cadence IP which is being                                  
>>>>>>>>> read to check for 64 bit address support - DMA mask is set based on that.                       
>>>>>>>>> But the addition of two descriptor words cannot be based on this runtime check.                 
>>>>>>>>> For this reason, all the static changes were placed under this check.                           
>>>>>>>>                                                                                                  
>>>>>>>> We have quite a bunch of options in this driver to determinate what is the real capacity of the underlying hardware.                                                                                      
>>>>>>>> If HW configuration registers are not appropriate, and it seems they are not, I would advice to simply use the DT compatibility string.                                                                   
>>>>>>>>                                                                                                  
>>>>>>>> Best regards,                                                                                    
>>>>>>>> --                                                                                               
>>>>>>>> Nicolas Ferre                                                                                    
>>>>>>>                                                                                                   
>>>>>>> HW configuration registers are appropriate. The issue is that this code doesn’t use the capability bit to switch between different dma descriptors (2 words vs. 4 words).                                  
>>>>>>> DMA descriptor size is chosen based on kernel configuration, not based on hardware capabilities.  
>>>>>>                                                                                                    
>>>>>> HW configuration register does give appropriate information.                                       
>>>>>> But addition of two address words in the macb descriptor structure is a static change.             
>>>>>>                                                                                                    
>>>>>> +static inline void macb_set_addr(struct macb_dma_desc *desc,                                      
>>>>>> +dma_addr_t                                                                                        
>>>>>> +addr) {                                                                                           
>>>>>> +       desc->addr = (u32)addr;                                                                    
>>>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT                                                               
>>>>>> +       desc->addrh = (u32)(addr >> 32); #endif                                                    
>>>>>> +                                                                                                  
>>>>>>                                                                                                    
>>>>>> Even if the #ifdef condition here is changed to HW config check, addr and addrh are different.     
>>>>>> And "addrh" entry has to be present for 64 bit desc case to be handled separately.                 
>>>>>> Can you please tell me how you propose change in DMA descriptor                                    
>>>>>> structure from                                                                                     
>>>>>> 4 to 2 or 2 to 4 words *after* reading the DCFG register?                                          
>>>>>                                                                                                     
>>>>> It is very complex problem. I wrote to you because I faced the same issue. I'm working on PTP implementation for macb. When PTP is enabled there are additional two words in buffer descriptor.              
>>>>
>>>> Moreover, we can use PTP without the 64bits descriptor support (early GEM revisions).
>>>>
>>>> BTW, note that there is an initiative ongoing with Andrei to support
>>>> PTP/1588 on these devices with patches already send: please synchronize with him.
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__marc.info_-3Fl-3D
>>>> linux-2Dnetdev-26m-3D147282092309112-26w-3D3&d=DgIDaQ&c=aUq983L2pue2Fq
>>>> KFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtC
>>>> g4L5E&m=jUmZzqwn1uRrvcqF3BHCHFC6ZsKoZkU3vT8gJ-7fnsE&s=kr_km0MKQBzpkaOt
>>>> 0fkM-FIajN1-pOylzzTjsXi-vak&e=
>>>> or
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.
>>>> org_patch_9310989_&d=DgIDaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_ha
>>>> XqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtCg4L5E&m=jUmZzqwn1uRrvcqF3B
>>>> HCHFC6ZsKoZkU3vT8gJ-7fnsE&s=ZbXVD5Lb5zGn7wUKIPYHxagIEKp_vJvrnkRa4qJfmT
>>>> Y&e=
>>>> and
>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.
>>>> org_patch_9310991_&d=DgIDaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_ha
>>>> XqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtCg4L5E&m=jUmZzqwn1uRrvcqF3B
>>>> HCHFC6ZsKoZkU3vT8gJ-7fnsE&s=Z0kRqUqh5Is4TmuaY7A4hnpizfdeq3HrgPhyDAMLuA
>>>> 8&e=
>>>>
>>>> Regards,
>>>> --
>>>> Nicolas Ferre
>>>
>>> Above patch doesn’t use hardware timestamps in descriptors at all. It doesn't use appropriate accuracy.
>>> We have our PTP patch ready, which use timestamps from descriptor. But we have not sent it yet because of compatibility issue.
>>> Of course, we can do it like Harini did:
>>>
>>> struct macb_dma_desc {
>>>         u32     addr;
>>>         u32     ctrl;
>>> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>>         u32     addrh;
>>>         u32     resvd;
>>> #endif
>>> #if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
>>>         u32     dma_desc_ts_1;
>>>         u32     dma_desc_ts_2;
>>> #endif
>>> };
>>>
>>> But in that approach we lose backward compatibility.
>>>
>>> What are your suggestion? Can we send patch like it is or wait for some common solution with backward compatibility?
>>>
>>
>> Yes, Andre's version of Cadence does not ability to report have RX timestamp.
>> The version I worked with did. This is the old series I sent:
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2015_9_11_92&d=DgIFaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtCg4L5E&m=1A1qxdlwY3kMlJ1JUJJL1EqLzWz4AKtfOxZf_vu8bWo&s=NGpEbbC4bYXUZjJ6n0Ud8F8p3fPz5EhTJ1O9-NKwCkQ&e=
>> But right now, i'm working on building on top of Andre's changes.
>>
>> Regards,
>> Harini
>>
> I can’t see a place where you enable extended descriptor for PTP. Did you add support for extended PTP descriptor?
> "DMA Configuration Register" 0x010:
> 29	tx_bd_extended_mode_en	"Enable TX extended BD mode. See TX BD control register definition for description of feature."
> 28	rx_bd_extended_mode_en	"Enable RX extended BD mode. See RX BD control register definition for description of feature."
> 
> Can I send you our patch for comparison ? 

You mean "send you and everyone else on the mailing-list" right?
Even if the first initiative has a bit or priority, like Harini's one
ported further by Andrei, we are still eager to share code and ideas to
make this support go forward.

I do think that the steps will be to include generic support for PTP
(without 64bits descriptors) and then plug the 64bits descriptor support
which is an newer extension of it (and definitively more robust and
leading to more accurate behavior, I know). Basically like Harini
suggested: "building on top of Andrei's changes".

Best regards,
-- 
Nicolas Ferre

^ permalink raw reply

* RE: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM
From: Rafal Ozieblo @ 2016-11-18  9:59 UTC (permalink / raw)
  To: Harini Katakam
  Cc: Nicolas Ferre, Andrei Pistirica, harini.katakam@xilinx.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAFcVECJT=b2jmQAtsTeXEwWad8L2VEVW1GOyTY9XYR5qUq=4Mw@mail.gmail.com>

>                                                     
>-----Original Message-----                           
>From: Harini Katakam [mailto:harinikatakamlinux@gmail.com] 
>Sent: 18 listopada 2016 10:44                              
>To: Rafal Ozieblo                                          
>Cc: Nicolas Ferre; Andrei Pistirica; harini.katakam@xilinx.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org                                                                                             
>Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM                           
>                                                                                                        
>Hi Rafal,                                                                                               
>                                                                                                        
>On Fri, Nov 18, 2016 at 3:00 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:                              
>>>-----Original Message-----                                                                            
>>>From: Nicolas Ferre [mailto:nicolas.ferre@atmel.com]                                                  
>>>Sent: 18 listopada 2016 10:10                                                                         
>>>To: Rafal Ozieblo; Harini Katakam; Andrei Pistirica                                                   
>>>Cc: harini.katakam@xilinx.com; netdev@vger.kernel.org;                                                
>>>linux-kernel@vger.kernel.org                                                                          
>>>Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support                                 
>>>for GEM                                                                                               
>>>                                                                                                      
>>>Le 18/11/2016 à 09:59, Rafal Ozieblo a écrit :                                                        
>>>> Hello,                                                                                              
>>>>                                                                                                     
>>>>> From: Harini Katakam [mailto:harinikatakamlinux@gmail.com]                                         
>>>>> Sent: 18 listopada 2016 05:30                                                                      
>>>>> To: Rafal Ozieblo                                                                                  
>>>>> Cc: Nicolas Ferre; harini.katakam@xilinx.com;                                                      
>>>>> netdev@vger.kernel.org; linux-kernel@vger.kernel.org                                               
>>>>> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing                                      
>>>>> support for GEM                                                                                    
>>>>>                                                                                                    
>>>>> Hi Rafal,                                                                                          
>>>>>                                                                                                    
>>>>> On Thu, Nov 17, 2016 at 7:05 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:                         
>>>>>> -----Original Message-----                                                                        
>>>>>> From: Nicolas Ferre [mailto:nicolas.ferre@atmel.com]                                              
>>>>>> Sent: 17 listopada 2016 14:29                                                                     
>>>>>> To: Harini Katakam; Rafal Ozieblo                                                                 
>>>>>> Cc: harini.katakam@xilinx.com; netdev@vger.kernel.org;                                            
>>>>>> linux-kernel@vger.kernel.org                                                                      
>>>>>> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing                                     
>>>>>> support for GEM                                                                                   
>>>>>>                                                                                                   
>>>>>>> Le 17/11/2016 à 13:21, Harini Katakam a écrit :                                                  
>>>>>>>> Hi Rafal,                                                                                       
>>>>>>>>                                                                                                 
>>>>>>>> On Thu, Nov 17, 2016 at 5:20 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:                      
>>>>>>>>> Hello,                                                                                         
>>>>>>>>> I think, there could a bug in your patch.                                                      
>>>>>>>>>                                                                                                
>>>>>>>>>> +                                                                                             
>>>>>>>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT                                                          
>>>>>>>>>> +             dmacfg |= GEM_BIT(ADDR64); #endif                                               
>>>>>>>>>                                                                                                
>>>>>>>>> You enable 64 bit addressing (64b dma bus width) always when appropriate architecture config option is enabled.                                                                                         
>>>>>>>>> But there are some legacy controllers which do not support that feature. According Cadence hardware team:                                                                                               
>>>>>>>>> "64 bit addressing was added in July 2013. Earlier version do not have it.                     
>>>>>>>>> This feature was enhanced in release August 2014 to have separate upper address values for transmit and receive."                                                                                       
>>>>>>>>>                                                                                                
>>>>>>>>>> /* Bitfields in NSR */                                                                        
>>>>>>>>>> @@ -474,6 +479,10 @@                                                                          
>>>>>>>>>>  struct macb_dma_desc {                                                                       
>>>>>>>>>  >      u32     addr;                                                                          
>>>>>>>>>>       u32     ctrl;                                                                           
>>>>>>>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT                                                          
>>>>>>>>>> +     u32     addrh;                                                                          
>>>>>>>>>> +     u32     resvd;                                                                          
>>>>>>>>>> +#endif                                                                                       
>>>>>>>>>>  };                                                                                           
>>>>>>>>>                                                                                                
>>>>>>>>> It will not work for legacy hardware. Old descriptor is 2 words wide, the new one is 4 words wide.                                                                                                      
>>>>>>>>> If you enable CONFIG_ARCH_DMA_ADDR_T_64BIT but hardware doesn't                                
>>>>>>>>> support it at all, you will miss every second descriptor.                                      
>>>>>>>>>                                                                                                
>>>>>>>>                                                                                                 
>>>>>>>> True, this feature is not available in all of Cadence IP versions.                              
>>>>>>>> In fact, the IP version Zynq does not support this. But the one in ZynqMP does.                 
>>>>>>>> So, we enable kernel config for 64 bit DMA addressing for this                                  
>>>>>>>> SoC and hence the driver picks it up. My assumption was that if                                 
>>>>>>>> the legacy IP does not support                                                                  
>>>>>>>> 64 bit addressing, then this DMA option wouldn't be enabled.                                    
>>>>>>>>                                                                                                 
>>>>>>>> There is a design config register in Cadence IP which is being                                  
>>>>>>>> read to check for 64 bit address support - DMA mask is set based on that.                       
>>>>>>>> But the addition of two descriptor words cannot be based on this runtime check.                 
>>>>>>>> For this reason, all the static changes were placed under this check.                           
>>>>>>>                                                                                                  
>>>>>>> We have quite a bunch of options in this driver to determinate what is the real capacity of the underlying hardware.                                                                                      
>>>>>>> If HW configuration registers are not appropriate, and it seems they are not, I would advice to simply use the DT compatibility string.                                                                   
>>>>>>>                                                                                                  
>>>>>>> Best regards,                                                                                    
>>>>>>> --                                                                                               
>>>>>>> Nicolas Ferre                                                                                    
>>>>>>                                                                                                   
>>>>>> HW configuration registers are appropriate. The issue is that this code doesn’t use the capability bit to switch between different dma descriptors (2 words vs. 4 words).                                  
>>>>>> DMA descriptor size is chosen based on kernel configuration, not based on hardware capabilities.  
>>>>>                                                                                                    
>>>>> HW configuration register does give appropriate information.                                       
>>>>> But addition of two address words in the macb descriptor structure is a static change.             
>>>>>                                                                                                    
>>>>> +static inline void macb_set_addr(struct macb_dma_desc *desc,                                      
>>>>> +dma_addr_t                                                                                        
>>>>> +addr) {                                                                                           
>>>>> +       desc->addr = (u32)addr;                                                                    
>>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT                                                               
>>>>> +       desc->addrh = (u32)(addr >> 32); #endif                                                    
>>>>> +                                                                                                  
>>>>>                                                                                                    
>>>>> Even if the #ifdef condition here is changed to HW config check, addr and addrh are different.     
>>>>> And "addrh" entry has to be present for 64 bit desc case to be handled separately.                 
>>>>> Can you please tell me how you propose change in DMA descriptor                                    
>>>>> structure from                                                                                     
>>>>> 4 to 2 or 2 to 4 words *after* reading the DCFG register?                                          
>>>>                                                                                                     
>>>> It is very complex problem. I wrote to you because I faced the same issue. I'm working on PTP implementation for macb. When PTP is enabled there are additional two words in buffer descriptor.              
>>>
>>>Moreover, we can use PTP without the 64bits descriptor support (early GEM revisions).
>>>
>>>BTW, note that there is an initiative ongoing with Andrei to support
>>>PTP/1588 on these devices with patches already send: please synchronize with him.
>>>https://urldefense.proofpoint.com/v2/url?u=https-3A__marc.info_-3Fl-3D
>>>linux-2Dnetdev-26m-3D147282092309112-26w-3D3&d=DgIDaQ&c=aUq983L2pue2Fq
>>>KFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtC
>>>g4L5E&m=jUmZzqwn1uRrvcqF3BHCHFC6ZsKoZkU3vT8gJ-7fnsE&s=kr_km0MKQBzpkaOt
>>>0fkM-FIajN1-pOylzzTjsXi-vak&e=
>>>or
>>>https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.
>>>org_patch_9310989_&d=DgIDaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_ha
>>>XqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtCg4L5E&m=jUmZzqwn1uRrvcqF3B
>>>HCHFC6ZsKoZkU3vT8gJ-7fnsE&s=ZbXVD5Lb5zGn7wUKIPYHxagIEKp_vJvrnkRa4qJfmT
>>>Y&e=
>>>and
>>>https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.
>>>org_patch_9310991_&d=DgIDaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_ha
>>>XqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtCg4L5E&m=jUmZzqwn1uRrvcqF3B
>>>HCHFC6ZsKoZkU3vT8gJ-7fnsE&s=Z0kRqUqh5Is4TmuaY7A4hnpizfdeq3HrgPhyDAMLuA
>>>8&e=
>>>
>>>Regards,
>>>--
>>>Nicolas Ferre
>>
>> Above patch doesn’t use hardware timestamps in descriptors at all. It doesn't use appropriate accuracy.
>> We have our PTP patch ready, which use timestamps from descriptor. But we have not sent it yet because of compatibility issue.
>> Of course, we can do it like Harini did:
>>
>> struct macb_dma_desc {
>>         u32     addr;
>>         u32     ctrl;
>> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>         u32     addrh;
>>         u32     resvd;
>> #endif
>> #if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
>>         u32     dma_desc_ts_1;
>>         u32     dma_desc_ts_2;
>> #endif
>> };
>>
>> But in that approach we lose backward compatibility.
>>
>> What are your suggestion? Can we send patch like it is or wait for some common solution with backward compatibility?
>>
>
>Yes, Andre's version of Cadence does not ability to report have RX timestamp.
>The version I worked with did. This is the old series I sent:
>https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_2015_9_11_92&d=DgIFaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtCg4L5E&m=1A1qxdlwY3kMlJ1JUJJL1EqLzWz4AKtfOxZf_vu8bWo&s=NGpEbbC4bYXUZjJ6n0Ud8F8p3fPz5EhTJ1O9-NKwCkQ&e=
>But right now, i'm working on building on top of Andre's changes.
>
>Regards,
>Harini
>
I can’t see a place where you enable extended descriptor for PTP. Did you add support for extended PTP descriptor?
"DMA Configuration Register" 0x010:
29	tx_bd_extended_mode_en	"Enable TX extended BD mode. See TX BD control register definition for description of feature."
28	rx_bd_extended_mode_en	"Enable RX extended BD mode. See RX BD control register definition for description of feature."

Can I send you our patch for comparison ? 

^ permalink raw reply

* Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM
From: Harini Katakam @ 2016-11-18  9:58 UTC (permalink / raw)
  To: Rafal Ozieblo
  Cc: Nicolas Ferre, harini.katakam@xilinx.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <BN3PR07MB25166D8301C60C4A35402AB1C9B00@BN3PR07MB2516.namprd07.prod.outlook.com>

Hi Rafal,

On Fri, Nov 18, 2016 at 2:29 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:
> Hello,
>
>> From: Harini Katakam [mailto:harinikatakamlinux@gmail.com]
>> Sent: 18 listopada 2016 05:30
>> To: Rafal Ozieblo
>> Cc: Nicolas Ferre; harini.katakam@xilinx.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM
>>
>> Hi Rafal,
>>
>> On Thu, Nov 17, 2016 at 7:05 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:
>> > -----Original Message-----
>> > From: Nicolas Ferre [mailto:nicolas.ferre@atmel.com]
>> > Sent: 17 listopada 2016 14:29
>> > To: Harini Katakam; Rafal Ozieblo
>> > Cc: harini.katakam@xilinx.com; netdev@vger.kernel.org;
>> > linux-kernel@vger.kernel.org
>> > Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support
>> > for GEM
>> >
>> >> Le 17/11/2016 à 13:21, Harini Katakam a écrit :
>> >> > Hi Rafal,
>> >> >
>> >> > On Thu, Nov 17, 2016 at 5:20 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:
>> >> >> Hello,
>> >> >> I think, there could a bug in your patch.
>> >> >>
>> >> >>> +
>> >> >>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>> >> >>> +             dmacfg |= GEM_BIT(ADDR64); #endif
>> >> >>
>> >> >> You enable 64 bit addressing (64b dma bus width) always when appropriate architecture config option is enabled.
>> >> >> But there are some legacy controllers which do not support that feature. According Cadence hardware team:
>> >> >> "64 bit addressing was added in July 2013. Earlier version do not have it.
>> >> >> This feature was enhanced in release August 2014 to have separate upper address values for transmit and receive."
>> >> >>
>> >> >>> /* Bitfields in NSR */
>> >> >>> @@ -474,6 +479,10 @@
>> >> >>>  struct macb_dma_desc {
>> >> >>  >      u32     addr;
>> >> >>>       u32     ctrl;
>> >> >>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>> >> >>> +     u32     addrh;
>> >> >>> +     u32     resvd;
>> >> >>> +#endif
>> >> >>>  };
>> >> >>
>> >> >> It will not work for legacy hardware. Old descriptor is 2 words wide, the new one is 4 words wide.
>> >> >> If you enable CONFIG_ARCH_DMA_ADDR_T_64BIT but hardware doesn't
>> >> >> support it at all, you will miss every second descriptor.
>> >> >>
>> >> >
>> >> > True, this feature is not available in all of Cadence IP versions.
>> >> > In fact, the IP version Zynq does not support this. But the one in ZynqMP does.
>> >> > So, we enable kernel config for 64 bit DMA addressing for this SoC
>> >> > and hence the driver picks it up. My assumption was that if the
>> >> > legacy IP does not support
>> >> > 64 bit addressing, then this DMA option wouldn't be enabled.
>> >> >
>> >> > There is a design config register in Cadence IP which is being read
>> >> > to check for 64 bit address support - DMA mask is set based on that.
>> >> > But the addition of two descriptor words cannot be based on this runtime check.
>> >> > For this reason, all the static changes were placed under this check.
>> >>
>> >> We have quite a bunch of options in this driver to determinate what is the real capacity of the underlying hardware.
>> >> If HW configuration registers are not appropriate, and it seems they are not, I would advice to simply use the DT compatibility string.
>> >>
>> >> Best regards,
>> >> --
>> >> Nicolas Ferre
>> >
>> > HW configuration registers are appropriate. The issue is that this code doesn’t use the capability bit to switch between different dma descriptors (2 words vs. 4 words).
>> > DMA descriptor size is chosen based on kernel configuration, not based on hardware capabilities.
>>
>> HW configuration register does give appropriate information.
>> But addition of two address words in the macb descriptor structure is a static change.
>>
>> +static inline void macb_set_addr(struct macb_dma_desc *desc, dma_addr_t
>> +addr) {
>> +       desc->addr = (u32)addr;
>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>> +       desc->addrh = (u32)(addr >> 32); #endif
>> +
>>
>> Even if the #ifdef condition here is changed to HW config check, addr and addrh are different.
>> And "addrh" entry has to be present for 64 bit desc case to be handled separately.
>> Can you please tell me how you propose change in DMA descriptor structure from
>> 4 to 2 or 2 to 4 words *after* reading the DCFG register?
>
> It is very complex problem. I wrote to you because I faced the same issue. I'm working on PTP implementation for macb. When PTP is enabled there are additional two words in buffer descriptor.
> But hardware might not be compatible with PTP. Therefore I have to change DMA descriptor between 2 and 4 words after reading DCFG register (the same as you between 32b and 64b).
> When we consider both PTP and 64 bits, we end up with 4 different descriptors!

Agree

<snip>
> (Below is kind of pseudo code only to show my idea. All defines like 64B or PTP are omitted for simplicity)
>
> 1. Prepare appropriate structures:
>
> struct macb_dma_desc {
>         u32     addr;
>         u32     ctrl;
> }
>
> struct macb_dma_desc_64 {
>         u32 addrh;
>         u32 resvd;
> }
>
> struct macb_dma_desc_ptp {
>         u32 dma_desc_ts_1;
>         u32     dma_desc_ts_2;
> }
>
> 2. Add hardware support information to macb:
>
> enum macb_hw_cap {
>         HW_CAP_NONE,
>         HW_CAP_64B,
>         HW_CAP_PTP,
>         HW_CAP_64B_PTP,
> };
>
> struct macb {
> // (...)
>         macb_hw_cap hw_cap;
>
> }
>
> 3. Set bp->hw_cap on macb_probe()
>

hw_cap can alreayd be obtained from config structures
based on compatible string.

> 4. Additional function might be helpful:
>
> static unsigned char macb_dma_desc_get_mul(struct macb *bp)
> {
>         switch (bp->hw_cap) {
>                 case HW_CAP_NONE:
>                         return 1;
>                 case HW_CAP_64B:
>                 case HW_CAP_PTP:
>                         return 2;
>                 case HW_CAP_64B_PTP:
>                         return 3;
>         }
> }
>
> 5. Change sizeof struct to function:
> (sizeof(struct macb_dma_desc) change to macb_dma_desc_get_size(bp). It will return sizeof(struct macb_dma_desc) * macb_dma_desc_get_mul(bp).
> There is a hidden assumption that all three structures have the same size.
>
> 6. macb_rx_desc() and macb_tx_desc() will still return struct macb_dma_desc * but descriptor index will be counted in different way, ex.
>
> static struct macb_dma_desc *macb_rx_desc(struct macb *bp, unsigned int index)
> {
>         index *= macb_dma_desc_get_mul(bp);
>         return &bp->rx_ring[macb_rx_ring_wrap(bp, index)];
> }
>
> 7. Two additional functions to dereference PTP and 64b information:
>
> static struct macb_dma_desc_64 *macb_64b_desc(struct macb *bp, struct macb_dma_desc *desc)
> {
>         switch (bp->hw_cap) {
>                 case HW_CAP_64B:
>                 case HW_CAP_64B_PTP:
>                         return (struct macb_dma_desc_64 *)(desc + 1);  // ugly casting
>                 default:
>                         return NULL;
>         }
> }
>
> static struct macb_dma_desc_ptp *macb_ptp_desc(struct macb *bp, struct macb_dma_desc *desc)
> {
>         switch (bp->hw_cap) {
>                 case HW_CAP_PTP:
>                         return (struct macb_dma_desc_ptp *)(desc + 1);
>                 case HW_CAP_64B_PTP:
>                         return (struct macb_dma_desc_ptp *)(desc + 2);
>                 default:
>                         return NULL;
>         }
> }
>
> Whenever you want to reach fields in appropriate descriptor, above function should be used.
>

Theoretically I agree this will work.
But we'll have to try to see how this will affect/slow down
the desc reading.. especially with PTP.

Regards,
Harini

> This is only my very first idea. Of course, we can leave it as it is and say, that old hardware is not support either when PTP enabled or 64b enabled.

^ permalink raw reply

* Re: [PATCH net-next] bridge: add igmpv3 and mldv2 query support
From: Nikolay Aleksandrov @ 2016-11-18 10:09 UTC (permalink / raw)
  To: Hangbin Liu, netdev; +Cc: Hannes Frederic Sowa, linus.luessing, roopa
In-Reply-To: <9c1e16c2-b716-21e0-fa2c-3e3126607953@cumulusnetworks.com>

On 18/11/16 11:04, Nikolay Aleksandrov wrote:
> On 18/11/16 08:32, Hangbin Liu wrote:
>> Add bridge IGMPv3 and MLDv2 query support. But before we think it is stable
>> enough, only enable it when declare in force_igmp/mld_version.
>>
>> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
>> ---
>>  net/bridge/br_multicast.c | 203 ++++++++++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 194 insertions(+), 9 deletions(-)
>>
>
> (+CC Roopa)
> Hi Hangbin,
> This patch is not correct, you should not use the net device IGMP config in the bridge.
* bridge net device, sorry not port

> These packets may never make it to the host and they may not be reflected, even more the
> host may have very different igmp config for the port net device than the bridge does.
* again bridge net device, not port

> Also your current implementation switches to V3 only if it is globally set, and that should
> be controlled on a per-bridge basis, even per-vlan later.
* it is per-bridge, you use the global net device IGMP config, but it should be in the bridge
mcast control options, so we can do it per-vlan later and also be able to do the compat parts
of the RFC and in general, the bridge is configured via its own options not the host net device
because as I said these packets may never be received locally

> One more thing, if someone does a V2 leave for a group, you can end up sending them V3
> group-specific query.
>
> I have been working on an implementation for IGMPv3/MLDv2 querier for the bridge device, it re-uses
> most of the current code and allows you to configure it per-bridge (and later per-vlan). The only
> reason I've not yet sent it to upstream is that we (at Cumulus) are working out the compatibility
> parts (e.g. RFC3376 sec 7, sec 8) of the RFC since it has some unclear cases.
> But I can rip the compatibility out and send it early for review if you'd like, we can add the
> compat code later.
>
> Cheers,
>  Nik
>
>> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
>> index 2136e45..9fb47f3 100644
>> --- a/net/bridge/br_multicast.c
>> +++ b/net/bridge/br_multicast.c
>> @@ -35,6 +35,10 @@
>>
>>  #include "br_private.h"
>>
>> +#define IGMP_V3_SEEN(in_dev) \
>> +    (IPV4_DEVCONF_ALL(dev_net(in_dev->dev), FORCE_IGMP_VERSION) == 3 || \
>> +     IN_DEV_CONF_GET((in_dev), FORCE_IGMP_VERSION) == 3)
>> +
>>  static void br_multicast_start_querier(struct net_bridge *br,
>>                         struct bridge_mcast_own_query *query);
>>  static void br_multicast_add_router(struct net_bridge *br,
>> @@ -360,9 +364,8 @@ static int br_mdb_rehash(struct net_bridge_mdb_htable __rcu **mdbp, int max,
>>      return 0;
>>  }
>>
>> -static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge *br,
>> -                            __be32 group,
>> -                            u8 *igmp_type)
>> +static struct sk_buff *br_ip4_alloc_query_v2(struct net_bridge *br,
>> +                         __be32 group, u8 *igmp_type)
>>  {
>>      struct sk_buff *skb;
>>      struct igmphdr *ih;
>> @@ -428,10 +431,82 @@ static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge *br,
>>      return skb;
>>  }
>>
>> +static struct sk_buff *br_ip4_alloc_query_v3(struct net_bridge *br,
>> +                         __be32 group, u8 *igmp_type)
>> +{
>> +    struct sk_buff *skb;
>> +    struct igmpv3_query *ih3;
>> +    struct ethhdr *eth;
>> +    struct iphdr *iph;
>> +
>> +    skb = netdev_alloc_skb_ip_align(br->dev, sizeof(*eth) + sizeof(*iph) +
>> +                         sizeof(*ih3) + 4);
>> +    if (!skb)
>> +        goto out;
>> +
>> +    skb->protocol = htons(ETH_P_IP);
>> +
>> +    skb_reset_mac_header(skb);
>> +    eth = eth_hdr(skb);
>> +
>> +    ether_addr_copy(eth->h_source, br->dev->dev_addr);
>> +    eth->h_dest[0] = 1;
>> +    eth->h_dest[1] = 0;
>> +    eth->h_dest[2] = 0x5e;
>> +    eth->h_dest[3] = 0;
>> +    eth->h_dest[4] = 0;
>> +    eth->h_dest[5] = 1;
>> +    eth->h_proto = htons(ETH_P_IP);
>> +    skb_put(skb, sizeof(*eth));
>> +
>> +    skb_set_network_header(skb, skb->len);
>> +    iph = ip_hdr(skb);
>> +
>> +    iph->version = 4;
>> +    iph->ihl = 6;
>> +    iph->tos = 0xc0;
>> +    iph->tot_len = htons(sizeof(*iph) + sizeof(*ih3) + 4);
>> +    iph->id = 0;
>> +    iph->frag_off = htons(IP_DF);
>> +    iph->ttl = 1;
>> +    iph->protocol = IPPROTO_IGMP;
>> +    iph->saddr = br->multicast_query_use_ifaddr ?
>> +             inet_select_addr(br->dev, 0, RT_SCOPE_LINK) : 0;
>> +    iph->daddr = htonl(INADDR_ALLHOSTS_GROUP);
>> +    ((u8 *)&iph[1])[0] = IPOPT_RA;
>> +    ((u8 *)&iph[1])[1] = 4;
>> +    ((u8 *)&iph[1])[2] = 0;
>> +    ((u8 *)&iph[1])[3] = 0;
>> +    ip_send_check(iph);
>> +    skb_put(skb, 24);
>> +
>> +    skb_set_transport_header(skb, skb->len);
>> +    ih3 = igmpv3_query_hdr(skb);
>> +
>> +    *igmp_type = IGMP_HOST_MEMBERSHIP_QUERY;
>> +    ih3->type = IGMP_HOST_MEMBERSHIP_QUERY;
>> +    ih3->code = (group ? br->multicast_last_member_interval :
>> +                br->multicast_query_response_interval) /
>> +           (HZ / IGMP_TIMER_SCALE);
>> +    ih3->csum = 0;
>> +    ih3->group = group;
>> +    ih3->resv = 0;
>> +    ih3->suppress = 0;
>> +    ih3->qrv= 2;
>> +    ih3->qqic = br->multicast_query_interval / HZ;
>> +    ih3->nsrcs = 0;
>> +    ih3->csum = ip_compute_csum((void *)ih3, sizeof(struct igmpv3_query ));
>> +    skb_put(skb, sizeof(*ih3));
>> +
>> +    __skb_pull(skb, sizeof(*eth));
>> +
>> +out:
>> +    return skb;
>> +}
>>  #if IS_ENABLED(CONFIG_IPV6)
>> -static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge *br,
>> -                            const struct in6_addr *grp,
>> -                            u8 *igmp_type)
>> +static struct sk_buff *br_ip6_alloc_query_v1(struct net_bridge *br,
>> +                         const struct in6_addr *grp,
>> +                         u8 *igmp_type)
>>  {
>>      struct sk_buff *skb;
>>      struct ipv6hdr *ip6h;
>> @@ -514,19 +589,129 @@ static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge *br,
>>  out:
>>      return skb;
>>  }
>> +
>> +static struct sk_buff *br_ip6_alloc_query_v2(struct net_bridge *br,
>> +                         const struct in6_addr *grp,
>> +                         u8 *igmp_type)
>> +{
>> +    struct sk_buff *skb;
>> +    struct ipv6hdr *ip6h;
>> +    struct mld2_query *mld2q;
>> +    struct ethhdr *eth;
>> +    u8 *hopopt;
>> +    unsigned long interval;
>> +
>> +    skb = netdev_alloc_skb_ip_align(br->dev, sizeof(*eth) + sizeof(*ip6h) +
>> +                         8 + sizeof(*mld2q));
>> +    if (!skb)
>> +        goto out;
>> +
>> +    skb->protocol = htons(ETH_P_IPV6);
>> +
>> +    /* Ethernet header */
>> +    skb_reset_mac_header(skb);
>> +    eth = eth_hdr(skb);
>> +
>> +    ether_addr_copy(eth->h_source, br->dev->dev_addr);
>> +    eth->h_proto = htons(ETH_P_IPV6);
>> +    skb_put(skb, sizeof(*eth));
>> +
>> +    /* IPv6 header + HbH option */
>> +    skb_set_network_header(skb, skb->len);
>> +    ip6h = ipv6_hdr(skb);
>> +
>> +    *(__force __be32 *)ip6h = htonl(0x60000000);
>> +    ip6h->payload_len = htons(8 + sizeof(*mld2q));
>> +    ip6h->nexthdr = IPPROTO_HOPOPTS;
>> +    ip6h->hop_limit = 1;
>> +    ipv6_addr_set(&ip6h->daddr, htonl(0xff020000), 0, 0, htonl(1));
>> +    if (ipv6_dev_get_saddr(dev_net(br->dev), br->dev, &ip6h->daddr, 0,
>> +                   &ip6h->saddr)) {
>> +        kfree_skb(skb);
>> +        br->has_ipv6_addr = 0;
>> +        return NULL;
>> +    }
>> +
>> +    br->has_ipv6_addr = 1;
>> +    ipv6_eth_mc_map(&ip6h->daddr, eth->h_dest);
>> +
>> +    hopopt = (u8 *)(ip6h + 1);
>> +    hopopt[0] = IPPROTO_ICMPV6;        /* next hdr */
>> +    hopopt[1] = 0;                /* length of HbH */
>> +    hopopt[2] = IPV6_TLV_ROUTERALERT;    /* Router Alert */
>> +    hopopt[3] = 2;                /* Length of RA Option */
>> +    hopopt[4] = 0;                /* Type = 0x0000 (MLD) */
>> +    hopopt[5] = 0;
>> +    hopopt[6] = IPV6_TLV_PAD1;        /* Pad1 */
>> +    hopopt[7] = IPV6_TLV_PAD1;        /* Pad1 */
>> +
>> +    skb_put(skb, sizeof(*ip6h) + 8);
>> +
>> +    /* ICMPv6 */
>> +    skb_set_transport_header(skb, skb->len);
>> +    mld2q = (struct mld2_query *) icmp6_hdr(skb);
>> +
>> +    interval = ipv6_addr_any(grp) ?
>> +            br->multicast_query_response_interval :
>> +            br->multicast_last_member_interval;
>> +
>> +    *igmp_type = ICMPV6_MGM_QUERY;
>> +    mld2q->mld2q_type = ICMPV6_MGM_QUERY;
>> +    mld2q->mld2q_code = 0;
>> +    mld2q->mld2q_cksum = 0;
>> +    mld2q->mld2q_mrc = htons((u16)jiffies_to_msecs(interval));
>> +    mld2q->mld2q_resv1 = 0;
>> +    mld2q->mld2q_mca = *grp;
>> +    mld2q->mld2q_resv2 = 0;
>> +    mld2q->mld2q_suppress = 0;
>> +    mld2q->mld2q_qrv = 2;
>> +    mld2q->mld2q_qqic = br->multicast_query_interval / HZ;
>> +    mld2q->mld2q_nsrcs = 0;
>> +
>> +    /* checksum */
>> +    mld2q->mld2q_cksum = csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr,
>> +                      sizeof(*mld2q), IPPROTO_ICMPV6,
>> +                      csum_partial(mld2q,
>> +                               sizeof(*mld2q), 0));
>> +    skb_put(skb, sizeof(*mld2q));
>> +
>> +    __skb_pull(skb, sizeof(*eth));
>> +
>> +out:
>> +    return skb;
>> +}
>>  #endif
>>
>> +static int mld_force_mld_version(const struct inet6_dev *idev)
>> +{
>> +    if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version != 0)
>> +        return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version;
>> +    else
>> +        return idev->cnf.force_mld_version;
>> +}
>> +
>>  static struct sk_buff *br_multicast_alloc_query(struct net_bridge *br,
>>                          struct br_ip *addr,
>>                          u8 *igmp_type)
>>  {
>> +    struct in_device *in_dev = __in_dev_get_rcu(br->dev);
>> +    struct inet6_dev *idev = __in6_dev_get(br->dev);
>>      switch (addr->proto) {
>>      case htons(ETH_P_IP):
>> -        return br_ip4_multicast_alloc_query(br, addr->u.ip4, igmp_type);
>> +        if (IGMP_V3_SEEN(in_dev))
>> +            return br_ip4_alloc_query_v3(br, addr->u.ip4,
>> +                             igmp_type);
>> +        else
>> +            return br_ip4_alloc_query_v2(br, addr->u.ip4,
>> +                             igmp_type);
>>  #if IS_ENABLED(CONFIG_IPV6)
>>      case htons(ETH_P_IPV6):
>> -        return br_ip6_multicast_alloc_query(br, &addr->u.ip6,
>> -                            igmp_type);
>> +        if (mld_force_mld_version(idev) == 2)
>> +            return br_ip6_alloc_query_v2(br, &addr->u.ip6,
>> +                             igmp_type);
>> +        else
>> +            return br_ip6_alloc_query_v1(br, &addr->u.ip6,
>> +                             igmp_type);
>>  #endif
>>      }
>>      return NULL;
>>
>

^ permalink raw reply

* Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM
From: Andrei Pistirica @ 2016-11-18 10:07 UTC (permalink / raw)
  To: Harini Katakam
  Cc: Rafal Ozieblo, Nicolas Ferre, harini.katakam@xilinx.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAFcVECJT=b2jmQAtsTeXEwWad8L2VEVW1GOyTY9XYR5qUq=4Mw@mail.gmail.com>

Hi Harini,

On 18.11.2016 10:43, Harini Katakam wrote:
> Hi Rafal,
>
> On Fri, Nov 18, 2016 at 3:00 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:
>>> -----Original Message-----
>>> From: Nicolas Ferre [mailto:nicolas.ferre@atmel.com]
>>> Sent: 18 listopada 2016 10:10
>>> To: Rafal Ozieblo; Harini Katakam; Andrei Pistirica
>>> Cc: harini.katakam@xilinx.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
>>> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM
>>>
>>> Le 18/11/2016 à 09:59, Rafal Ozieblo a écrit :
>>>> Hello,
>>>>
>>>>> From: Harini Katakam [mailto:harinikatakamlinux@gmail.com]
>>>>> Sent: 18 listopada 2016 05:30
>>>>> To: Rafal Ozieblo
>>>>> Cc: Nicolas Ferre; harini.katakam@xilinx.com; netdev@vger.kernel.org;
>>>>> linux-kernel@vger.kernel.org
>>>>> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support
>>>>> for GEM
>>>>>
>>>>> Hi Rafal,
>>>>>
>>>>> On Thu, Nov 17, 2016 at 7:05 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:
>>>>>> -----Original Message-----
>>>>>> From: Nicolas Ferre [mailto:nicolas.ferre@atmel.com]
>>>>>> Sent: 17 listopada 2016 14:29
>>>>>> To: Harini Katakam; Rafal Ozieblo
>>>>>> Cc: harini.katakam@xilinx.com; netdev@vger.kernel.org;
>>>>>> linux-kernel@vger.kernel.org
>>>>>> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing
>>>>>> support for GEM
>>>>>>
>>>>>>> Le 17/11/2016 à 13:21, Harini Katakam a écrit :
>>>>>>>> Hi Rafal,
>>>>>>>>
>>>>>>>> On Thu, Nov 17, 2016 at 5:20 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:
>>>>>>>>> Hello,
>>>>>>>>> I think, there could a bug in your patch.
>>>>>>>>>
>>>>>>>>>> +
>>>>>>>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>>>>>>>>> +             dmacfg |= GEM_BIT(ADDR64); #endif
>>>>>>>>>
>>>>>>>>> You enable 64 bit addressing (64b dma bus width) always when appropriate architecture config option is enabled.
>>>>>>>>> But there are some legacy controllers which do not support that feature. According Cadence hardware team:
>>>>>>>>> "64 bit addressing was added in July 2013. Earlier version do not have it.
>>>>>>>>> This feature was enhanced in release August 2014 to have separate upper address values for transmit and receive."
>>>>>>>>>
>>>>>>>>>> /* Bitfields in NSR */
>>>>>>>>>> @@ -474,6 +479,10 @@
>>>>>>>>>>  struct macb_dma_desc {
>>>>>>>>>  >      u32     addr;
>>>>>>>>>>       u32     ctrl;
>>>>>>>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>>>>>>>>> +     u32     addrh;
>>>>>>>>>> +     u32     resvd;
>>>>>>>>>> +#endif
>>>>>>>>>>  };
>>>>>>>>>
>>>>>>>>> It will not work for legacy hardware. Old descriptor is 2 words wide, the new one is 4 words wide.
>>>>>>>>> If you enable CONFIG_ARCH_DMA_ADDR_T_64BIT but hardware doesn't
>>>>>>>>> support it at all, you will miss every second descriptor.
>>>>>>>>>
>>>>>>>>
>>>>>>>> True, this feature is not available in all of Cadence IP versions.
>>>>>>>> In fact, the IP version Zynq does not support this. But the one in ZynqMP does.
>>>>>>>> So, we enable kernel config for 64 bit DMA addressing for this SoC
>>>>>>>> and hence the driver picks it up. My assumption was that if the
>>>>>>>> legacy IP does not support
>>>>>>>> 64 bit addressing, then this DMA option wouldn't be enabled.
>>>>>>>>
>>>>>>>> There is a design config register in Cadence IP which is being
>>>>>>>> read to check for 64 bit address support - DMA mask is set based on that.
>>>>>>>> But the addition of two descriptor words cannot be based on this runtime check.
>>>>>>>> For this reason, all the static changes were placed under this check.
>>>>>>>
>>>>>>> We have quite a bunch of options in this driver to determinate what is the real capacity of the underlying hardware.
>>>>>>> If HW configuration registers are not appropriate, and it seems they are not, I would advice to simply use the DT compatibility string.
>>>>>>>
>>>>>>> Best regards,
>>>>>>> --
>>>>>>> Nicolas Ferre
>>>>>>
>>>>>> HW configuration registers are appropriate. The issue is that this code doesn’t use the capability bit to switch between different dma descriptors (2 words vs. 4 words).
>>>>>> DMA descriptor size is chosen based on kernel configuration, not based on hardware capabilities.
>>>>>
>>>>> HW configuration register does give appropriate information.
>>>>> But addition of two address words in the macb descriptor structure is a static change.
>>>>>
>>>>> +static inline void macb_set_addr(struct macb_dma_desc *desc,
>>>>> +dma_addr_t
>>>>> +addr) {
>>>>> +       desc->addr = (u32)addr;
>>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>>>> +       desc->addrh = (u32)(addr >> 32); #endif
>>>>> +
>>>>>
>>>>> Even if the #ifdef condition here is changed to HW config check, addr and addrh are different.
>>>>> And "addrh" entry has to be present for 64 bit desc case to be handled separately.
>>>>> Can you please tell me how you propose change in DMA descriptor
>>>>> structure from
>>>>> 4 to 2 or 2 to 4 words *after* reading the DCFG register?
>>>>
>>>> It is very complex problem. I wrote to you because I faced the same issue. I'm working on PTP implementation for macb. When PTP is enabled there are additional two words in buffer descriptor.
>>>
>>> Moreover, we can use PTP without the 64bits descriptor support (early GEM revisions).
>>>
>>> BTW, note that there is an initiative ongoing with Andrei to support
>>> PTP/1588 on these devices with patches already send: please synchronize with him.
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__marc.info_-3Fl-3Dlinux-2Dnetdev-26m-3D147282092309112-26w-3D3&d=DgIDaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtCg4L5E&m=jUmZzqwn1uRrvcqF3BHCHFC6ZsKoZkU3vT8gJ-7fnsE&s=kr_km0MKQBzpkaOt0fkM-FIajN1-pOylzzTjsXi-vak&e=
>>> or
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.org_patch_9310989_&d=DgIDaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtCg4L5E&m=jUmZzqwn1uRrvcqF3BHCHFC6ZsKoZkU3vT8gJ-7fnsE&s=ZbXVD5Lb5zGn7wUKIPYHxagIEKp_vJvrnkRa4qJfmTY&e=
>>> and
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.org_patch_9310991_&d=DgIDaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtCg4L5E&m=jUmZzqwn1uRrvcqF3BHCHFC6ZsKoZkU3vT8gJ-7fnsE&s=Z0kRqUqh5Is4TmuaY7A4hnpizfdeq3HrgPhyDAMLuA8&e=
>>>
>>> Regards,
>>> --
>>> Nicolas Ferre
>>
>> Above patch doesn’t use hardware timestamps in descriptors at all. It doesn't use appropriate accuracy.
>> We have our PTP patch ready, which use timestamps from descriptor. But we have not sent it yet because of compatibility issue.
>> Of course, we can do it like Harini did:
>>
>> struct macb_dma_desc {
>>         u32     addr;
>>         u32     ctrl;
>> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>         u32     addrh;
>>         u32     resvd;
>> #endif
>> #if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
>>         u32     dma_desc_ts_1;
>>         u32     dma_desc_ts_2;
>> #endif
>> };
>>
>> But in that approach we lose backward compatibility.
>>
>> What are your suggestion? Can we send patch like it is or wait for some common solution with backward compatibility?
>>
>
> Yes, Andre's version of Cadence does not ability to report have RX timestamp.
> The version I worked with did. This is the old series I sent:
> https://lkml.org/lkml/2015/9/11/92
> But right now, i'm working on building on top of Andre's changes.

I'm addressing maintainer's feedback on both patches:
https://patchwork.kernel.org/patch/9310989/
https://patchwork.kernel.org/patch/9310991/

I've done all suggested updates, except the numerous 64bit divisions in 
the frequency adjustment callback. I've implemented a different 
algorithm which uses 2 64bit division, but I couldn't find a way to use 
only 1.

Also, I have a version with timecounter/cyclecounter which shows a much 
better accuracy (less than 100ns level). In my opinion this could be a 
better implementation. What is you opinion about this? Did you try it?

Tuesday I can send an updated version of these patches.

>
> Regards,
> Harini
>

Regards,
Andrei

^ permalink raw reply

* Re: [PATCH v9 0/8] thunderbolt: Introducing Thunderbolt(TM) Networking
From: gregkh @ 2016-11-18 10:07 UTC (permalink / raw)
  To: Levy, Amir (Jer)
  Cc: Simon Guinot, andreas.noever@gmail.com, bhelgaas@google.com,
	corbet@lwn.net, linux-kernel@vger.kernel.org,
	linux-pci@vger.kernel.org, netdev@vger.kernel.org,
	linux-doc@vger.kernel.org, mario_limonciello@dell.com,
	thunderbolt-linux, Westerberg, Mika, Winkler, Tomas,
	Zhang, Xiong Y, Jamet, Michael, remi.rerolle@seagate.com
In-Reply-To: <E607265CB020454880711A6F96C05A03BE2199EE@hasmsx108.ger.corp.intel.com>

On Fri, Nov 18, 2016 at 08:48:36AM +0000, Levy, Amir (Jer) wrote:
> > BTW, it is quite a shame that the Thunderbolt firmware version can't 
> > be read from Linux.
> > 
> 
> This is WIP, once this patch will be upstream, we will be able to focus more
> on aligning Linux with the Thunderbolt features that we have for windows.

Why is this patch somehow holding that work back?  You aren't just
sitting around waiting for people to review this and not doing anything
else, right?  Is there some basic building block in these patches that
your firmware download code is going to rely on?

confused,

greg k-h

^ permalink raw reply

* Re: [PATCH net-next] bridge: add igmpv3 and mldv2 query support
From: Nikolay Aleksandrov @ 2016-11-18 10:04 UTC (permalink / raw)
  To: Hangbin Liu, netdev; +Cc: Hannes Frederic Sowa, linus.luessing, roopa
In-Reply-To: <1479454321-31304-1-git-send-email-liuhangbin@gmail.com>

On 18/11/16 08:32, Hangbin Liu wrote:
> Add bridge IGMPv3 and MLDv2 query support. But before we think it is stable
> enough, only enable it when declare in force_igmp/mld_version.
>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  net/bridge/br_multicast.c | 203 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 194 insertions(+), 9 deletions(-)
>

(+CC Roopa)
Hi Hangbin,
This patch is not correct, you should not use the port net device IGMP config in the bridge.
These packets may never make it to the host and they may not be reflected, even more the
host may have very different igmp config for the port net device than the bridge does.
Also your current implementation switches to V3 only if it is globally set, and that should
be controlled on a per-bridge basis, even per-vlan later.
One more thing, if someone does a V2 leave for a group, you can end up sending them V3
group-specific query.

I have been working on an implementation for IGMPv3/MLDv2 querier for the bridge device, it re-uses
most of the current code and allows you to configure it per-bridge (and later per-vlan). The only
reason I've not yet sent it to upstream is that we (at Cumulus) are working out the compatibility
parts (e.g. RFC3376 sec 7, sec 8) of the RFC since it has some unclear cases.
But I can rip the compatibility out and send it early for review if you'd like, we can add the
compat code later.

Cheers,
  Nik

> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 2136e45..9fb47f3 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -35,6 +35,10 @@
>
>  #include "br_private.h"
>
> +#define IGMP_V3_SEEN(in_dev) \
> +	(IPV4_DEVCONF_ALL(dev_net(in_dev->dev), FORCE_IGMP_VERSION) == 3 || \
> +	 IN_DEV_CONF_GET((in_dev), FORCE_IGMP_VERSION) == 3)
> +
>  static void br_multicast_start_querier(struct net_bridge *br,
>  				       struct bridge_mcast_own_query *query);
>  static void br_multicast_add_router(struct net_bridge *br,
> @@ -360,9 +364,8 @@ static int br_mdb_rehash(struct net_bridge_mdb_htable __rcu **mdbp, int max,
>  	return 0;
>  }
>
> -static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge *br,
> -						    __be32 group,
> -						    u8 *igmp_type)
> +static struct sk_buff *br_ip4_alloc_query_v2(struct net_bridge *br,
> +					     __be32 group, u8 *igmp_type)
>  {
>  	struct sk_buff *skb;
>  	struct igmphdr *ih;
> @@ -428,10 +431,82 @@ static struct sk_buff *br_ip4_multicast_alloc_query(struct net_bridge *br,
>  	return skb;
>  }
>
> +static struct sk_buff *br_ip4_alloc_query_v3(struct net_bridge *br,
> +					     __be32 group, u8 *igmp_type)
> +{
> +	struct sk_buff *skb;
> +	struct igmpv3_query *ih3;
> +	struct ethhdr *eth;
> +	struct iphdr *iph;
> +
> +	skb = netdev_alloc_skb_ip_align(br->dev, sizeof(*eth) + sizeof(*iph) +
> +						 sizeof(*ih3) + 4);
> +	if (!skb)
> +		goto out;
> +
> +	skb->protocol = htons(ETH_P_IP);
> +
> +	skb_reset_mac_header(skb);
> +	eth = eth_hdr(skb);
> +
> +	ether_addr_copy(eth->h_source, br->dev->dev_addr);
> +	eth->h_dest[0] = 1;
> +	eth->h_dest[1] = 0;
> +	eth->h_dest[2] = 0x5e;
> +	eth->h_dest[3] = 0;
> +	eth->h_dest[4] = 0;
> +	eth->h_dest[5] = 1;
> +	eth->h_proto = htons(ETH_P_IP);
> +	skb_put(skb, sizeof(*eth));
> +
> +	skb_set_network_header(skb, skb->len);
> +	iph = ip_hdr(skb);
> +
> +	iph->version = 4;
> +	iph->ihl = 6;
> +	iph->tos = 0xc0;
> +	iph->tot_len = htons(sizeof(*iph) + sizeof(*ih3) + 4);
> +	iph->id = 0;
> +	iph->frag_off = htons(IP_DF);
> +	iph->ttl = 1;
> +	iph->protocol = IPPROTO_IGMP;
> +	iph->saddr = br->multicast_query_use_ifaddr ?
> +		     inet_select_addr(br->dev, 0, RT_SCOPE_LINK) : 0;
> +	iph->daddr = htonl(INADDR_ALLHOSTS_GROUP);
> +	((u8 *)&iph[1])[0] = IPOPT_RA;
> +	((u8 *)&iph[1])[1] = 4;
> +	((u8 *)&iph[1])[2] = 0;
> +	((u8 *)&iph[1])[3] = 0;
> +	ip_send_check(iph);
> +	skb_put(skb, 24);
> +
> +	skb_set_transport_header(skb, skb->len);
> +	ih3 = igmpv3_query_hdr(skb);
> +
> +	*igmp_type = IGMP_HOST_MEMBERSHIP_QUERY;
> +	ih3->type = IGMP_HOST_MEMBERSHIP_QUERY;
> +	ih3->code = (group ? br->multicast_last_member_interval :
> +			    br->multicast_query_response_interval) /
> +		   (HZ / IGMP_TIMER_SCALE);
> +	ih3->csum = 0;
> +	ih3->group = group;
> +	ih3->resv = 0;
> +	ih3->suppress = 0;
> +	ih3->qrv= 2;
> +	ih3->qqic = br->multicast_query_interval / HZ;
> +	ih3->nsrcs = 0;
> +	ih3->csum = ip_compute_csum((void *)ih3, sizeof(struct igmpv3_query ));
> +	skb_put(skb, sizeof(*ih3));
> +
> +	__skb_pull(skb, sizeof(*eth));
> +
> +out:
> +	return skb;
> +}
>  #if IS_ENABLED(CONFIG_IPV6)
> -static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge *br,
> -						    const struct in6_addr *grp,
> -						    u8 *igmp_type)
> +static struct sk_buff *br_ip6_alloc_query_v1(struct net_bridge *br,
> +					     const struct in6_addr *grp,
> +					     u8 *igmp_type)
>  {
>  	struct sk_buff *skb;
>  	struct ipv6hdr *ip6h;
> @@ -514,19 +589,129 @@ static struct sk_buff *br_ip6_multicast_alloc_query(struct net_bridge *br,
>  out:
>  	return skb;
>  }
> +
> +static struct sk_buff *br_ip6_alloc_query_v2(struct net_bridge *br,
> +					     const struct in6_addr *grp,
> +					     u8 *igmp_type)
> +{
> +	struct sk_buff *skb;
> +	struct ipv6hdr *ip6h;
> +	struct mld2_query *mld2q;
> +	struct ethhdr *eth;
> +	u8 *hopopt;
> +	unsigned long interval;
> +
> +	skb = netdev_alloc_skb_ip_align(br->dev, sizeof(*eth) + sizeof(*ip6h) +
> +						 8 + sizeof(*mld2q));
> +	if (!skb)
> +		goto out;
> +
> +	skb->protocol = htons(ETH_P_IPV6);
> +
> +	/* Ethernet header */
> +	skb_reset_mac_header(skb);
> +	eth = eth_hdr(skb);
> +
> +	ether_addr_copy(eth->h_source, br->dev->dev_addr);
> +	eth->h_proto = htons(ETH_P_IPV6);
> +	skb_put(skb, sizeof(*eth));
> +
> +	/* IPv6 header + HbH option */
> +	skb_set_network_header(skb, skb->len);
> +	ip6h = ipv6_hdr(skb);
> +
> +	*(__force __be32 *)ip6h = htonl(0x60000000);
> +	ip6h->payload_len = htons(8 + sizeof(*mld2q));
> +	ip6h->nexthdr = IPPROTO_HOPOPTS;
> +	ip6h->hop_limit = 1;
> +	ipv6_addr_set(&ip6h->daddr, htonl(0xff020000), 0, 0, htonl(1));
> +	if (ipv6_dev_get_saddr(dev_net(br->dev), br->dev, &ip6h->daddr, 0,
> +			       &ip6h->saddr)) {
> +		kfree_skb(skb);
> +		br->has_ipv6_addr = 0;
> +		return NULL;
> +	}
> +
> +	br->has_ipv6_addr = 1;
> +	ipv6_eth_mc_map(&ip6h->daddr, eth->h_dest);
> +
> +	hopopt = (u8 *)(ip6h + 1);
> +	hopopt[0] = IPPROTO_ICMPV6;		/* next hdr */
> +	hopopt[1] = 0;				/* length of HbH */
> +	hopopt[2] = IPV6_TLV_ROUTERALERT;	/* Router Alert */
> +	hopopt[3] = 2;				/* Length of RA Option */
> +	hopopt[4] = 0;				/* Type = 0x0000 (MLD) */
> +	hopopt[5] = 0;
> +	hopopt[6] = IPV6_TLV_PAD1;		/* Pad1 */
> +	hopopt[7] = IPV6_TLV_PAD1;		/* Pad1 */
> +
> +	skb_put(skb, sizeof(*ip6h) + 8);
> +
> +	/* ICMPv6 */
> +	skb_set_transport_header(skb, skb->len);
> +	mld2q = (struct mld2_query *) icmp6_hdr(skb);
> +
> +	interval = ipv6_addr_any(grp) ?
> +			br->multicast_query_response_interval :
> +			br->multicast_last_member_interval;
> +
> +	*igmp_type = ICMPV6_MGM_QUERY;
> +	mld2q->mld2q_type = ICMPV6_MGM_QUERY;
> +	mld2q->mld2q_code = 0;
> +	mld2q->mld2q_cksum = 0;
> +	mld2q->mld2q_mrc = htons((u16)jiffies_to_msecs(interval));
> +	mld2q->mld2q_resv1 = 0;
> +	mld2q->mld2q_mca = *grp;
> +	mld2q->mld2q_resv2 = 0;
> +	mld2q->mld2q_suppress = 0;
> +	mld2q->mld2q_qrv = 2;
> +	mld2q->mld2q_qqic = br->multicast_query_interval / HZ;
> +	mld2q->mld2q_nsrcs = 0;
> +
> +	/* checksum */
> +	mld2q->mld2q_cksum = csum_ipv6_magic(&ip6h->saddr, &ip6h->daddr,
> +					  sizeof(*mld2q), IPPROTO_ICMPV6,
> +					  csum_partial(mld2q,
> +						       sizeof(*mld2q), 0));
> +	skb_put(skb, sizeof(*mld2q));
> +
> +	__skb_pull(skb, sizeof(*eth));
> +
> +out:
> +	return skb;
> +}
>  #endif
>
> +static int mld_force_mld_version(const struct inet6_dev *idev)
> +{
> +	if (dev_net(idev->dev)->ipv6.devconf_all->force_mld_version != 0)
> +		return dev_net(idev->dev)->ipv6.devconf_all->force_mld_version;
> +	else
> +		return idev->cnf.force_mld_version;
> +}
> +
>  static struct sk_buff *br_multicast_alloc_query(struct net_bridge *br,
>  						struct br_ip *addr,
>  						u8 *igmp_type)
>  {
> +	struct in_device *in_dev = __in_dev_get_rcu(br->dev);
> +	struct inet6_dev *idev = __in6_dev_get(br->dev);
>  	switch (addr->proto) {
>  	case htons(ETH_P_IP):
> -		return br_ip4_multicast_alloc_query(br, addr->u.ip4, igmp_type);
> +		if (IGMP_V3_SEEN(in_dev))
> +			return br_ip4_alloc_query_v3(br, addr->u.ip4,
> +						     igmp_type);
> +		else
> +			return br_ip4_alloc_query_v2(br, addr->u.ip4,
> +						     igmp_type);
>  #if IS_ENABLED(CONFIG_IPV6)
>  	case htons(ETH_P_IPV6):
> -		return br_ip6_multicast_alloc_query(br, &addr->u.ip6,
> -						    igmp_type);
> +		if (mld_force_mld_version(idev) == 2)
> +			return br_ip6_alloc_query_v2(br, &addr->u.ip6,
> +						     igmp_type);
> +		else
> +			return br_ip6_alloc_query_v1(br, &addr->u.ip6,
> +						     igmp_type);
>  #endif
>  	}
>  	return NULL;
>

^ permalink raw reply

* Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM
From: Harini Katakam @ 2016-11-18 10:03 UTC (permalink / raw)
  To: Rafal Ozieblo
  Cc: Nicolas Ferre, Andrei Pistirica, harini.katakam@xilinx.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <BN3PR07MB25162D82CEF09B8C7586B799C9B00@BN3PR07MB2516.namprd07.prod.outlook.com>

Hi Rafal,


> I can’t see a place where you enable extended descriptor for PTP. Did you add support for extended PTP descriptor?
Sorry, that was separate patch in the series:
http://lkml.iu.edu/hypermail/linux/kernel/1509.1/02239.html

> "DMA Configuration Register" 0x010:
> 29      tx_bd_extended_mode_en  "Enable TX extended BD mode. See TX BD control register definition for description of feature."
> 28      rx_bd_extended_mode_en  "Enable RX extended BD mode. See RX BD control register definition for description of feature."
>
> Can I send you our patch for comparison ?
Sure, please mail. Also, please mention the Cadence IP version number you use.

Regards,
Harini

^ permalink raw reply

* Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM
From: Harini Katakam @ 2016-11-18  9:43 UTC (permalink / raw)
  To: Rafal Ozieblo
  Cc: Nicolas Ferre, Andrei Pistirica, harini.katakam@xilinx.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <BN3PR07MB251655CAFDB38CCF0F240BB5C9B00@BN3PR07MB2516.namprd07.prod.outlook.com>

Hi Rafal,

On Fri, Nov 18, 2016 at 3:00 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:
>>-----Original Message-----
>>From: Nicolas Ferre [mailto:nicolas.ferre@atmel.com]
>>Sent: 18 listopada 2016 10:10
>>To: Rafal Ozieblo; Harini Katakam; Andrei Pistirica
>>Cc: harini.katakam@xilinx.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
>>Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM
>>
>>Le 18/11/2016 à 09:59, Rafal Ozieblo a écrit :
>>> Hello,
>>>
>>>> From: Harini Katakam [mailto:harinikatakamlinux@gmail.com]
>>>> Sent: 18 listopada 2016 05:30
>>>> To: Rafal Ozieblo
>>>> Cc: Nicolas Ferre; harini.katakam@xilinx.com; netdev@vger.kernel.org;
>>>> linux-kernel@vger.kernel.org
>>>> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support
>>>> for GEM
>>>>
>>>> Hi Rafal,
>>>>
>>>> On Thu, Nov 17, 2016 at 7:05 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:
>>>>> -----Original Message-----
>>>>> From: Nicolas Ferre [mailto:nicolas.ferre@atmel.com]
>>>>> Sent: 17 listopada 2016 14:29
>>>>> To: Harini Katakam; Rafal Ozieblo
>>>>> Cc: harini.katakam@xilinx.com; netdev@vger.kernel.org;
>>>>> linux-kernel@vger.kernel.org
>>>>> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing
>>>>> support for GEM
>>>>>
>>>>>> Le 17/11/2016 à 13:21, Harini Katakam a écrit :
>>>>>>> Hi Rafal,
>>>>>>>
>>>>>>> On Thu, Nov 17, 2016 at 5:20 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:
>>>>>>>> Hello,
>>>>>>>> I think, there could a bug in your patch.
>>>>>>>>
>>>>>>>>> +
>>>>>>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>>>>>>>> +             dmacfg |= GEM_BIT(ADDR64); #endif
>>>>>>>>
>>>>>>>> You enable 64 bit addressing (64b dma bus width) always when appropriate architecture config option is enabled.
>>>>>>>> But there are some legacy controllers which do not support that feature. According Cadence hardware team:
>>>>>>>> "64 bit addressing was added in July 2013. Earlier version do not have it.
>>>>>>>> This feature was enhanced in release August 2014 to have separate upper address values for transmit and receive."
>>>>>>>>
>>>>>>>>> /* Bitfields in NSR */
>>>>>>>>> @@ -474,6 +479,10 @@
>>>>>>>>>  struct macb_dma_desc {
>>>>>>>>  >      u32     addr;
>>>>>>>>>       u32     ctrl;
>>>>>>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>>>>>>>> +     u32     addrh;
>>>>>>>>> +     u32     resvd;
>>>>>>>>> +#endif
>>>>>>>>>  };
>>>>>>>>
>>>>>>>> It will not work for legacy hardware. Old descriptor is 2 words wide, the new one is 4 words wide.
>>>>>>>> If you enable CONFIG_ARCH_DMA_ADDR_T_64BIT but hardware doesn't
>>>>>>>> support it at all, you will miss every second descriptor.
>>>>>>>>
>>>>>>>
>>>>>>> True, this feature is not available in all of Cadence IP versions.
>>>>>>> In fact, the IP version Zynq does not support this. But the one in ZynqMP does.
>>>>>>> So, we enable kernel config for 64 bit DMA addressing for this SoC
>>>>>>> and hence the driver picks it up. My assumption was that if the
>>>>>>> legacy IP does not support
>>>>>>> 64 bit addressing, then this DMA option wouldn't be enabled.
>>>>>>>
>>>>>>> There is a design config register in Cadence IP which is being
>>>>>>> read to check for 64 bit address support - DMA mask is set based on that.
>>>>>>> But the addition of two descriptor words cannot be based on this runtime check.
>>>>>>> For this reason, all the static changes were placed under this check.
>>>>>>
>>>>>> We have quite a bunch of options in this driver to determinate what is the real capacity of the underlying hardware.
>>>>>> If HW configuration registers are not appropriate, and it seems they are not, I would advice to simply use the DT compatibility string.
>>>>>>
>>>>>> Best regards,
>>>>>> --
>>>>>> Nicolas Ferre
>>>>>
>>>>> HW configuration registers are appropriate. The issue is that this code doesn’t use the capability bit to switch between different dma descriptors (2 words vs. 4 words).
>>>>> DMA descriptor size is chosen based on kernel configuration, not based on hardware capabilities.
>>>>
>>>> HW configuration register does give appropriate information.
>>>> But addition of two address words in the macb descriptor structure is a static change.
>>>>
>>>> +static inline void macb_set_addr(struct macb_dma_desc *desc,
>>>> +dma_addr_t
>>>> +addr) {
>>>> +       desc->addr = (u32)addr;
>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>>> +       desc->addrh = (u32)(addr >> 32); #endif
>>>> +
>>>>
>>>> Even if the #ifdef condition here is changed to HW config check, addr and addrh are different.
>>>> And "addrh" entry has to be present for 64 bit desc case to be handled separately.
>>>> Can you please tell me how you propose change in DMA descriptor
>>>> structure from
>>>> 4 to 2 or 2 to 4 words *after* reading the DCFG register?
>>>
>>> It is very complex problem. I wrote to you because I faced the same issue. I'm working on PTP implementation for macb. When PTP is enabled there are additional two words in buffer descriptor.
>>
>>Moreover, we can use PTP without the 64bits descriptor support (early GEM revisions).
>>
>>BTW, note that there is an initiative ongoing with Andrei to support
>>PTP/1588 on these devices with patches already send: please synchronize with him.
>>https://urldefense.proofpoint.com/v2/url?u=https-3A__marc.info_-3Fl-3Dlinux-2Dnetdev-26m-3D147282092309112-26w-3D3&d=DgIDaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtCg4L5E&m=jUmZzqwn1uRrvcqF3BHCHFC6ZsKoZkU3vT8gJ-7fnsE&s=kr_km0MKQBzpkaOt0fkM-FIajN1-pOylzzTjsXi-vak&e=
>>or
>>https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.org_patch_9310989_&d=DgIDaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtCg4L5E&m=jUmZzqwn1uRrvcqF3BHCHFC6ZsKoZkU3vT8gJ-7fnsE&s=ZbXVD5Lb5zGn7wUKIPYHxagIEKp_vJvrnkRa4qJfmTY&e=
>>and
>>https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.org_patch_9310991_&d=DgIDaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtCg4L5E&m=jUmZzqwn1uRrvcqF3BHCHFC6ZsKoZkU3vT8gJ-7fnsE&s=Z0kRqUqh5Is4TmuaY7A4hnpizfdeq3HrgPhyDAMLuA8&e=
>>
>>Regards,
>>--
>>Nicolas Ferre
>
> Above patch doesn’t use hardware timestamps in descriptors at all. It doesn't use appropriate accuracy.
> We have our PTP patch ready, which use timestamps from descriptor. But we have not sent it yet because of compatibility issue.
> Of course, we can do it like Harini did:
>
> struct macb_dma_desc {
>         u32     addr;
>         u32     ctrl;
> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>         u32     addrh;
>         u32     resvd;
> #endif
> #if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
>         u32     dma_desc_ts_1;
>         u32     dma_desc_ts_2;
> #endif
> };
>
> But in that approach we lose backward compatibility.
>
> What are your suggestion? Can we send patch like it is or wait for some common solution with backward compatibility?
>

Yes, Andre's version of Cadence does not ability to report have RX timestamp.
The version I worked with did. This is the old series I sent:
https://lkml.org/lkml/2015/9/11/92
But right now, i'm working on building on top of Andre's changes.

Regards,
Harini

^ permalink raw reply

* [PATCH] netns: fix get_net_ns_by_fd(int pid) typo
From: Stefan Hajnoczi @ 2016-11-18  9:41 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, Stefan Hajnoczi

The argument to get_net_ns_by_fd() is a /proc/$PID/ns/net file
descriptor not a pid.  Fix the typo.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/net/net_namespace.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index fc4f757..0940598 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -170,7 +170,7 @@ static inline struct net *copy_net_ns(unsigned long flags,
 extern struct list_head net_namespace_list;
 
 struct net *get_net_ns_by_pid(pid_t pid);
-struct net *get_net_ns_by_fd(int pid);
+struct net *get_net_ns_by_fd(int fd);
 
 #ifdef CONFIG_SYSCTL
 void ipx_register_sysctl(void);
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH net v2 7/7] net: ethernet: ti: cpsw: fix fixed-link phy probe deferral
From: Johan Hovold @ 2016-11-18  9:33 UTC (permalink / raw)
  To: David Miller
  Cc: johan, mugunthanvnm, grygorii.strashko, linux-omap, netdev,
	linux-kernel
In-Reply-To: <20161117171920.GD10490@localhost>

On Thu, Nov 17, 2016 at 06:19:20PM +0100, Johan Hovold wrote:
> On Thu, Nov 17, 2016 at 12:04:16PM -0500, David Miller wrote:
> > From: Johan Hovold <johan@kernel.org>
> > Date: Thu, 17 Nov 2016 17:40:04 +0100
> > 
> > > Make sure to propagate errors from of_phy_register_fixed_link() which
> > > can fail with -EPROBE_DEFER.
> > > 
> > > Fixes: 1f71e8c96fc6 ("drivers: net: cpsw: Add support for fixed-link
> > > PHY")
> > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > 
> > Johan, when you update a patch within a series you must post the
> > entire series freshly to the lists, cover posting and all.
> 
> I'm quite sure that is exactly what I did. Did you only get this last
> patch out of the seven?

The full series has made it to the lists:

	https://marc.info/?l=linux-netdev&m=147940433115984&w=2

Perhaps some messages just got held up.

Let me know if you still want me to resend.

Thanks,
Johan

^ permalink raw reply

* RE: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM
From: Rafal Ozieblo @ 2016-11-18  8:59 UTC (permalink / raw)
  To: Harini Katakam
  Cc: Nicolas Ferre, harini.katakam@xilinx.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <CAFcVEC+RqUMQF39oe2EuCoisX6dCwRQ3G81zFMqoOHU3HpfPHQ@mail.gmail.com>

Hello,

> From: Harini Katakam [mailto:harinikatakamlinux@gmail.com]
> Sent: 18 listopada 2016 05:30
> To: Rafal Ozieblo
> Cc: Nicolas Ferre; harini.katakam@xilinx.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM
>
> Hi Rafal,
>
> On Thu, Nov 17, 2016 at 7:05 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:
> > -----Original Message-----
> > From: Nicolas Ferre [mailto:nicolas.ferre@atmel.com]
> > Sent: 17 listopada 2016 14:29
> > To: Harini Katakam; Rafal Ozieblo
> > Cc: harini.katakam@xilinx.com; netdev@vger.kernel.org;
> > linux-kernel@vger.kernel.org
> > Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support
> > for GEM
> >
> >> Le 17/11/2016 à 13:21, Harini Katakam a écrit :
> >> > Hi Rafal,
> >> >
> >> > On Thu, Nov 17, 2016 at 5:20 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:
> >> >> Hello,
> >> >> I think, there could a bug in your patch.
> >> >>
> >> >>> +
> >> >>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> >> >>> +             dmacfg |= GEM_BIT(ADDR64); #endif
> >> >>
> >> >> You enable 64 bit addressing (64b dma bus width) always when appropriate architecture config option is enabled.
> >> >> But there are some legacy controllers which do not support that feature. According Cadence hardware team:
> >> >> "64 bit addressing was added in July 2013. Earlier version do not have it.
> >> >> This feature was enhanced in release August 2014 to have separate upper address values for transmit and receive."
> >> >>
> >> >>> /* Bitfields in NSR */
> >> >>> @@ -474,6 +479,10 @@
> >> >>>  struct macb_dma_desc {
> >> >>  >      u32     addr;
> >> >>>       u32     ctrl;
> >> >>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> >> >>> +     u32     addrh;
> >> >>> +     u32     resvd;
> >> >>> +#endif
> >> >>>  };
> >> >>
> >> >> It will not work for legacy hardware. Old descriptor is 2 words wide, the new one is 4 words wide.
> >> >> If you enable CONFIG_ARCH_DMA_ADDR_T_64BIT but hardware doesn't
> >> >> support it at all, you will miss every second descriptor.
> >> >>
> >> >
> >> > True, this feature is not available in all of Cadence IP versions.
> >> > In fact, the IP version Zynq does not support this. But the one in ZynqMP does.
> >> > So, we enable kernel config for 64 bit DMA addressing for this SoC
> >> > and hence the driver picks it up. My assumption was that if the
> >> > legacy IP does not support
> >> > 64 bit addressing, then this DMA option wouldn't be enabled.
> >> >
> >> > There is a design config register in Cadence IP which is being read
> >> > to check for 64 bit address support - DMA mask is set based on that.
> >> > But the addition of two descriptor words cannot be based on this runtime check.
> >> > For this reason, all the static changes were placed under this check.
> >>
> >> We have quite a bunch of options in this driver to determinate what is the real capacity of the underlying hardware.
> >> If HW configuration registers are not appropriate, and it seems they are not, I would advice to simply use the DT compatibility string.
> >>
> >> Best regards,
> >> --
> >> Nicolas Ferre
> >
> > HW configuration registers are appropriate. The issue is that this code doesn’t use the capability bit to switch between different dma descriptors (2 words vs. 4 words).
> > DMA descriptor size is chosen based on kernel configuration, not based on hardware capabilities.
>
> HW configuration register does give appropriate information.
> But addition of two address words in the macb descriptor structure is a static change.
>
> +static inline void macb_set_addr(struct macb_dma_desc *desc, dma_addr_t
> +addr) {
> +       desc->addr = (u32)addr;
> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> +       desc->addrh = (u32)(addr >> 32); #endif
> +
>
> Even if the #ifdef condition here is changed to HW config check, addr and addrh are different.
> And "addrh" entry has to be present for 64 bit desc case to be handled separately.
> Can you please tell me how you propose change in DMA descriptor structure from
> 4 to 2 or 2 to 4 words *after* reading the DCFG register?

It is very complex problem. I wrote to you because I faced the same issue. I'm working on PTP implementation for macb. When PTP is enabled there are additional two words in buffer descriptor.
But hardware might not be compatible with PTP. Therefore I have to change DMA descriptor between 2 and 4 words after reading DCFG register (the same as you between 32b and 64b).
When we consider both PTP and 64 bits, we end up with 4 different descriptors!

1. 32b and without PTP support:

struct macb_dma_desc {
	u32	addr;
	u32	ctrl;
}

2. 64b and without PTP support:

struct macb_dma_desc {
	u32	addr;
	u32	ctrl;
	u32 	addrh;
	u32  	resvd;
}

3. 32 and PTP support:

struct macb_dma_desc {
	u32	addr;
	u32	ctrl;
	u32	dma_desc_ts_1;
	u32	dma_desc_ts_2;
};

4. 64b and PTP support:

struct macb_dma_desc {
	u32 	addr;
	u32 	ctrl;
	u32 	addrh;
	u32  	resvd;
	u32 	dma_desc_ts_1;
	u32	dma_desc_ts_2;
};

> Can you please tell me how you propose change in DMA descriptor structure from
> 4 to 2 or 2 to 4 words *after* reading the DCFG register?

I have some ideas but I don’t know whether it is possible to upstream it later on.
IMHO, we need to use the same mechanism in both our cases.

My very first idea to start discussion:
(Below is kind of pseudo code only to show my idea. All defines like 64B or PTP are omitted for simplicity)

1. Prepare appropriate structures:

struct macb_dma_desc {
	u32	addr;
	u32	ctrl;
}

struct macb_dma_desc_64 {
	u32 addrh;
	u32 resvd;
}

struct macb_dma_desc_ptp {
	u32 dma_desc_ts_1;
	u32	dma_desc_ts_2;
}

2. Add hardware support information to macb:

enum macb_hw_cap {
	HW_CAP_NONE,
	HW_CAP_64B,
	HW_CAP_PTP,
	HW_CAP_64B_PTP,
};

struct macb {
// (...)
	macb_hw_cap hw_cap;

}

3. Set bp->hw_cap on macb_probe()

4. Additional function might be helpful:

static unsigned char macb_dma_desc_get_mul(struct macb *bp)
{
	switch (bp->hw_cap) {
		case HW_CAP_NONE:
			return 1;
		case HW_CAP_64B:
		case HW_CAP_PTP:
			return 2;
		case HW_CAP_64B_PTP:
			return 3;
	}
}

5. Change sizeof struct to function:
(sizeof(struct macb_dma_desc) change to macb_dma_desc_get_size(bp). It will return sizeof(struct macb_dma_desc) * macb_dma_desc_get_mul(bp).
There is a hidden assumption that all three structures have the same size.

6. macb_rx_desc() and macb_tx_desc() will still return struct macb_dma_desc * but descriptor index will be counted in different way, ex.

static struct macb_dma_desc *macb_rx_desc(struct macb *bp, unsigned int index)
{
	index *= macb_dma_desc_get_mul(bp);
	return &bp->rx_ring[macb_rx_ring_wrap(bp, index)];
}

7. Two additional functions to dereference PTP and 64b information:

static struct macb_dma_desc_64 *macb_64b_desc(struct macb *bp, struct macb_dma_desc *desc)
{
	switch (bp->hw_cap) {
		case HW_CAP_64B:
		case HW_CAP_64B_PTP:
			return (struct macb_dma_desc_64 *)(desc + 1);  // ugly casting
		default:
			return NULL;
	}
}

static struct macb_dma_desc_ptp *macb_ptp_desc(struct macb *bp, struct macb_dma_desc *desc)
{
	switch (bp->hw_cap) {
		case HW_CAP_PTP:
			return (struct macb_dma_desc_ptp *)(desc + 1);
		case HW_CAP_64B_PTP:
			return (struct macb_dma_desc_ptp *)(desc + 2);
		default:
			return NULL;
	}
}

Whenever you want to reach fields in appropriate descriptor, above function should be used.

This is only my very first idea. Of course, we can leave it as it is and say, that old hardware is not support either when PTP enabled or 64b enabled.

^ permalink raw reply

* RE: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM
From: Rafal Ozieblo @ 2016-11-18  9:30 UTC (permalink / raw)
  To: Nicolas Ferre, Harini Katakam, Andrei Pistirica
  Cc: harini.katakam@xilinx.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <a315b554-8b44-ac82-a104-e382940750bb@atmel.com>

>-----Original Message-----         
>From: Nicolas Ferre [mailto:nicolas.ferre@atmel.com] 
>Sent: 18 listopada 2016 10:10                        
>To: Rafal Ozieblo; Harini Katakam; Andrei Pistirica  
>Cc: harini.katakam@xilinx.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
>Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM      
>                                                                                   
>Le 18/11/2016 à 09:59, Rafal Ozieblo a écrit :                                     
>> Hello,                                                                           
>>                                                                                  
>>> From: Harini Katakam [mailto:harinikatakamlinux@gmail.com]                      
>>> Sent: 18 listopada 2016 05:30                                                   
>>> To: Rafal Ozieblo                                                               
>>> Cc: Nicolas Ferre; harini.katakam@xilinx.com; netdev@vger.kernel.org;           
>>> linux-kernel@vger.kernel.org                                                    
>>> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support           
>>> for GEM                                                                         
>>>                                                                                 
>>> Hi Rafal,                                                                       
>>>                                                                                 
>>> On Thu, Nov 17, 2016 at 7:05 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:      
>>>> -----Original Message-----                                                     
>>>> From: Nicolas Ferre [mailto:nicolas.ferre@atmel.com]                           
>>>> Sent: 17 listopada 2016 14:29                                                  
>>>> To: Harini Katakam; Rafal Ozieblo                                              
>>>> Cc: harini.katakam@xilinx.com; netdev@vger.kernel.org;                         
>>>> linux-kernel@vger.kernel.org                                                   
>>>> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing                  
>>>> support for GEM                                                                
>>>>                                                                                
>>>>> Le 17/11/2016 à 13:21, Harini Katakam a écrit :                               
>>>>>> Hi Rafal,                                                                    
>>>>>>                                                                              
>>>>>> On Thu, Nov 17, 2016 at 5:20 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:   
>>>>>>> Hello,                                                                      
>>>>>>> I think, there could a bug in your patch.                                   
>>>>>>>                                                                             
>>>>>>>> +                                                                          
>>>>>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT                                       
>>>>>>>> +             dmacfg |= GEM_BIT(ADDR64); #endif                            
>>>>>>>                                                                             
>>>>>>> You enable 64 bit addressing (64b dma bus width) always when appropriate architecture config option is enabled.                                                                                           
>>>>>>> But there are some legacy controllers which do not support that feature. According Cadence hardware team:                                                                                                 
>>>>>>> "64 bit addressing was added in July 2013. Earlier version do not have it.                       
>>>>>>> This feature was enhanced in release August 2014 to have separate upper address values for transmit and receive."                                                                                         
>>>>>>>                                                                                                  
>>>>>>>> /* Bitfields in NSR */                                                                          
>>>>>>>> @@ -474,6 +479,10 @@                                                                            
>>>>>>>>  struct macb_dma_desc {                                                                         
>>>>>>>  >      u32     addr;                                                                            
>>>>>>>>       u32     ctrl;                                                                             
>>>>>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT                                                            
>>>>>>>> +     u32     addrh;                                                                            
>>>>>>>> +     u32     resvd;                                                                            
>>>>>>>> +#endif                                                                                         
>>>>>>>>  };                                                                                             
>>>>>>>                                                                                                  
>>>>>>> It will not work for legacy hardware. Old descriptor is 2 words wide, the new one is 4 words wide.                                                                                                        
>>>>>>> If you enable CONFIG_ARCH_DMA_ADDR_T_64BIT but hardware doesn't                                  
>>>>>>> support it at all, you will miss every second descriptor.                                        
>>>>>>>                                                                                                  
>>>>>>                                                                                                   
>>>>>> True, this feature is not available in all of Cadence IP versions.                                
>>>>>> In fact, the IP version Zynq does not support this. But the one in ZynqMP does.                   
>>>>>> So, we enable kernel config for 64 bit DMA addressing for this SoC                                
>>>>>> and hence the driver picks it up. My assumption was that if the                                   
>>>>>> legacy IP does not support                                                                        
>>>>>> 64 bit addressing, then this DMA option wouldn't be enabled.                                      
>>>>>>                                                                                                   
>>>>>> There is a design config register in Cadence IP which is being                                    
>>>>>> read to check for 64 bit address support - DMA mask is set based on that.                         
>>>>>> But the addition of two descriptor words cannot be based on this runtime check.                   
>>>>>> For this reason, all the static changes were placed under this check.                             
>>>>>                                                                                                    
>>>>> We have quite a bunch of options in this driver to determinate what is the real capacity of the underlying hardware.                                                                                        
>>>>> If HW configuration registers are not appropriate, and it seems they are not, I would advice to simply use the DT compatibility string.                                                                     
>>>>>                                                                                                    
>>>>> Best regards,                                                                                      
>>>>> --                                                                                                 
>>>>> Nicolas Ferre                                                                                      
>>>>                                                                                                     
>>>> HW configuration registers are appropriate. The issue is that this code doesn’t use the capability bit to switch between different dma descriptors (2 words vs. 4 words).                                    
>>>> DMA descriptor size is chosen based on kernel configuration, not based on hardware capabilities.    
>>>                                                                                                      
>>> HW configuration register does give appropriate information.                                         
>>> But addition of two address words in the macb descriptor structure is a static change.               
>>>                                                                                                      
>>> +static inline void macb_set_addr(struct macb_dma_desc *desc,                                        
>>> +dma_addr_t                                                                                          
>>> +addr) {                                                                                             
>>> +       desc->addr = (u32)addr;                                                                      
>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT                                                                 
>>> +       desc->addrh = (u32)(addr >> 32); #endif                                                      
>>> +                                                                                                    
>>>                                                                                                      
>>> Even if the #ifdef condition here is changed to HW config check, addr and addrh are different.       
>>> And "addrh" entry has to be present for 64 bit desc case to be handled separately.                   
>>> Can you please tell me how you propose change in DMA descriptor                                      
>>> structure from                                                                                       
>>> 4 to 2 or 2 to 4 words *after* reading the DCFG register?                                            
>>                                                                                                       
>> It is very complex problem. I wrote to you because I faced the same issue. I'm working on PTP implementation for macb. When PTP is enabled there are additional two words in buffer descriptor.                
>                                                                                                        
>Moreover, we can use PTP without the 64bits descriptor support (early GEM revisions).                   
>                                                                                                        
>BTW, note that there is an initiative ongoing with Andrei to support                                    
>PTP/1588 on these devices with patches already send: please synchronize with him.                       
>https://urldefense.proofpoint.com/v2/url?u=https-3A__marc.info_-3Fl-3Dlinux-2Dnetdev-26m-3D147282092309112-26w-3D3&d=DgIDaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtCg4L5E&m=jUmZzqwn1uRrvcqF3BHCHFC6ZsKoZkU3vT8gJ-7fnsE&s=kr_km0MKQBzpkaOt0fkM-FIajN1-pOylzzTjsXi-vak&e=    
>or                                                                                                      
>https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.org_patch_9310989_&d=DgIDaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtCg4L5E&m=jUmZzqwn1uRrvcqF3BHCHFC6ZsKoZkU3vT8gJ-7fnsE&s=ZbXVD5Lb5zGn7wUKIPYHxagIEKp_vJvrnkRa4qJfmTY&e=                              
>and                                                                                                     
>https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.kernel.org_patch_9310991_&d=DgIDaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=MWSZN_jP4BHjuU9UFsrndALGeJqIXzD0WtGCtCg4L5E&m=jUmZzqwn1uRrvcqF3BHCHFC6ZsKoZkU3vT8gJ-7fnsE&s=Z0kRqUqh5Is4TmuaY7A4hnpizfdeq3HrgPhyDAMLuA8&e=                              
>                                                                                                        
>Regards,                                                                                                
>--
>Nicolas Ferre

Above patch doesn’t use hardware timestamps in descriptors at all. It doesn't use appropriate accuracy. 
We have our PTP patch ready, which use timestamps from descriptor. But we have not sent it yet because of compatibility issue.
Of course, we can do it like Harini did:

struct macb_dma_desc {
	u32	addr;
	u32	ctrl;
#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
	u32     addrh;
	u32     resvd;
#endif
#if IS_ENABLED(CONFIG_PTP_1588_CLOCK)
	u32	dma_desc_ts_1;
	u32	dma_desc_ts_2;
#endif
};

But in that approach we lose backward compatibility.

What are your suggestion? Can we send patch like it is or wait for some common solution with backward compatibility?

Best regards,

Rafal Ozieblo   |   Firmware System Engineer, 
phone nbr.: +48 32 5085469    
www.cadence.com

^ permalink raw reply

* Re: [PATCH ipsec] xfrm: unbreak xfrm_sk_policy_lookup
From: Steffen Klassert @ 2016-11-18  9:17 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netdev
In-Reply-To: <1479385306-24731-1-git-send-email-fw@strlen.de>

On Thu, Nov 17, 2016 at 01:21:46PM +0100, Florian Westphal wrote:
> if we succeed grabbing the refcount, then
>   if (err && !xfrm_pol_hold_rcu)
> 
> will evaluate to false so this hits last else branch which then
> sets policy to ERR_PTR(0).
> 
> Fixes: ae33786f73a7ce ("xfrm: policy: only use rcu in xfrm_sk_policy_lookup")
> Reported-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Tested-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>

Applied to the ipsec tree, thanks everyone!

^ permalink raw reply

* RE: [PATCH iproute2 1/1] tc: updated man page to reflect GET command to retrieve a single filter.
From: Sathya Perla @ 2016-11-18  9:11 UTC (permalink / raw)
  To: Roman Mashak, davem; +Cc: netdev, jhs, stephen
In-Reply-To: <1478485658-14379-1-git-send-email-mrv@mojatatu.com>

> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
On Behalf Of Roman Mashak
>
> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> ---
>  man/man8/tc.8 | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/man/man8/tc.8 b/man/man8/tc.8 index 4e99dca..3f4f8b5 100644
> --- a/man/man8/tc.8
> +++ b/man/man8/tc.8
> @@ -28,7 +28,7 @@ class-id ] qdisc
>
>  .B tc
>  .RI "[ " OPTIONS " ]"
> -.B filter [ add | change | replace | delete ] dev
> +.B filter [ add | change | replace | delete | get ] dev
>  DEV
>  .B [ parent
>  qdisc-id

The handle parameter seems to be missing from the TC filter syntax
synopsis ; as in:
tc [ OPTIONS ] filter [ add | change | replace | delete | get ]  dev  DEV
[
       parent  qdisc-id  | root ] protocol protocol prio priority handle
handle-ID

^^^^^^^^^^^^^

I guess the user would have to specify the handle-ID to retrieve a single
filter.

thanks,
-Sathya

^ permalink raw reply

* Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM
From: Nicolas Ferre @ 2016-11-18  9:10 UTC (permalink / raw)
  To: Rafal Ozieblo, Harini Katakam, Andrei Pistirica
  Cc: harini.katakam@xilinx.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <BN3PR07MB25166D8301C60C4A35402AB1C9B00@BN3PR07MB2516.namprd07.prod.outlook.com>

Le 18/11/2016 à 09:59, Rafal Ozieblo a écrit :
> Hello,
> 
>> From: Harini Katakam [mailto:harinikatakamlinux@gmail.com]
>> Sent: 18 listopada 2016 05:30
>> To: Rafal Ozieblo
>> Cc: Nicolas Ferre; harini.katakam@xilinx.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support for GEM
>>
>> Hi Rafal,
>>
>> On Thu, Nov 17, 2016 at 7:05 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:
>>> -----Original Message-----
>>> From: Nicolas Ferre [mailto:nicolas.ferre@atmel.com]
>>> Sent: 17 listopada 2016 14:29
>>> To: Harini Katakam; Rafal Ozieblo
>>> Cc: harini.katakam@xilinx.com; netdev@vger.kernel.org;
>>> linux-kernel@vger.kernel.org
>>> Subject: Re: [RFC PATCH 2/2] net: macb: Add 64 bit addressing support
>>> for GEM
>>>
>>>> Le 17/11/2016 à 13:21, Harini Katakam a écrit :
>>>>> Hi Rafal,
>>>>>
>>>>> On Thu, Nov 17, 2016 at 5:20 PM, Rafal Ozieblo <rafalo@cadence.com> wrote:
>>>>>> Hello,
>>>>>> I think, there could a bug in your patch.
>>>>>>
>>>>>>> +
>>>>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>>>>>> +             dmacfg |= GEM_BIT(ADDR64); #endif
>>>>>>
>>>>>> You enable 64 bit addressing (64b dma bus width) always when appropriate architecture config option is enabled.
>>>>>> But there are some legacy controllers which do not support that feature. According Cadence hardware team:
>>>>>> "64 bit addressing was added in July 2013. Earlier version do not have it.
>>>>>> This feature was enhanced in release August 2014 to have separate upper address values for transmit and receive."
>>>>>>
>>>>>>> /* Bitfields in NSR */
>>>>>>> @@ -474,6 +479,10 @@
>>>>>>>  struct macb_dma_desc {
>>>>>>  >      u32     addr;
>>>>>>>       u32     ctrl;
>>>>>>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>>>>>>> +     u32     addrh;
>>>>>>> +     u32     resvd;
>>>>>>> +#endif
>>>>>>>  };
>>>>>>
>>>>>> It will not work for legacy hardware. Old descriptor is 2 words wide, the new one is 4 words wide.
>>>>>> If you enable CONFIG_ARCH_DMA_ADDR_T_64BIT but hardware doesn't
>>>>>> support it at all, you will miss every second descriptor.
>>>>>>
>>>>>
>>>>> True, this feature is not available in all of Cadence IP versions.
>>>>> In fact, the IP version Zynq does not support this. But the one in ZynqMP does.
>>>>> So, we enable kernel config for 64 bit DMA addressing for this SoC
>>>>> and hence the driver picks it up. My assumption was that if the
>>>>> legacy IP does not support
>>>>> 64 bit addressing, then this DMA option wouldn't be enabled.
>>>>>
>>>>> There is a design config register in Cadence IP which is being read
>>>>> to check for 64 bit address support - DMA mask is set based on that.
>>>>> But the addition of two descriptor words cannot be based on this runtime check.
>>>>> For this reason, all the static changes were placed under this check.
>>>>
>>>> We have quite a bunch of options in this driver to determinate what is the real capacity of the underlying hardware.
>>>> If HW configuration registers are not appropriate, and it seems they are not, I would advice to simply use the DT compatibility string.
>>>>
>>>> Best regards,
>>>> --
>>>> Nicolas Ferre
>>>
>>> HW configuration registers are appropriate. The issue is that this code doesn’t use the capability bit to switch between different dma descriptors (2 words vs. 4 words).
>>> DMA descriptor size is chosen based on kernel configuration, not based on hardware capabilities.
>>
>> HW configuration register does give appropriate information.
>> But addition of two address words in the macb descriptor structure is a static change.
>>
>> +static inline void macb_set_addr(struct macb_dma_desc *desc, dma_addr_t
>> +addr) {
>> +       desc->addr = (u32)addr;
>> +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>> +       desc->addrh = (u32)(addr >> 32); #endif
>> +
>>
>> Even if the #ifdef condition here is changed to HW config check, addr and addrh are different.
>> And "addrh" entry has to be present for 64 bit desc case to be handled separately.
>> Can you please tell me how you propose change in DMA descriptor structure from
>> 4 to 2 or 2 to 4 words *after* reading the DCFG register?
> 
> It is very complex problem. I wrote to you because I faced the same issue. I'm working on PTP implementation for macb. When PTP is enabled there are additional two words in buffer descriptor.

Moreover, we can use PTP without the 64bits descriptor support (early
GEM revisions).

BTW, note that there is an initiative ongoing with Andrei to support
PTP/1588 on these devices with patches already send: please synchronize
with him.
https://marc.info/?l=linux-netdev&m=147282092309112&w=3
or
https://patchwork.kernel.org/patch/9310989/
and
https://patchwork.kernel.org/patch/9310991/

Regards,

> But hardware might not be compatible with PTP. Therefore I have to change DMA descriptor between 2 and 4 words after reading DCFG register (the same as you between 32b and 64b).
> When we consider both PTP and 64 bits, we end up with 4 different descriptors!
> 
> 1. 32b and without PTP support:
> 
> struct macb_dma_desc {
> 	u32	addr;
> 	u32	ctrl;
> }
> 
> 2. 64b and without PTP support:
> 
> struct macb_dma_desc {
> 	u32	addr;
> 	u32	ctrl;
> 	u32 	addrh;
> 	u32  	resvd;
> }
> 
> 3. 32 and PTP support:
> 
> struct macb_dma_desc {
> 	u32	addr;
> 	u32	ctrl;
> 	u32	dma_desc_ts_1;
> 	u32	dma_desc_ts_2;
> };
> 
> 4. 64b and PTP support:
> 
> struct macb_dma_desc {
> 	u32 	addr;
> 	u32 	ctrl;
> 	u32 	addrh;
> 	u32  	resvd;
> 	u32 	dma_desc_ts_1;
> 	u32	dma_desc_ts_2;
> };
> 
>> Can you please tell me how you propose change in DMA descriptor structure from
>> 4 to 2 or 2 to 4 words *after* reading the DCFG register?
> 
> I have some ideas but I don’t know whether it is possible to upstream it later on.
> IMHO, we need to use the same mechanism in both our cases.
> 
> My very first idea to start discussion:
> (Below is kind of pseudo code only to show my idea. All defines like 64B or PTP are omitted for simplicity)
> 
> 1. Prepare appropriate structures:
> 
> struct macb_dma_desc {
> 	u32	addr;
> 	u32	ctrl;
> }
> 
> struct macb_dma_desc_64 {
> 	u32 addrh;
> 	u32 resvd;
> }
> 
> struct macb_dma_desc_ptp {
> 	u32 dma_desc_ts_1;
> 	u32	dma_desc_ts_2;
> }
> 
> 2. Add hardware support information to macb:
> 
> enum macb_hw_cap {
> 	HW_CAP_NONE,
> 	HW_CAP_64B,
> 	HW_CAP_PTP,
> 	HW_CAP_64B_PTP,
> };
> 
> struct macb {
> // (...)
> 	macb_hw_cap hw_cap;
> 
> }
> 
> 3. Set bp->hw_cap on macb_probe()
> 
> 4. Additional function might be helpful:
> 
> static unsigned char macb_dma_desc_get_mul(struct macb *bp)
> {
> 	switch (bp->hw_cap) {
> 		case HW_CAP_NONE:
> 			return 1;
> 		case HW_CAP_64B:
> 		case HW_CAP_PTP:
> 			return 2;
> 		case HW_CAP_64B_PTP:
> 			return 3;
> 	}
> }
> 
> 5. Change sizeof struct to function:
> (sizeof(struct macb_dma_desc) change to macb_dma_desc_get_size(bp). It will return sizeof(struct macb_dma_desc) * macb_dma_desc_get_mul(bp).
> There is a hidden assumption that all three structures have the same size.
> 
> 6. macb_rx_desc() and macb_tx_desc() will still return struct macb_dma_desc * but descriptor index will be counted in different way, ex.
> 
> static struct macb_dma_desc *macb_rx_desc(struct macb *bp, unsigned int index)
> {
> 	index *= macb_dma_desc_get_mul(bp);
> 	return &bp->rx_ring[macb_rx_ring_wrap(bp, index)];
> }
> 
> 7. Two additional functions to dereference PTP and 64b information:
> 
> static struct macb_dma_desc_64 *macb_64b_desc(struct macb *bp, struct macb_dma_desc *desc)
> {
> 	switch (bp->hw_cap) {
> 		case HW_CAP_64B:
> 		case HW_CAP_64B_PTP:
> 			return (struct macb_dma_desc_64 *)(desc + 1);  // ugly casting
> 		default:
> 			return NULL;
> 	}
> }
> 
> static struct macb_dma_desc_ptp *macb_ptp_desc(struct macb *bp, struct macb_dma_desc *desc)
> {
> 	switch (bp->hw_cap) {
> 		case HW_CAP_PTP:
> 			return (struct macb_dma_desc_ptp *)(desc + 1);
> 		case HW_CAP_64B_PTP:
> 			return (struct macb_dma_desc_ptp *)(desc + 2);
> 		default:
> 			return NULL;
> 	}
> }
> 
> Whenever you want to reach fields in appropriate descriptor, above function should be used.
> 
> This is only my very first idea. Of course, we can leave it as it is and say, that old hardware is not support either when PTP enabled or 64b enabled.
> 


-- 
Nicolas Ferre

^ permalink raw reply

* RE: [PATCH v9 0/8] thunderbolt: Introducing Thunderbolt(TM) Networking
From: Levy, Amir (Jer) @ 2016-11-18  8:48 UTC (permalink / raw)
  To: Simon Guinot
  Cc: gregkh@linuxfoundation.org, andreas.noever@gmail.com,
	bhelgaas@google.com, corbet@lwn.net, linux-kernel@vger.kernel.org,
	linux-pci@vger.kernel.org, netdev@vger.kernel.org,
	linux-doc@vger.kernel.org, mario_limonciello@dell.com,
	thunderbolt-linux, Westerberg, Mika, Winkler, Tomas,
	Zhang, Xiong Y, Jamet, Michael, remi.rerolle@seagate.com
In-Reply-To: <20161115105920.GH6167@kw.sim.vm.gnt>

On Tue, Nov 15 2016, 12:59 PM, Simon Guinot wrote:
> On Wed, Nov 09, 2016 at 03:42:53PM +0000, Levy, Amir (Jer) wrote:
> > On Wed, Nov 9 2016, 04:36 PM, Simon Guinot wrote:
> > > Hi Amir,
> > >
> > > I have an ASUS "All Series/Z87-DELUXE/QUAD" motherboard with a 
> > > Thunderbolt 2 "Falcon Ridge" chipset (device ID 156d).
> > >
> > > Is the thunderbolt-icm driver supposed to work with this chipset ?
> > >
> >
> > Yes, the thunderbolt-icm supports Falcon Ridge, device ID 156c.
> > 156d is the bridge -
> > http://lxr.free-electrons.com/source/include/linux/pci_ids.h#L2619
> >
> > > I have installed both a 4.8.6 Linux kernel (patched with your v9
> > > series) and the thunderbolt-software-daemon (27 october release) 
> > > inside a Debian system (Jessie).
> > >
> > > If I connect the ASUS motherboard with a MacBook Pro (Thunderbolt 
> > > 2, device ID 156c), I can see that the thunderbolt-icm driver is 
> > > loaded and that the thunderbolt-software-daemon is well started. 
> > > But the Ethernet interface is not created.
> > >
> > > I have attached to this email the syslog file. There is the logs 
> > > from both the kernel and the daemon inside. Note that the daemon 
> > > logs are everything but clear about what could be the issue. Maybe 
> > > I missed some kind of configuration ? But I failed to find any 
> > > valuable information about configuring the driver and/or the 
> > > daemon in
> the various documentation files.
> > >
> > > Please, can you provide some guidance ? I'd really like to test 
> > > your patch series.
> >
> > First, thank you very much for willing to test it.
> > Thunderbolt Networking support was added during Falcon Ridge, in the
> latest FR images.
> > Do you know which Thunderbolt image version you have on your system?
> > Currently I submitted only Thunderbolt Networking feature in Linux, 
> > and we plan to add more features like reading the image version and
> updating the image.
> > If you don't know the image version, the only thing I can suggest is 
> > to load windows, install thunderbolt SW and check in the Thunderbolt
> application the image version.
> > To know if image update is needed, you can check - 
> > https://thunderbolttechnology.net/updates
> 
> Hi Amir,
> 
> From the Windows Thunderbolt software, I can read 13.00 for the 
> firmware version. And from https://thunderbolttechnology.net/updates, 
> I can see that there is no update available for my ASUS motherboard.
> 
> Am I good to go ?
> 

Thunderbolt Networking is supported on both Thunderbolt(tm) 2 and Thunderbolt(tm) 3 systems.  
Thunderbolt 2 systems must have updated NVM (version 25 or later) in order for the functionality to work properly.  
If the system does not have the update, please contact the OEM directly for an updated NVM.  
For best functionality and support, Intel recommends using Thunderbolt 3 systems for all validation and testing.

> BTW, it is quite a shame that the Thunderbolt firmware version can't 
> be read from Linux.
> 

This is WIP, once this patch will be upstream, we will be able to focus more
on aligning Linux with the Thunderbolt features that we have for windows.

Regards,
Amir

^ permalink raw reply

* Re: [PATCH net-next 1/4] geneve: Unify LWT and netdev handling.
From: kbuild test robot @ 2016-11-18  8:40 UTC (permalink / raw)
  To: Pravin B Shelar; +Cc: kbuild-all, netdev, Pravin B Shelar
In-Reply-To: <1479453029-29619-2-git-send-email-pshelar@ovn.org>

[-- Attachment #1: Type: text/plain, Size: 2771 bytes --]

Hi Pravin,

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Pravin-B-Shelar/geneve-Use-LWT-more-effectively/20161118-151426
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   drivers/net/geneve.c: In function 'geneve_xmit':
>> drivers/net/geneve.c:942:10: warning: 'err' may be used uninitialized in this function [-Wmaybe-uninitialized]
     else if (err == -ENETUNREACH)
             ^

vim +/err +942 drivers/net/geneve.c

8ed66f0e John W. Linville 2015-10-26  926  	}
8eb3b995 Daniel Borkmann  2016-03-09  927  
e6efd144 Pravin B Shelar  2016-11-17  928  #if IS_ENABLED(CONFIG_IPV6)
e6efd144 Pravin B Shelar  2016-11-17  929  	if (info->mode & IP_TUNNEL_INFO_IPV6)
e6efd144 Pravin B Shelar  2016-11-17  930  		err = geneve6_xmit_skb(skb, dev, geneve, info);
e6efd144 Pravin B Shelar  2016-11-17  931  	else
e6efd144 Pravin B Shelar  2016-11-17  932  #endif
e6efd144 Pravin B Shelar  2016-11-17  933  		err = geneve_xmit_skb(skb, dev, geneve, info);
8ed66f0e John W. Linville 2015-10-26  934  
e6efd144 Pravin B Shelar  2016-11-17  935  	if (likely(!err))
e6efd144 Pravin B Shelar  2016-11-17  936  		return NETDEV_TX_OK;
8ed66f0e John W. Linville 2015-10-26  937  tx_error:
8ed66f0e John W. Linville 2015-10-26  938  	dev_kfree_skb(skb);
aed069df Alexander Duyck  2016-04-14  939  
8ed66f0e John W. Linville 2015-10-26  940  	if (err == -ELOOP)
8ed66f0e John W. Linville 2015-10-26  941  		dev->stats.collisions++;
8ed66f0e John W. Linville 2015-10-26 @942  	else if (err == -ENETUNREACH)
8ed66f0e John W. Linville 2015-10-26  943  		dev->stats.tx_carrier_errors++;
efeb2267 Haishuang Yan    2016-06-21  944  
8ed66f0e John W. Linville 2015-10-26  945  	dev->stats.tx_errors++;
8ed66f0e John W. Linville 2015-10-26  946  	return NETDEV_TX_OK;
8ed66f0e John W. Linville 2015-10-26  947  }
8ed66f0e John W. Linville 2015-10-26  948  
91572088 Jarod Wilson     2016-10-20  949  static int geneve_change_mtu(struct net_device *dev, int new_mtu)
55e5bfb5 David Wragg      2016-02-10  950  {

:::::: The code at line 942 was first introduced by commit
:::::: 8ed66f0e8235118a31720acdab3bbbe9debd0f6a geneve: implement support for IPv6-based tunnels

:::::: TO: John W. Linville <linville@tuxdriver.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 56934 bytes --]

^ permalink raw reply

* Re: [PATCH net-next v4 3/5] bus: mvebu-bus: Provide inline stub for mvebu_mbus_get_dram_win_info
From: Gregory CLEMENT @ 2016-11-18  8:32 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: netdev, davem, mw, arnd, Shaohui.Xie, andrew
In-Reply-To: <20161117191914.11077-4-f.fainelli@gmail.com>

Hi Florian,
 
 On jeu., nov. 17 2016, Florian Fainelli <f.fainelli@gmail.com> wrote:

> In preparation for allowing CONFIG_MVNETA_BM to build with COMPILE_TEST,
> provide an inline stub for mvebu_mbus_get_dram_win_info().

Actually the set of SoCs supporting mbus is more reduce than MVEBU. You
can have a look on 434cec62a6d7 ("bus: mvebu-mbus: Provide stub function
for mvebu_mbus_get_io_win_info()"), PLAT_ORION seems the good option.


Thanks,

Gregory

>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  include/linux/mbus.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/include/linux/mbus.h b/include/linux/mbus.h
> index 2931aa43dab1..0d3f14fd2621 100644
> --- a/include/linux/mbus.h
> +++ b/include/linux/mbus.h
> @@ -82,6 +82,7 @@ static inline int mvebu_mbus_get_io_win_info(phys_addr_t phyaddr, u32 *size,
>  }
>  #endif
>  
> +#ifdef CONFIG_MVEBU_MBUS
>  int mvebu_mbus_save_cpu_target(u32 __iomem *store_addr);
>  void mvebu_mbus_get_pcie_mem_aperture(struct resource *res);
>  void mvebu_mbus_get_pcie_io_aperture(struct resource *res);
> @@ -97,5 +98,12 @@ int mvebu_mbus_init(const char *soc, phys_addr_t mbus_phys_base,
>  		    size_t mbus_size, phys_addr_t sdram_phys_base,
>  		    size_t sdram_size);
>  int mvebu_mbus_dt_init(bool is_coherent);
> +#else
> +static inline int mvebu_mbus_get_dram_win_info(phys_addr_t phyaddr, u8 *target,
> +					       u8 *attr)
> +{
> +	return -EINVAL;
> +}
> +#endif /* CONFIG_MVEBU_MBUS */
>  
>  #endif /* __LINUX_MBUS_H */
> -- 
> 2.9.3
>

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply

* [PATCH 2/2] vhost: forbid IOTLB invalidation when not enabled
From: Jason Wang @ 2016-11-18  7:58 UTC (permalink / raw)
  To: mst, jasowang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <1479455920-3285-1-git-send-email-jasowang@redhat.com>

When IOTLB is not enabled, we should forbid IOTLB invalidation to
avoid a NULL pointer dereference.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/vhost.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index c6f2d89..7d338d5 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -959,6 +959,10 @@ int vhost_process_iotlb_msg(struct vhost_dev *dev,
 		vhost_iotlb_notify_vq(dev, msg);
 		break;
 	case VHOST_IOTLB_INVALIDATE:
+		if (!dev->iotlb) {
+			ret = -EFAULT;
+			break;
+		}
 		vhost_del_umem_range(dev->iotlb, msg->iova,
 				     msg->iova + msg->size - 1);
 		break;
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/2] vhost: remove unused feature bit
From: Jason Wang @ 2016-11-18  7:58 UTC (permalink / raw)
  To: mst, jasowang; +Cc: netdev, linux-kernel, kvm, virtualization

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 include/uapi/linux/vhost.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index 56b7ab5..60180c0 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -172,8 +172,6 @@ struct vhost_memory {
 #define VHOST_F_LOG_ALL 26
 /* vhost-net should add virtio_net_hdr for RX, and strip for TX packets. */
 #define VHOST_NET_F_VIRTIO_NET_HDR 27
-/* Vhost have device IOTLB */
-#define VHOST_F_DEVICE_IOTLB 63
 
 /* VHOST_SCSI specific definitions */
 
-- 
2.7.4

^ permalink raw reply related

* RE: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: Hayes Wang @ 2016-11-18  7:57 UTC (permalink / raw)
  To: Mark Lord, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
  Cc: nic_swsd, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <d683c019-4e0f-6fe6-368c-c4fc86c72fe6-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org>

Mark Lord [mailto:mlord-e+AXbWqSrlAAvxtiuMwx3w@public.gmane.org]
> Sent: Thursday, November 17, 2016 9:42 PM
[...]
> What the above sample shows, is the URB transfer buffer ran out of space in the
> middle
> of a packet, and the hardware then tried to just continue that same packet in the
> next URB,
> without an rx_desc header inserted.  The r8152.c driver always assumes the URB
> buffer begins
> with an rx_desc, so of course this behaviour produces really weird effects, and
> system crashes, etc..

The USB device wouldn't know the address and size of buffer. Only
the USB host controller knows. Therefore, the device sends the
data to host, and the host fills the memory. According to your
description, it seems the host splits the data from the device
into two different buffers (or URB transfers). I wonder if it would
occur. As far as I know, the host wouldn't allow the buffer size
less than the data length.

Our hw engineers need the log from the USB analyzer to confirm
what the device sends to the host. However, I don't think you
have USB analyzer to do this. I would try to reproduce the issue.
But, I am busy, so I don't think I would response quickly.

Besides, the maximum data length which the RTL8152 would send to
the host is 16KB. That is, if the agg_buf_sz is 16KB, the host
wouldn't split it. However, you still see problems for it.

[...]
> It is not clear to me how the chip decides when to forward an rx URB to the host.
> If you could describe how that part works for us, then it would help in further
> understanding why fast systems (eg. a PC) don't generally notice the issue,
> while much slower embedded systems do see the issue regularly.

The driver expects the rx buffer would be

	rx_desc + a packet + padding to 8 alignment + 
	rx_desc + a packet + padding to 8 alignment + ...
	
Therefore, when a urb transfer is completed, the driver parsers
the buffer by this way. After the buffer is handled, it would
be submitted to the host, until the transfer is completed again.
If the submitting fail, the driver would try again later. The
urb->actual_length means how much data the host fills. The drive
uses it to check the end of the data. The urb->status mean if
the transfer is successful. The driver submits the urb to the
host directly if the status is not successful.

Best Regards,
Hayes

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* pull-request: mac80211 2016-11-18
From: Johannes Berg @ 2016-11-18  7:52 UTC (permalink / raw)
  To: David Miller
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA

Hi Dave,

Due to travel/vacation, this is a bit late, but there aren't
that many fixes either. Most interesting/important are the
fixes from Felix and perhaps the scan entry limit.

Please pull and let me know if there's any problem.

Thanks,
johannes



The following changes since commit 269ebce4531b8edc4224259a02143181a1c1d77c:

  xen-netfront: cast grant table reference first to type int (2016-11-02 15:33:36 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git tags/mac80211-for-davem-2016-11-18

for you to fetch changes up to 9853a55ef1bb66d7411136046060bbfb69c714fa:

  cfg80211: limit scan results cache size (2016-11-18 08:44:44 +0100)

----------------------------------------------------------------
A few more bugfixes:
 * limit # of scan results stored in memory - this is a long-standing bug
   Jouni and I only noticed while discussing other things in Santa Fe
 * revert AP_LINK_PS patch that was causing issues (Felix)
 * various A-MSDU/A-MPDU fixes for TXQ code (Felix)
 * interoperability workaround for peers with broken VHT capabilities
   (Filip Matusiak)
 * add bitrate definition for a VHT MCS that's supposed to be invalid
   but gets used by some hardware anyway (Thomas Pedersen)
 * beacon timer fix in hwsim (Benjamin Beichler)

----------------------------------------------------------------
Benjamin Beichler (1):
      mac80211_hwsim: fix beacon delta calculation

Felix Fietkau (4):
      Revert "mac80211: allow using AP_LINK_PS with mac80211-generated TIM IE"
      mac80211: update A-MPDU flag on tx dequeue
      mac80211: remove bogus skb vif assignment
      mac80211: fix A-MSDU aggregation with fast-xmit + txq

Filip Matusiak (1):
      mac80211: Ignore VHT IE from peer with wrong rx_mcs_map

Johannes Berg (1):
      cfg80211: limit scan results cache size

Pedersen, Thomas (1):
      cfg80211: add bitrate for 20MHz MCS 9

 drivers/net/wireless/mac80211_hwsim.c |  2 +-
 net/mac80211/sta_info.c               |  2 +-
 net/mac80211/tx.c                     | 14 +++++--
 net/mac80211/vht.c                    | 16 ++++++++
 net/wireless/core.h                   |  1 +
 net/wireless/scan.c                   | 69 +++++++++++++++++++++++++++++++++++
 net/wireless/util.c                   |  3 +-
 7 files changed, 100 insertions(+), 7 deletions(-)

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox