Re: [PATCH net-next v2] bridge: mrp: Extend MRP netlink interface with IFLA_BRIDGE_MRP_CLEAR

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:

quoted

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.

quoted

[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 <redacted>


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] *

quoted

quoted

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@lists.01.org

-- 
/Horatiu