On Thu, 11 Oct 2018 at 02:32, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
>
Hi Pablo,
Thank you for review!
> On Sat, Oct 06, 2018 at 01:42:42AM +0900, Taehee Yoo wrote:
> > diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c
> > index 2c8d313ae216..6ccabe6f74a6 100644
> > --- a/net/ipv4/netfilter/ipt_CLUSTERIP.c
> > +++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c
> > @@ -59,7 +59,6 @@ struct clusterip_config {
> > struct rcu_head rcu;
> >
> > char ifname[IFNAMSIZ]; /* device ifname */
> > - struct notifier_block notifier; /* refresh c->ifindex in it */
> > };
> >
> > #ifdef CONFIG_PROC_FS
> > @@ -118,8 +117,6 @@ clusterip_config_entry_put(struct net *net, struct clusterip_config *c)
> > spin_unlock(&cn->lock);
> > local_bh_enable();
> >
> > - unregister_netdevice_notifier(&c->notifier);
> > -
> > return;
> > }
> > local_bh_enable();
> > @@ -181,32 +178,37 @@ clusterip_netdev_event(struct notifier_block *this, unsigned long event,
> > void *ptr)
> > {
> > struct net_device *dev = netdev_notifier_info_to_dev(ptr);
> > + struct net *net = dev_net(dev);
> > + struct clusterip_net *cn = net_generic(net, clusterip_net_id);
> > struct clusterip_config *c;
> >
> > - c = container_of(this, struct clusterip_config, notifier);
> > - switch (event) {
> > - case NETDEV_REGISTER:
> > - if (!strcmp(dev->name, c->ifname)) {
> > - c->ifindex = dev->ifindex;
> > - dev_mc_add(dev, c->clustermac);
> > - }
> > - break;
> > - case NETDEV_UNREGISTER:
> > - if (dev->ifindex == c->ifindex) {
> > - dev_mc_del(dev, c->clustermac);
> > - c->ifindex = -1;
> > - }
> > - break;
> > - case NETDEV_CHANGENAME:
> > - if (!strcmp(dev->name, c->ifname)) {
> > - c->ifindex = dev->ifindex;
> > - dev_mc_add(dev, c->clustermac);
> > - } else if (dev->ifindex == c->ifindex) {
> > - dev_mc_del(dev, c->clustermac);
> > - c->ifindex = -1;
> > + spin_lock(&cn->lock);
>
> Do we need spin_lock_bh() here?
I checked that, you're right.
config is modified in the BH. so, that should be spin_lock_bh().
I will send v2 patch
Thanks!
