* [PATCH net-next 7/8] mld: convert ip6_sf_socklist to list macros
@ 2021-02-08 17:58 Taehee Yoo
2021-02-08 18:31 ` Eric Dumazet
2021-02-09 1:55 ` kernel test robot
0 siblings, 2 replies; 9+ messages in thread
From: Taehee Yoo @ 2021-02-08 17:58 UTC (permalink / raw)
To: davem, kuba, netdev, dsahern, xiyou.wangcong; +Cc: ap420073
Currently, struct ip6_sf_socklist doesn't use list API so that code
shape is a little bit different from others.
So it converts ip6_sf_socklist to use list API so it would
improve readability.
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
include/net/if_inet6.h | 19 +-
include/uapi/linux/in.h | 4 +-
net/ipv6/mcast.c | 387 +++++++++++++++++++++++++---------------
3 files changed, 256 insertions(+), 154 deletions(-)
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index babf19c27b29..6885ab8ec2e9 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -13,6 +13,7 @@
#include <net/snmp.h>
#include <linux/ipv6.h>
#include <linux/refcount.h>
+#include <linux/types.h>
/* inet6_dev.if_flags */
@@ -76,23 +77,19 @@ struct inet6_ifaddr {
};
struct ip6_sf_socklist {
- unsigned int sl_max;
- unsigned int sl_count;
- struct in6_addr sl_addr[];
+ struct list_head list;
+ struct in6_addr sl_addr;
+ struct rcu_head rcu;
};
-#define IP6_SFLSIZE(count) (sizeof(struct ip6_sf_socklist) + \
- (count) * sizeof(struct in6_addr))
-
-#define IP6_SFBLOCK 10 /* allocate this many at once */
-
struct ipv6_mc_socklist {
struct in6_addr addr;
int ifindex;
- unsigned int sfmode; /* MCAST_{INCLUDE,EXCLUDE} */
+ bool sfmode; /* MCAST_{INCLUDE,EXCLUDE} */
struct list_head list;
+ struct list_head sflist;
rwlock_t sflock;
- struct ip6_sf_socklist *sflist;
+ atomic_t sl_count;
struct rcu_head rcu;
};
@@ -101,7 +98,7 @@ struct ip6_sf_list {
struct in6_addr sf_addr;
unsigned long sf_count[2]; /* include/exclude counts */
unsigned char sf_gsresp; /* include in g & s response? */
- unsigned char sf_oldin; /* change state */
+ bool sf_oldin; /* change state */
unsigned char sf_crcount; /* retrans. left to send */
};
diff --git a/include/uapi/linux/in.h b/include/uapi/linux/in.h
index 7d6687618d80..97024873afd0 100644
--- a/include/uapi/linux/in.h
+++ b/include/uapi/linux/in.h
@@ -160,8 +160,8 @@ struct in_addr {
#define IP_MULTICAST_ALL 49
#define IP_UNICAST_IF 50
-#define MCAST_EXCLUDE 0
-#define MCAST_INCLUDE 1
+#define MCAST_EXCLUDE false
+#define MCAST_INCLUDE true
/* These need to appear somewhere around here */
#define IP_DEFAULT_MULTICAST_TTL 1
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index f4fc29fcdf48..45b683b15835 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -80,13 +80,18 @@ static int sf_setstate(struct ifmcaddr6 *mc);
static void sf_markstate(struct ifmcaddr6 *mc);
static void ip6_mc_clear_src(struct ifmcaddr6 *mc);
static int ip6_mc_del_src(struct inet6_dev *idev, const struct in6_addr *mca,
- int sfmode, int sfcount, const struct in6_addr *psfsrc,
- int delta);
+ int sfmode, const struct in6_addr *psfsrc, int delta);
+static void ip6_mc_del_src_bulk(struct inet6_dev *idev,
+ struct ipv6_mc_socklist *mc_lst,
+ struct sock *sk);
static int ip6_mc_add_src(struct inet6_dev *idev, const struct in6_addr *mca,
- int sfmode, int sfcount, const struct in6_addr *psfsrc,
- int delta);
-static int ip6_mc_leave_src(struct sock *sk, struct ipv6_mc_socklist *mc_lst,
- struct inet6_dev *idev);
+ int sfmode, const struct in6_addr *psfsrc, int delta);
+static int ip6_mc_add_src_bulk(struct inet6_dev *idev, struct group_filter *gsf,
+ struct list_head *head,
+ struct sockaddr_storage *list,
+ struct sock *sk);
+static void ip6_mc_leave_src(struct sock *sk, struct ipv6_mc_socklist *mc_lst,
+ struct inet6_dev *idev);
static int __ipv6_dev_mc_inc(struct net_device *dev,
const struct in6_addr *addr, unsigned int mode);
@@ -178,8 +183,9 @@ static int __ipv6_sock_mc_join(struct sock *sk, int ifindex,
mc_lst->ifindex = dev->ifindex;
mc_lst->sfmode = mode;
+ atomic_set(&mc_lst->sl_count, 0);
rwlock_init(&mc_lst->sflock);
- mc_lst->sflist = NULL;
+ INIT_LIST_HEAD(&mc_lst->sflist);
/*
* now add/increase the group membership on the device
@@ -334,7 +340,6 @@ int ip6_mc_source(int add, int omode, struct sock *sk,
int leavegroup = 0;
bool found = false;
int mclocked = 0;
- int i, j, rv;
int err;
source = &((struct sockaddr_in6 *)&pgsr->gsr_source)->sin6_addr;
@@ -365,89 +370,70 @@ int ip6_mc_source(int add, int omode, struct sock *sk,
goto done;
}
/* if a source filter was set, must be the same mode as before */
- if (mc_lst->sflist) {
+ if (!list_empty(&mc_lst->sflist)) {
if (mc_lst->sfmode != omode) {
err = -EINVAL;
goto done;
}
} else if (mc_lst->sfmode != omode) {
/* allow mode switches for empty-set filters */
- ip6_mc_add_src(idev, group, omode, 0, NULL, 0);
- ip6_mc_del_src(idev, group, mc_lst->sfmode, 0, NULL, 0);
+ ip6_mc_add_src(idev, group, omode, NULL, 0);
+ ip6_mc_del_src(idev, group, mc_lst->sfmode, NULL, 0);
mc_lst->sfmode = omode;
}
write_lock(&mc_lst->sflock);
mclocked = 1;
- psl = mc_lst->sflist;
if (!add) {
- if (!psl)
- goto done; /* err = -EADDRNOTAVAIL */
- rv = !0;
- for (i = 0; i < psl->sl_count; i++) {
- rv = !ipv6_addr_equal(&psl->sl_addr[i], source);
- if (rv == 0)
+ found = false;
+ list_for_each_entry(psl, &mc_lst->sflist, list) {
+ if (ipv6_addr_equal(&psl->sl_addr, source)) {
+ found = true;
break;
+ }
}
- if (rv) /* source not found */
+ if (!found)
goto done; /* err = -EADDRNOTAVAIL */
/* special case - (INCLUDE, empty) == LEAVE_GROUP */
- if (psl->sl_count == 1 && omode == MCAST_INCLUDE) {
+ if (atomic_read(&mc_lst->sl_count) == 1 &&
+ omode == MCAST_INCLUDE) {
leavegroup = 1;
goto done;
}
/* update the interface filter */
- ip6_mc_del_src(idev, group, omode, 1, source, 1);
+ ip6_mc_del_src(idev, group, omode, &psl->sl_addr, 1);
- for (j = i+1; j < psl->sl_count; j++)
- psl->sl_addr[j-1] = psl->sl_addr[j];
- psl->sl_count--;
+ list_del_rcu(&psl->list);
+ atomic_dec(&mc_lst->sl_count);
err = 0;
goto done;
}
/* else, add a new source to the filter */
- if (psl && psl->sl_count >= sysctl_mld_max_msf) {
+ if (atomic_read(&mc_lst->sl_count) >= sysctl_mld_max_msf) {
err = -ENOBUFS;
goto done;
}
- if (!psl || psl->sl_count == psl->sl_max) {
- struct ip6_sf_socklist *newpsl;
- int count = IP6_SFBLOCK;
- if (psl)
- count += psl->sl_max;
- newpsl = sock_kmalloc(sk, IP6_SFLSIZE(count), GFP_ATOMIC);
- if (!newpsl) {
- err = -ENOBUFS;
- goto done;
- }
- newpsl->sl_max = count;
- newpsl->sl_count = count - IP6_SFBLOCK;
- if (psl) {
- for (i = 0; i < psl->sl_count; i++)
- newpsl->sl_addr[i] = psl->sl_addr[i];
- sock_kfree_s(sk, psl, IP6_SFLSIZE(psl->sl_max));
- }
- psl = newpsl;
- mc_lst->sflist = psl;
- }
- rv = 1; /* > 0 for insert logic below if sl_count is 0 */
- for (i = 0; i < psl->sl_count; i++) {
- rv = !ipv6_addr_equal(&psl->sl_addr[i], source);
- if (rv == 0) /* There is an error in the address. */
+ list_for_each_entry(psl, &mc_lst->sflist, list)
+ if (ipv6_addr_equal(&psl->sl_addr, source))
goto done;
+
+ psl = sock_kmalloc(sk, sizeof(struct ip6_sf_socklist), GFP_ATOMIC);
+ if (!psl) {
+ err = -ENOBUFS;
+ goto done;
}
- for (j = psl->sl_count-1; j >= i; j--)
- psl->sl_addr[j+1] = psl->sl_addr[j];
- psl->sl_addr[i] = *source;
- psl->sl_count++;
+ atomic_inc(&mc_lst->sl_count);
+ psl->sl_addr = *source;
+ list_add_rcu(&psl->list, &mc_lst->sflist);
+
err = 0;
/* update the interface list */
- ip6_mc_add_src(idev, group, omode, 1, source, 1);
+ ip6_mc_add_src(idev, group, omode, &psl->sl_addr, 1);
done:
if (mclocked)
write_unlock(&mc_lst->sflock);
@@ -462,14 +448,14 @@ int ip6_mc_msfilter(struct sock *sk, struct group_filter *gsf,
struct sockaddr_storage *list)
{
struct ipv6_pinfo *inet6 = inet6_sk(sk);
- struct ip6_sf_socklist *newpsl, *psl;
struct ipv6_mc_socklist *mc_lst;
struct net *net = sock_net(sk);
const struct in6_addr *group;
struct inet6_dev *idev;
int leavegroup = 0;
bool found = false;
- int i, err;
+ LIST_HEAD(head);
+ int err;
group = &((struct sockaddr_in6 *)&gsf->gf_group)->sin6_addr;
@@ -506,40 +492,19 @@ int ip6_mc_msfilter(struct sock *sk, struct group_filter *gsf,
err = -EINVAL;
goto done;
}
- if (gsf->gf_numsrc) {
- newpsl = sock_kmalloc(sk, IP6_SFLSIZE(gsf->gf_numsrc),
- GFP_ATOMIC);
- if (!newpsl) {
- err = -ENOBUFS;
- goto done;
- }
- newpsl->sl_max = newpsl->sl_count = gsf->gf_numsrc;
- for (i = 0; i < newpsl->sl_count; ++i, ++list) {
- struct sockaddr_in6 *psin6;
- psin6 = (struct sockaddr_in6 *)list;
- newpsl->sl_addr[i] = psin6->sin6_addr;
- }
- err = ip6_mc_add_src(idev, group, gsf->gf_fmode,
- newpsl->sl_count, newpsl->sl_addr, 0);
- if (err) {
- sock_kfree_s(sk, newpsl, IP6_SFLSIZE(newpsl->sl_max));
- goto done;
- }
- } else {
- newpsl = NULL;
- ip6_mc_add_src(idev, group, gsf->gf_fmode, 0, NULL, 0);
- }
+ if (gsf->gf_numsrc)
+ err = ip6_mc_add_src_bulk(idev, gsf, &head, list, sk);
+ else
+ err = ip6_mc_add_src(idev, group, gsf->gf_fmode, NULL, 0);
+
+ if (err)
+ goto done;
write_lock(&mc_lst->sflock);
- psl = mc_lst->sflist;
- if (psl) {
- ip6_mc_del_src(idev, group, mc_lst->sfmode,
- psl->sl_count, psl->sl_addr, 0);
- sock_kfree_s(sk, psl, IP6_SFLSIZE(psl->sl_max));
- } else
- ip6_mc_del_src(idev, group, mc_lst->sfmode, 0, NULL, 0);
- mc_lst->sflist = newpsl;
+ ip6_mc_del_src_bulk(idev, mc_lst, sk);
+ atomic_set(&mc_lst->sl_count, gsf->gf_numsrc);
+ list_splice(&head, &mc_lst->sflist);
mc_lst->sfmode = gsf->gf_fmode;
write_unlock(&mc_lst->sflock);
err = 0;
@@ -548,6 +513,7 @@ int ip6_mc_msfilter(struct sock *sk, struct group_filter *gsf,
rcu_read_unlock();
if (leavegroup)
err = ipv6_sock_mc_drop(sk, gsf->gf_interface, group);
+
return err;
}
@@ -557,11 +523,11 @@ int ip6_mc_msfget(struct sock *sk, struct group_filter *gsf,
struct ipv6_pinfo *inet6 = inet6_sk(sk);
struct ipv6_mc_socklist *mc_lst;
struct net *net = sock_net(sk);
- int err, i, count, copycount;
const struct in6_addr *group;
struct ip6_sf_socklist *psl;
struct inet6_dev *idev;
bool found = false;
+ int err, i;
group = &((struct sockaddr_in6 *)&gsf->gf_group)->sin6_addr;
@@ -593,27 +559,31 @@ int ip6_mc_msfget(struct sock *sk, struct group_filter *gsf,
if (!found) /* must have a prior join */
goto done;
gsf->gf_fmode = mc_lst->sfmode;
- psl = mc_lst->sflist;
- count = psl ? psl->sl_count : 0;
read_unlock_bh(&idev->lock);
rcu_read_unlock();
- copycount = count < gsf->gf_numsrc ? count : gsf->gf_numsrc;
- gsf->gf_numsrc = count;
- /* changes to psl require the socket lock, and a write lock
- * on mc_lst->sflock. We have the socket lock so reading here is safe.
- */
- for (i = 0; i < copycount; i++, p++) {
+ i = 0;
+ read_lock(&mc_lst->sflock);
+ list_for_each_entry(psl, &mc_lst->sflist, list) {
struct sockaddr_in6 *psin6;
struct sockaddr_storage ss;
+ if (i >= gsf->gf_numsrc)
+ break;
+
psin6 = (struct sockaddr_in6 *)&ss;
memset(&ss, 0, sizeof(ss));
psin6->sin6_family = AF_INET6;
- psin6->sin6_addr = psl->sl_addr[i];
- if (copy_to_user(p, &ss, sizeof(ss)))
+ psin6->sin6_addr = psl->sl_addr;
+ if (copy_to_user(p, &ss, sizeof(ss))) {
+ read_unlock(&mc_lst->sflock);
return -EFAULT;
+ }
+ p++;
+ i++;
}
+ gsf->gf_numsrc = i;
+ read_unlock(&mc_lst->sflock);
return 0;
done:
read_unlock_bh(&idev->lock);
@@ -641,19 +611,20 @@ bool inet6_mc_check(struct sock *sk, const struct in6_addr *mc_addr,
return np->mc_all;
}
read_lock(&mc_lst->sflock);
- psl = mc_lst->sflist;
- if (!psl) {
+
+ found = false;
+ if (list_empty(&mc_lst->sflist)) {
rv = mc_lst->sfmode == MCAST_EXCLUDE;
} else {
- int i;
-
- for (i = 0; i < psl->sl_count; i++) {
- if (ipv6_addr_equal(&psl->sl_addr[i], src_addr))
+ list_for_each_entry_rcu(psl, &mc_lst->sflist, list) {
+ if (ipv6_addr_equal(&psl->sl_addr, src_addr)) {
+ found = true;
break;
+ }
}
- if (mc_lst->sfmode == MCAST_INCLUDE && i >= psl->sl_count)
+ if (mc_lst->sfmode == MCAST_INCLUDE && !found)
rv = false;
- if (mc_lst->sfmode == MCAST_EXCLUDE && i < psl->sl_count)
+ if (mc_lst->sfmode == MCAST_EXCLUDE && found)
rv = false;
}
read_unlock(&mc_lst->sflock);
@@ -900,7 +871,7 @@ static int __ipv6_dev_mc_inc(struct net_device *dev,
if (ipv6_addr_equal(&mc->mca_addr, addr)) {
mc->mca_users++;
write_unlock_bh(&idev->lock);
- ip6_mc_add_src(idev, &mc->mca_addr, mode, 0, NULL, 0);
+ ip6_mc_add_src(idev, &mc->mca_addr, mode, NULL, 0);
in6_dev_put(idev);
return 0;
}
@@ -2171,16 +2142,16 @@ static int ip6_mc_del1_src(struct ifmcaddr6 *mc, int sfmode,
}
static int ip6_mc_del_src(struct inet6_dev *idev, const struct in6_addr *mca,
- int sfmode, int sfcount, const struct in6_addr *psfsrc,
- int delta)
+ int sfmode, const struct in6_addr *psfsrc, int delta)
{
struct ifmcaddr6 *mc;
bool found = false;
int changerec = 0;
- int i, err;
+ int i, err, rv;
if (!idev)
return -ENODEV;
+
read_lock_bh(&idev->lock);
list_for_each_entry(mc, &idev->mc_list, list) {
if (ipv6_addr_equal(mca, &mc->mca_addr)) {
@@ -2204,13 +2175,16 @@ static int ip6_mc_del_src(struct inet6_dev *idev, const struct in6_addr *mca,
mc->mca_sfcount[sfmode]--;
}
err = 0;
- for (i = 0; i < sfcount; i++) {
- int rv = ip6_mc_del1_src(mc, sfmode, &psfsrc[i]);
+ i = 0;
+
+ if (psfsrc) {
+ rv = ip6_mc_del1_src(mc, sfmode, psfsrc);
changerec |= rv > 0;
if (!err && rv < 0)
err = rv;
}
+
if (mc->mca_sfmode == MCAST_EXCLUDE &&
mc->mca_sfcount[MCAST_EXCLUDE] == 0 &&
mc->mca_sfcount[MCAST_INCLUDE]) {
@@ -2231,6 +2205,71 @@ static int ip6_mc_del_src(struct inet6_dev *idev, const struct in6_addr *mca,
return err;
}
+static void ip6_mc_del_src_bulk(struct inet6_dev *idev,
+ struct ipv6_mc_socklist *mc_lst,
+ struct sock *sk)
+{
+ struct in6_addr *mca = &mc_lst->addr;
+ struct ip6_sf_socklist *psl, *tmp;
+ int sfmode = mc_lst->sfmode;
+ struct ifmcaddr6 *mc;
+ bool found = false;
+ int changerec = 0;
+ int i, rv;
+
+ if (!idev)
+ return;
+
+ read_lock_bh(&idev->lock);
+ list_for_each_entry(mc, &idev->mc_list, list) {
+ if (ipv6_addr_equal(mca, &mc->mca_addr)) {
+ found = true;
+ break;
+ }
+ }
+ if (!found) {
+ /* MCA not found?? bug */
+ read_unlock_bh(&idev->lock);
+ return;
+ }
+ spin_lock_bh(&mc->mca_lock);
+ sf_markstate(mc);
+ if (!mc->mca_sfcount[sfmode]) {
+ spin_unlock_bh(&mc->mca_lock);
+ read_unlock_bh(&idev->lock);
+ return;
+ }
+ mc->mca_sfcount[sfmode]--;
+ i = 0;
+
+ list_for_each_entry_safe(psl, tmp, &mc_lst->sflist, list) {
+ rv = ip6_mc_del1_src(mc, sfmode, &psl->sl_addr);
+ list_del_rcu(&psl->list);
+ atomic_sub(sizeof(*psl), &sk->sk_omem_alloc);
+ kfree_rcu(psl, rcu);
+
+ changerec |= rv > 0;
+ }
+
+ if (mc->mca_sfmode == MCAST_EXCLUDE &&
+ mc->mca_sfcount[MCAST_EXCLUDE] == 0 &&
+ mc->mca_sfcount[MCAST_INCLUDE]) {
+ struct ip6_sf_list *psf;
+
+ /* filter mode change */
+ mc->mca_sfmode = MCAST_INCLUDE;
+ mc->mca_crcount = idev->mc_qrv;
+ idev->mc_ifc_count = mc->mca_crcount;
+ list_for_each_entry(psf, &mc->mca_source_list, list)
+ psf->sf_crcount = 0;
+ mld_ifc_event(mc->idev);
+ } else if (sf_setstate(mc) || changerec) {
+ mld_ifc_event(mc->idev);
+ }
+ spin_unlock_bh(&mc->mca_lock);
+ read_unlock_bh(&idev->lock);
+}
+
/*
* Add multicast single-source filter to the interface list
*/
@@ -2353,14 +2392,14 @@ static int sf_setstate(struct ifmcaddr6 *mc)
/*
* Add multicast source filter list to the interface list
*/
+
static int ip6_mc_add_src(struct inet6_dev *idev, const struct in6_addr *mca,
- int sfmode, int sfcount, const struct in6_addr *psfsrc,
- int delta)
+ int sfmode, const struct in6_addr *psfsrc, int delta)
{
struct ifmcaddr6 *mc;
bool found = false;
int isexclude;
- int i, err;
+ int err = 0;
if (!idev)
return -ENODEV;
@@ -2383,19 +2422,99 @@ static int ip6_mc_add_src(struct inet6_dev *idev, const struct in6_addr *mca,
isexclude = mc->mca_sfmode == MCAST_EXCLUDE;
if (!delta)
mc->mca_sfcount[sfmode]++;
- err = 0;
- for (i = 0; i < sfcount; i++) {
- err = ip6_mc_add1_src(mc, sfmode, &psfsrc[i]);
+
+ if (psfsrc)
+ err = ip6_mc_add1_src(mc, sfmode, psfsrc);
+
+ if (err) {
+ if (!delta)
+ mc->mca_sfcount[sfmode]--;
+ } else if (isexclude != (mc->mca_sfcount[MCAST_EXCLUDE] != 0)) {
+ struct ip6_sf_list *psf;
+
+ /* filter mode change */
+ if (mc->mca_sfcount[MCAST_EXCLUDE])
+ mc->mca_sfmode = MCAST_EXCLUDE;
+ else if (mc->mca_sfcount[MCAST_INCLUDE])
+ mc->mca_sfmode = MCAST_INCLUDE;
+ /* else no filters; keep old mode for reports */
+
+ mc->mca_crcount = idev->mc_qrv;
+ idev->mc_ifc_count = mc->mca_crcount;
+ list_for_each_entry(psf, &mc->mca_source_list, list)
+ psf->sf_crcount = 0;
+ mld_ifc_event(idev);
+ } else if (sf_setstate(mc)) {
+ mld_ifc_event(idev);
+ }
+
+ spin_unlock_bh(&mc->mca_lock);
+ read_unlock_bh(&idev->lock);
+ return err;
+}
+
+static int ip6_mc_add_src_bulk(struct inet6_dev *idev, struct group_filter *gsf,
+ struct list_head *head,
+ struct sockaddr_storage *list,
+ struct sock *sk)
+{
+ struct ip6_sf_socklist *psl, *tmp;
+ const struct in6_addr *group;
+ int sfmode = gsf->gf_fmode;
+ struct ifmcaddr6 *mc;
+ bool found = false;
+ int isexclude;
+ int i, err = 0;
+
+ group = &((struct sockaddr_in6 *)&gsf->gf_group)->sin6_addr;
+
+ if (!idev)
+ return -ENODEV;
+
+ list_for_each_entry(mc, &idev->mc_list, list) {
+ if (ipv6_addr_equal(group, &mc->mca_addr)) {
+ found = true;
+ break;
+ }
+ }
+ if (!found) {
+ /* MCA not found?? bug */
+ return -ESRCH;
+ }
+ spin_lock_bh(&mc->mca_lock);
+
+ sf_markstate(mc);
+ isexclude = mc->mca_sfmode == MCAST_EXCLUDE;
+ mc->mca_sfcount[sfmode]++;
+
+ for (i = 0; i < gsf->gf_numsrc; i++, ++list) {
+ struct sockaddr_in6 *psin6;
+
+ psl = sock_kmalloc(sk, sizeof(struct ip6_sf_socklist),
+ GFP_ATOMIC);
+ if (!psl) {
+ err = -ENOBUFS;
+ break;
+ }
+ INIT_LIST_HEAD(&psl->list);
+ psin6 = (struct sockaddr_in6 *)list;
+ psl->sl_addr = psin6->sin6_addr;
+
+ err = ip6_mc_add1_src(mc, gsf->gf_fmode, &psl->sl_addr);
if (err)
break;
+
+ list_add_tail(&psl->list, head);
}
+
if (err) {
- int j;
+ mc->mca_sfcount[sfmode]--;
- if (!delta)
- mc->mca_sfcount[sfmode]--;
- for (j = 0; j < i; j++)
- ip6_mc_del1_src(mc, sfmode, &psfsrc[j]);
+ list_for_each_entry_safe(psl, tmp, head, list) {
+ list_del(&psl->list);
+ atomic_sub(sizeof(*psl), &sk->sk_omem_alloc);
+ kfree(psl);
+ }
} else if (isexclude != (mc->mca_sfcount[MCAST_EXCLUDE] != 0)) {
struct ip6_sf_list *psf;
@@ -2414,7 +2533,7 @@ static int ip6_mc_add_src(struct inet6_dev *idev, const struct in6_addr *mca,
} else if (sf_setstate(mc))
mld_ifc_event(idev);
spin_unlock_bh(&mc->mca_lock);
- read_unlock_bh(&idev->lock);
+
return err;
}
@@ -2462,26 +2581,12 @@ static void mld_join_group(struct ifmcaddr6 *mc)
spin_unlock_bh(&mc->mca_lock);
}
-static int ip6_mc_leave_src(struct sock *sk, struct ipv6_mc_socklist *mc_lst,
- struct inet6_dev *idev)
+static void ip6_mc_leave_src(struct sock *sk, struct ipv6_mc_socklist *mc_lst,
+ struct inet6_dev *idev)
{
- int err;
-
write_lock_bh(&mc_lst->sflock);
- if (!mc_lst->sflist) {
- /* any-source empty exclude case */
- err = ip6_mc_del_src(idev, &mc_lst->addr, mc_lst->sfmode,
- 0, NULL, 0);
- } else {
- err = ip6_mc_del_src(idev, &mc_lst->addr, mc_lst->sfmode,
- mc_lst->sflist->sl_count,
- mc_lst->sflist->sl_addr, 0);
- sock_kfree_s(sk, mc_lst->sflist,
- IP6_SFLSIZE(mc_lst->sflist->sl_max));
- mc_lst->sflist = NULL;
- }
+ ip6_mc_del_src_bulk(idev, mc_lst, sk);
write_unlock_bh(&mc_lst->sflock);
- return err;
}
static void mld_leave_group(struct ifmcaddr6 *mc)
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 7/8] mld: convert ip6_sf_socklist to list macros
2021-02-08 17:58 [PATCH net-next 7/8] mld: convert ip6_sf_socklist to list macros Taehee Yoo
@ 2021-02-08 18:31 ` Eric Dumazet
2021-02-09 2:05 ` Taehee Yoo
2021-02-09 1:55 ` kernel test robot
1 sibling, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2021-02-08 18:31 UTC (permalink / raw)
To: Taehee Yoo, davem, kuba, netdev, dsahern, xiyou.wangcong
On 2/8/21 6:58 PM, Taehee Yoo wrote:
> Currently, struct ip6_sf_socklist doesn't use list API so that code
> shape is a little bit different from others.
> So it converts ip6_sf_socklist to use list API so it would
> improve readability.
>
> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> ---
> include/net/if_inet6.h | 19 +-
> include/uapi/linux/in.h | 4 +-
> net/ipv6/mcast.c | 387 +++++++++++++++++++++++++---------------
> 3 files changed, 256 insertions(+), 154 deletions(-)
>
> diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
> index babf19c27b29..6885ab8ec2e9 100644
> --- a/include/net/if_inet6.h
> +++ b/include/net/if_inet6.h
> @@ -13,6 +13,7 @@
> #include <net/snmp.h>
> #include <linux/ipv6.h>
> #include <linux/refcount.h>
> +#include <linux/types.h>
>
> /* inet6_dev.if_flags */
>
> @@ -76,23 +77,19 @@ struct inet6_ifaddr {
> };
>
> struct ip6_sf_socklist {
> - unsigned int sl_max;
> - unsigned int sl_count;
> - struct in6_addr sl_addr[];
> + struct list_head list;
> + struct in6_addr sl_addr;
> + struct rcu_head rcu;
> };
>
I dunno about readability, but :
Your patches adds/delete more than 1000 lines, who is really going to review this ?
You replace dense arrays by lists, so the performance will likely be hurt,
as cpu caches will be poorly used.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 7/8] mld: convert ip6_sf_socklist to list macros
2021-02-08 17:58 [PATCH net-next 7/8] mld: convert ip6_sf_socklist to list macros Taehee Yoo
@ 2021-02-09 1:55 ` kernel test robot
2021-02-09 1:55 ` kernel test robot
1 sibling, 0 replies; 9+ messages in thread
From: kernel test robot @ 2021-02-09 1:55 UTC (permalink / raw)
To: Taehee Yoo, davem, kuba, netdev, dsahern, xiyou.wangcong
Cc: kbuild-all, ap420073
[-- Attachment #1: Type: text/plain, Size: 3626 bytes --]
Hi Taehee,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Taehee-Yoo/mld-change-context-from-atomic-to-sleepable/20210209-072339
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 6626a0266566c5aea16178c5e6cd7fc4db3f2f56
config: mips-allyesconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/0631c9a306555a1d9ee0aefbd7a0cdc892719dd6
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Taehee-Yoo/mld-change-context-from-atomic-to-sleepable/20210209-072339
git checkout 0631c9a306555a1d9ee0aefbd7a0cdc892719dd6
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
net/ipv6/mcast.c: In function 'ip6_mc_del_src':
>> net/ipv6/mcast.c:2150:6: warning: variable 'i' set but not used [-Wunused-but-set-variable]
2150 | int i, err, rv;
| ^
net/ipv6/mcast.c: In function 'ip6_mc_del_src_bulk':
net/ipv6/mcast.c:2218:6: warning: variable 'i' set but not used [-Wunused-but-set-variable]
2218 | int i, rv;
| ^
vim +/i +2150 net/ipv6/mcast.c
2143
2144 static int ip6_mc_del_src(struct inet6_dev *idev, const struct in6_addr *mca,
2145 int sfmode, const struct in6_addr *psfsrc, int delta)
2146 {
2147 struct ifmcaddr6 *mc;
2148 bool found = false;
2149 int changerec = 0;
> 2150 int i, err, rv;
2151
2152 if (!idev)
2153 return -ENODEV;
2154
2155 read_lock_bh(&idev->lock);
2156 list_for_each_entry(mc, &idev->mc_list, list) {
2157 if (ipv6_addr_equal(mca, &mc->mca_addr)) {
2158 found = true;
2159 break;
2160 }
2161 }
2162 if (!found) {
2163 /* MCA not found?? bug */
2164 read_unlock_bh(&idev->lock);
2165 return -ESRCH;
2166 }
2167 spin_lock_bh(&mc->mca_lock);
2168 sf_markstate(mc);
2169 if (!delta) {
2170 if (!mc->mca_sfcount[sfmode]) {
2171 spin_unlock_bh(&mc->mca_lock);
2172 read_unlock_bh(&idev->lock);
2173 return -EINVAL;
2174 }
2175 mc->mca_sfcount[sfmode]--;
2176 }
2177 err = 0;
2178 i = 0;
2179
2180 if (psfsrc) {
2181 rv = ip6_mc_del1_src(mc, sfmode, psfsrc);
2182
2183 changerec |= rv > 0;
2184 if (!err && rv < 0)
2185 err = rv;
2186 }
2187
2188 if (mc->mca_sfmode == MCAST_EXCLUDE &&
2189 mc->mca_sfcount[MCAST_EXCLUDE] == 0 &&
2190 mc->mca_sfcount[MCAST_INCLUDE]) {
2191 struct ip6_sf_list *psf;
2192
2193 /* filter mode change */
2194 mc->mca_sfmode = MCAST_INCLUDE;
2195 mc->mca_crcount = idev->mc_qrv;
2196 idev->mc_ifc_count = mc->mca_crcount;
2197 list_for_each_entry(psf, &mc->mca_source_list, list)
2198 psf->sf_crcount = 0;
2199 mld_ifc_event(mc->idev);
2200 } else if (sf_setstate(mc) || changerec) {
2201 mld_ifc_event(mc->idev);
2202 }
2203 spin_unlock_bh(&mc->mca_lock);
2204 read_unlock_bh(&idev->lock);
2205 return err;
2206 }
2207
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 69939 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 7/8] mld: convert ip6_sf_socklist to list macros
@ 2021-02-09 1:55 ` kernel test robot
0 siblings, 0 replies; 9+ messages in thread
From: kernel test robot @ 2021-02-09 1:55 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 3734 bytes --]
Hi Taehee,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Taehee-Yoo/mld-change-context-from-atomic-to-sleepable/20210209-072339
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 6626a0266566c5aea16178c5e6cd7fc4db3f2f56
config: mips-allyesconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/0631c9a306555a1d9ee0aefbd7a0cdc892719dd6
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Taehee-Yoo/mld-change-context-from-atomic-to-sleepable/20210209-072339
git checkout 0631c9a306555a1d9ee0aefbd7a0cdc892719dd6
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
net/ipv6/mcast.c: In function 'ip6_mc_del_src':
>> net/ipv6/mcast.c:2150:6: warning: variable 'i' set but not used [-Wunused-but-set-variable]
2150 | int i, err, rv;
| ^
net/ipv6/mcast.c: In function 'ip6_mc_del_src_bulk':
net/ipv6/mcast.c:2218:6: warning: variable 'i' set but not used [-Wunused-but-set-variable]
2218 | int i, rv;
| ^
vim +/i +2150 net/ipv6/mcast.c
2143
2144 static int ip6_mc_del_src(struct inet6_dev *idev, const struct in6_addr *mca,
2145 int sfmode, const struct in6_addr *psfsrc, int delta)
2146 {
2147 struct ifmcaddr6 *mc;
2148 bool found = false;
2149 int changerec = 0;
> 2150 int i, err, rv;
2151
2152 if (!idev)
2153 return -ENODEV;
2154
2155 read_lock_bh(&idev->lock);
2156 list_for_each_entry(mc, &idev->mc_list, list) {
2157 if (ipv6_addr_equal(mca, &mc->mca_addr)) {
2158 found = true;
2159 break;
2160 }
2161 }
2162 if (!found) {
2163 /* MCA not found?? bug */
2164 read_unlock_bh(&idev->lock);
2165 return -ESRCH;
2166 }
2167 spin_lock_bh(&mc->mca_lock);
2168 sf_markstate(mc);
2169 if (!delta) {
2170 if (!mc->mca_sfcount[sfmode]) {
2171 spin_unlock_bh(&mc->mca_lock);
2172 read_unlock_bh(&idev->lock);
2173 return -EINVAL;
2174 }
2175 mc->mca_sfcount[sfmode]--;
2176 }
2177 err = 0;
2178 i = 0;
2179
2180 if (psfsrc) {
2181 rv = ip6_mc_del1_src(mc, sfmode, psfsrc);
2182
2183 changerec |= rv > 0;
2184 if (!err && rv < 0)
2185 err = rv;
2186 }
2187
2188 if (mc->mca_sfmode == MCAST_EXCLUDE &&
2189 mc->mca_sfcount[MCAST_EXCLUDE] == 0 &&
2190 mc->mca_sfcount[MCAST_INCLUDE]) {
2191 struct ip6_sf_list *psf;
2192
2193 /* filter mode change */
2194 mc->mca_sfmode = MCAST_INCLUDE;
2195 mc->mca_crcount = idev->mc_qrv;
2196 idev->mc_ifc_count = mc->mca_crcount;
2197 list_for_each_entry(psf, &mc->mca_source_list, list)
2198 psf->sf_crcount = 0;
2199 mld_ifc_event(mc->idev);
2200 } else if (sf_setstate(mc) || changerec) {
2201 mld_ifc_event(mc->idev);
2202 }
2203 spin_unlock_bh(&mc->mca_lock);
2204 read_unlock_bh(&idev->lock);
2205 return err;
2206 }
2207
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 69939 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 7/8] mld: convert ip6_sf_socklist to list macros
2021-02-08 18:31 ` Eric Dumazet
@ 2021-02-09 2:05 ` Taehee Yoo
2021-02-09 2:18 ` David Ahern
0 siblings, 1 reply; 9+ messages in thread
From: Taehee Yoo @ 2021-02-09 2:05 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, Jakub Kicinski, Netdev, dsahern, Cong Wang
On Tue, 9 Feb 2021 at 03:31, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
Hi Eric,
Thank you for the review!
>
>
> On 2/8/21 6:58 PM, Taehee Yoo wrote:
> > Currently, struct ip6_sf_socklist doesn't use list API so that code
> > shape is a little bit different from others.
> > So it converts ip6_sf_socklist to use list API so it would
> > improve readability.
> >
> > Signed-off-by: Taehee Yoo <ap420073@gmail.com>
> > ---
> > include/net/if_inet6.h | 19 +-
> > include/uapi/linux/in.h | 4 +-
> > net/ipv6/mcast.c | 387 +++++++++++++++++++++++++---------------
> > 3 files changed, 256 insertions(+), 154 deletions(-)
> >
> > diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
> > index babf19c27b29..6885ab8ec2e9 100644
> > --- a/include/net/if_inet6.h
> > +++ b/include/net/if_inet6.h
> > @@ -13,6 +13,7 @@
> > #include <net/snmp.h>
> > #include <linux/ipv6.h>
> > #include <linux/refcount.h>
> > +#include <linux/types.h>
> >
> > /* inet6_dev.if_flags */
> >
> > @@ -76,23 +77,19 @@ struct inet6_ifaddr {
> > };
> >
> > struct ip6_sf_socklist {
> > - unsigned int sl_max;
> > - unsigned int sl_count;
> > - struct in6_addr sl_addr[];
> > + struct list_head list;
> > + struct in6_addr sl_addr;
> > + struct rcu_head rcu;
> > };
> >
>
> I dunno about readability, but :
>
> Your patches adds/delete more than 1000 lines, who is really going to review this ?
>
Sorry for that, I think I was so hasty.
> You replace dense arrays by lists, so the performance will likely be hurt,
> as cpu caches will be poorly used.
>
Thanks, I understand why the arrays have been using.
I will send a v2 patch, which contains only the necessary changes.
Thanks a lot,
Taehee Yoo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 7/8] mld: convert ip6_sf_socklist to list macros
2021-02-09 2:05 ` Taehee Yoo
@ 2021-02-09 2:18 ` David Ahern
2021-02-09 2:22 ` Taehee Yoo
2021-02-09 2:56 ` Jakub Kicinski
0 siblings, 2 replies; 9+ messages in thread
From: David Ahern @ 2021-02-09 2:18 UTC (permalink / raw)
To: Taehee Yoo, Eric Dumazet
Cc: David Miller, Jakub Kicinski, Netdev, dsahern, Cong Wang
On 2/8/21 7:05 PM, Taehee Yoo wrote:
> Thanks, I understand why the arrays have been using.
> I will send a v2 patch, which contains only the necessary changes.
please send v2 as a patch series and not 8 individual patches + cover
letter.
Thanks,
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 7/8] mld: convert ip6_sf_socklist to list macros
2021-02-09 2:18 ` David Ahern
@ 2021-02-09 2:22 ` Taehee Yoo
2021-02-09 2:56 ` Jakub Kicinski
1 sibling, 0 replies; 9+ messages in thread
From: Taehee Yoo @ 2021-02-09 2:22 UTC (permalink / raw)
To: David Ahern
Cc: Eric Dumazet, David Miller, Jakub Kicinski, Netdev, dsahern, Cong Wang
On Tue, 9 Feb 2021 at 11:18, David Ahern <dsahern@gmail.com> wrote:
>
> On 2/8/21 7:05 PM, Taehee Yoo wrote:
> > Thanks, I understand why the arrays have been using.
> > I will send a v2 patch, which contains only the necessary changes.
>
> please send v2 as a patch series and not 8 individual patches + cover
> letter.
>
Okay, I will do that.
Thanks,
Taehee Yoo
> Thanks,
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 7/8] mld: convert ip6_sf_socklist to list macros
2021-02-09 2:18 ` David Ahern
2021-02-09 2:22 ` Taehee Yoo
@ 2021-02-09 2:56 ` Jakub Kicinski
2021-02-09 3:33 ` Taehee Yoo
1 sibling, 1 reply; 9+ messages in thread
From: Jakub Kicinski @ 2021-02-09 2:56 UTC (permalink / raw)
To: Taehee Yoo
Cc: David Ahern, Eric Dumazet, David Miller, Netdev, dsahern, Cong Wang
On Mon, 8 Feb 2021 19:18:40 -0700 David Ahern wrote:
> On 2/8/21 7:05 PM, Taehee Yoo wrote:
> > Thanks, I understand why the arrays have been using.
> > I will send a v2 patch, which contains only the necessary changes.
>
> please send v2 as a patch series and not 8 individual patches + cover
> letter.
And if you use git send email please use --thread and
--no-chain-reply-to, so that the patches are all "in response"
to the cover letter.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 7/8] mld: convert ip6_sf_socklist to list macros
2021-02-09 2:56 ` Jakub Kicinski
@ 2021-02-09 3:33 ` Taehee Yoo
0 siblings, 0 replies; 9+ messages in thread
From: Taehee Yoo @ 2021-02-09 3:33 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David Ahern, Eric Dumazet, David Miller, Netdev, dsahern, Cong Wang
On Tue, 9 Feb 2021 at 11:56, Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 8 Feb 2021 19:18:40 -0700 David Ahern wrote:
> > On 2/8/21 7:05 PM, Taehee Yoo wrote:
> > > Thanks, I understand why the arrays have been using.
> > > I will send a v2 patch, which contains only the necessary changes.
> >
> > please send v2 as a patch series and not 8 individual patches + cover
> > letter.
>
> And if you use git send email please use --thread and
> --no-chain-reply-to, so that the patches are all "in response"
> to the cover letter.
Thank you for the advice!
I will use those options :)
Thanks!
Taehee Yoo
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-02-09 3:49 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-08 17:58 [PATCH net-next 7/8] mld: convert ip6_sf_socklist to list macros Taehee Yoo
2021-02-08 18:31 ` Eric Dumazet
2021-02-09 2:05 ` Taehee Yoo
2021-02-09 2:18 ` David Ahern
2021-02-09 2:22 ` Taehee Yoo
2021-02-09 2:56 ` Jakub Kicinski
2021-02-09 3:33 ` Taehee Yoo
2021-02-09 1:55 ` kernel test robot
2021-02-09 1:55 ` kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.