Re: [PATCH net-next v2] bridge: mrp: Extend MRP netlink interface with IFLA_BRIDGE_MRP_CLEAR
From: Horatiu Vultur
Date: Thu Jun 25 2020 - 09:51:35 EST
The 06/25/2020 15:17, Nikolay Aleksandrov wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 25/06/2020 15:03, kernel test robot wrote:
> > Hi Horatiu,
> >
> > Thank you for the patch! Perhaps something to improve:
> >
>
Hi Nik,
> Hi,
> I think you should drop the __rcu tag for the mrp_list member and also
> from the "list" member of struct br_mrp to fix most of the below.
>
> Cheers,
> Nik
Thanks, your suggestion worked. I really had problems understanding
these sparse warnings.
I will move this patch in a patch series where I will fix also these
warnings.
>
> > [auto build test WARNING on net-next/master]
> >
> > url: https://github.com/0day-ci/linux/commits/Horatiu-Vultur/bridge-mrp-Extend-MRP-netlink-interface-with-IFLA_BRIDGE_MRP_CLEAR/20200625-150941
> > base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 147373d968f1c1b5d6bb71e4e8b7495eeb9cdcae
> > config: i386-randconfig-s001-20200624 (attached as .config)
> > compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
> > reproduce:
> > # apt-get install sparse
> > # sparse version: v0.6.2-dirty
> > # save the attached .config to linux build tree
> > make W=1 C=1 ARCH=i386 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@xxxxxxxxx>
> >
> >
> > sparse warnings: (new ones prefixed by >>)
> >
> > net/bridge/br_mrp.c:106:18: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned short [usertype] @@ got restricted __be16 [usertype] @@
> > net/bridge/br_mrp.c:106:18: sparse: expected unsigned short [usertype]
> > net/bridge/br_mrp.c:106:18: sparse: got restricted __be16 [usertype]
> > net/bridge/br_mrp.c:281:23: sparse: sparse: incorrect type in argument 1 (different modifiers) @@ expected struct list_head *entry @@ got struct list_head [noderef] * @@
> > net/bridge/br_mrp.c:281:23: sparse: expected struct list_head *entry
> > net/bridge/br_mrp.c:281:23: sparse: got struct list_head [noderef] *
> > net/bridge/br_mrp.c:332:28: sparse: sparse: incorrect type in argument 1 (different modifiers) @@ expected struct list_head *new @@ got struct list_head [noderef] * @@
> > net/bridge/br_mrp.c:332:28: sparse: expected struct list_head *new
> > net/bridge/br_mrp.c:332:28: sparse: got struct list_head [noderef] *
> > net/bridge/br_mrp.c:332:40: sparse: sparse: incorrect type in argument 2 (different modifiers) @@ expected struct list_head *head @@ got struct list_head [noderef] * @@
> > net/bridge/br_mrp.c:332:40: sparse: expected struct list_head *head
> > net/bridge/br_mrp.c:332:40: sparse: got struct list_head [noderef] *
> > net/bridge/br_mrp.c:691:29: sparse: sparse: incorrect type in argument 1 (different modifiers) @@ expected struct list_head const *head @@ got struct list_head [noderef] * @@
> > net/bridge/br_mrp.c:691:29: sparse: expected struct list_head const *head
> > net/bridge/br_mrp.c:691:29: sparse: got struct list_head [noderef] *
> >>> net/bridge/br_mrp.c:383:9: sparse: sparse: dereference of noderef expression
> >>> net/bridge/br_mrp.c:383:9: sparse: sparse: dereference of noderef expression
> >>> net/bridge/br_mrp.c:383:9: sparse: sparse: dereference of noderef expression
> >
> > vim +383 net/bridge/br_mrp.c
> >
> > 284
> > 285 /* Adds a new MRP instance.
> > 286 * note: called under rtnl_lock
> > 287 */
> > 288 int br_mrp_add(struct net_bridge *br, struct br_mrp_instance *instance)
> > 289 {
> > 290 struct net_bridge_port *p;
> > 291 struct br_mrp *mrp;
> > 292 int err;
> > 293
> > 294 /* If the ring exists, it is not possible to create another one with the
> > 295 * same ring_id
> > 296 */
> > 297 mrp = br_mrp_find_id(br, instance->ring_id);
> > 298 if (mrp)
> > 299 return -EINVAL;
> > 300
> > 301 if (!br_mrp_get_port(br, instance->p_ifindex) ||
> > 302 !br_mrp_get_port(br, instance->s_ifindex))
> > 303 return -EINVAL;
> > 304
> > 305 /* It is not possible to have the same port part of multiple rings */
> > 306 if (!br_mrp_unique_ifindex(br, instance->p_ifindex) ||
> > 307 !br_mrp_unique_ifindex(br, instance->s_ifindex))
> > 308 return -EINVAL;
> > 309
> > 310 mrp = kzalloc(sizeof(*mrp), GFP_KERNEL);
> > 311 if (!mrp)
> > 312 return -ENOMEM;
> > 313
> > 314 mrp->ring_id = instance->ring_id;
> > 315 mrp->prio = instance->prio;
> > 316
> > 317 p = br_mrp_get_port(br, instance->p_ifindex);
> > 318 spin_lock_bh(&br->lock);
> > 319 p->state = BR_STATE_FORWARDING;
> > 320 p->flags |= BR_MRP_AWARE;
> > 321 spin_unlock_bh(&br->lock);
> > 322 rcu_assign_pointer(mrp->p_port, p);
> > 323
> > 324 p = br_mrp_get_port(br, instance->s_ifindex);
> > 325 spin_lock_bh(&br->lock);
> > 326 p->state = BR_STATE_FORWARDING;
> > 327 p->flags |= BR_MRP_AWARE;
> > 328 spin_unlock_bh(&br->lock);
> > 329 rcu_assign_pointer(mrp->s_port, p);
> > 330
> > 331 INIT_DELAYED_WORK(&mrp->test_work, br_mrp_test_work_expired);
> > > 332 list_add_tail_rcu(&mrp->list, &br->mrp_list);
> > 333
> > 334 err = br_mrp_switchdev_add(br, mrp);
> > 335 if (err)
> > 336 goto delete_mrp;
> > 337
> > 338 return 0;
> > 339
> > 340 delete_mrp:
> > 341 br_mrp_del_impl(br, mrp);
> > 342
> > 343 return err;
> > 344 }
> > 345
> > 346 /* Deletes the MRP instance from which the port is part of
> > 347 * note: called under rtnl_lock
> > 348 */
> > 349 void br_mrp_port_del(struct net_bridge *br, struct net_bridge_port *p)
> > 350 {
> > 351 struct br_mrp *mrp = br_mrp_find_port(br, p);
> > 352
> > 353 /* If the port is not part of a MRP instance just bail out */
> > 354 if (!mrp)
> > 355 return;
> > 356
> > 357 br_mrp_del_impl(br, mrp);
> > 358 }
> > 359
> > 360 /* Deletes existing MRP instance based on ring_id
> > 361 * note: called under rtnl_lock
> > 362 */
> > 363 int br_mrp_del(struct net_bridge *br, struct br_mrp_instance *instance)
> > 364 {
> > 365 struct br_mrp *mrp = br_mrp_find_id(br, instance->ring_id);
> > 366
> > 367 if (!mrp)
> > 368 return -EINVAL;
> > 369
> > 370 br_mrp_del_impl(br, mrp);
> > 371
> > 372 return 0;
> > 373 }
> > 374
> > 375 /* Deletes all MRP instances on the bridge
> > 376 * note: called under rtnl_lock
> > 377 */
> > 378 int br_mrp_clear(struct net_bridge *br)
> > 379 {
> > 380 struct br_mrp *mrp;
> > 381 struct br_mrp *tmp;
> > 382
> > > 383 list_for_each_entry_safe(mrp, tmp, &br->mrp_list, list) {
> > 384 br_mrp_del_impl(br, mrp);
> > 385 }
> > 386
> > 387 return 0;
> > 388 }
> > 389
> >
> > ---
> > 0-DAY CI Kernel Test Service, Intel Corporation
> > https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx
> >
>
--
/Horatiu