Re: [PATCH net-next] net: stmmac: Add support for U32 TC filter using Flexible RX Parser

[kbuild test robot]

Hi Jose,

I love your patch! Perhaps something to improve:

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

url:
https://github.com/0day-ci/linux/commits/Jose-Abreu/net-stmmac-Add-support-for-U32-TC-filter-using-Flexible-RX-Parser/20180504-164205
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c:104:14: sparse: incorrect 
>> type in assignment (different base types) @@expected unsigned int 
>> [unsigned] [usertype] data @@got ed int [unsigned] [usertype] data @@
   drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c:104:14:expected unsigned 
int [unsigned] [usertype] data
   drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c:104:14:got restricted 
__be32 [usertype] val
>> drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c:105:14: sparse: incorrect 
>> type in assignment (different base types) @@expected unsigned int 
>> [unsigned] [usertype] mask @@got ed int [unsigned] [usertype] mask @@
   drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c:105:14:expected unsigned 
int [unsigned] [usertype] mask
   drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c:105:14:got restricted 
__be32 [usertype] mask

vim +104 drivers/net/ethernet/stmicro/stmmac/stmmac_tc.c

89  
90  static int tc_fill_entry(struct stmmac_priv *priv,
91  struct tc_cls_u32_offload *cls)
92  {
93  struct stmmac_tc_entry *entry, *frag = NULL;
94  struct tc_u32_sel *sel = cls->knode.sel;
95  u32 off, data, mask, real_off, rem;
96  u32 prio = cls->common.prio;
97  int ret;
98  
99  /* Only 1 match per entry */
   100  if (sel->nkeys <= 0 || sel->nkeys > 1)
   101  return -EINVAL;
   102  
   103  off = sel->keys[0].off << sel->offshift;
 > 104  data = sel->keys[0].val;
 > 105  mask = sel->keys[0].mask;
   106  
   107  switch (ntohs(cls->common.protocol)) {
   108  case ETH_P_ALL:
   109  break;
   110  case ETH_P_IP:
   111  off += ETH_HLEN;
   112  break;
   113  default:
   114  return -EINVAL;
   115  }
   116  
   117  if (off > priv->tc_off_max)
   118  return -EINVAL;
   119  
   120  real_off = off / 4;
   121  rem = off % 4;
   122  
   123  entry = tc_find_entry(priv, cls, true);
   124  if (!entry)
   125  return -EINVAL;
   126  
   127  if (rem) {
   128  frag = tc_find_entry(priv, cls, true);
   129  if (!frag) {
   130  ret = -EINVAL;
   131  goto err_unuse;
   132  }
   133  
   134  entry->frag_ptr = frag;
   135  entry->val.match_en = (mask << (rem * 8)) &
   136  GENMASK(31, rem * 8);
   137  entry->val.match_data = (data << (rem * 8)) &
   138  GENMASK(31, rem * 8);
   139  entry->val.frame_offset = real_off;
   140  entry->prio = prio;
   141  
   142  frag->val.match_en = (mask >> (rem * 8)) &
   143  GENMASK(rem * 8 - 1, 0);
   144  frag->val.match_data = (data >> (rem * 8)) &
   145  GENMASK(rem * 8 - 1, 0);
   146  frag->val.frame_offset = real_off + 1;
   147  frag->prio = prio;
   148  frag->is_frag = true;
   149  } else {
   150  entry->frag_ptr = NULL;
   151  entry->val.match_en = mask;
   152  entry->val.match_data = data;
   153  entry->val.frame_offset = real_off;
   154  entry->prio = prio;
   155  }
   156  
   157  ret = tc_fill_actions(entry, frag, cls);
   158  if (ret)
   159  goto err_unuse;
   160  
   161  return 0;
   162  
   163  err_unuse:
   164  if (frag)
   165  frag->in_use = false;
   166  entry->in_use = false;
   167  return ret;
   168  }
   169  

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

Re: [PATCH net-next] net: stmmac: Add support for U32 TC filter using Flexible RX Parser

[Jakub Kicinski]

On Thu,  3 May 2018 13:45:30 +0100, Jose Abreu wrote:
> + case TC_SETUP_CLSU32:
> + if (!(priv->dev->hw_features & NETIF_F_HW_TC))
> + ret = -EOPNOTSUPP;
> + else
> + ret = stmmac_tc_setup_cls_u32(priv, priv, type_data);
> + break;

Please just use tc_cls_can_offload_and_chain0().  The extack message it
sets is useful for debugging and you don't support chains other than 0
either.

Re: [PATCH net-next] net: stmmac: Add support for U32 TC filter using Flexible RX Parser

[David Miller]

From: Jose Abreu 
Date: Thu,  3 May 2018 13:45:30 +0100

> +static int dwmac5_rxp_update_single_entry(void __iomem *ioaddr,
> + struct stmmac_tc_entry *entry, int pos)

Please follow the Linux networking coding style for function arguments
in function declarations and definitions.

Each second and subsequent line of the declaration/definition shall start
precisely at the column after the openning parenthesis of the first line.
Use the appropriate number of TAB then SPACE characters necessary to
achieve this.

Otherwise, this patch looks great.

Thanks.