| Message ID | 1383411524-16743-1-git-send-email-florent.fourcot@enst-bretagne.fr |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Sat, Nov 02, 2013 at 05:58:44PM +0100, Florent Fourcot wrote: > It is already possible to set/put/renew a label > with IPV6_FLOWLABEL_MGR and setsockopt. This patch > add the possibility to get information about this > label (current value, time before expiration, etc). > > It helps application to take decision for a renew > or a release of the label. > > Signed-off-by: Florent Fourcot <florent.fourcot@enst-bretagne.fr> > --- > include/net/ipv6.h | 1 + > net/ipv6/ip6_flowlabel.c | 23 +++++++++++++++++++++++ > net/ipv6/ipv6_sockglue.c | 23 +++++++++++++++++++++++ > 3 files changed, 47 insertions(+) > > diff --git a/include/net/ipv6.h b/include/net/ipv6.h > index dd96638..2a5f668 100644 > --- a/include/net/ipv6.h > +++ b/include/net/ipv6.h > @@ -250,6 +250,7 @@ struct ipv6_txoptions *fl6_merge_options(struct ipv6_txoptions *opt_space, > struct ipv6_txoptions *fopt); > void fl6_free_socklist(struct sock *sk); > int ipv6_flowlabel_opt(struct sock *sk, char __user *optval, int optlen); > +int ipv6_flowlabel_opt_get(struct sock *sk, struct in6_flowlabel_req *freq); > int ip6_flowlabel_init(void); > void ip6_flowlabel_cleanup(void); > > diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c > index 819578e..34f58b7 100644 > --- a/net/ipv6/ip6_flowlabel.c > +++ b/net/ipv6/ip6_flowlabel.c > @@ -475,6 +475,29 @@ static inline void fl_link(struct ipv6_pinfo *np, struct ipv6_fl_socklist *sfl, > spin_unlock_bh(&ip6_sk_fl_lock); > } > > +int ipv6_flowlabel_opt_get(struct sock *sk, struct in6_flowlabel_req *freq) > +{ > + struct ipv6_pinfo *np = inet6_sk(sk); > + struct ipv6_fl_socklist *sfl; > + > + rcu_read_lock_bh(); > + for_each_sk_fl_rcu(np, sfl) { > + if (sfl->fl->label == (np->flow_label & IPV6_FLOWLABEL_MASK)) { We should take ip6_fl_lock here, otherwise expires extraction races with the garbage collector (which can update it). There seem to be some other unsafe places, e.g. fl6_renew. > + freq->flr_label = sfl->fl->label; > + freq->flr_dst = sfl->fl->dst; > + freq->flr_share = sfl->fl->share; > + freq->flr_expires = (sfl->fl->expires - jiffies) / HZ; > + freq->flr_linger = sfl->fl->linger / HZ; > + > + rcu_read_unlock_bh(); > + return 0; > + } > + } > + rcu_read_unlock_bh(); > + > + return 0; Maybe return -EINVAL for not found? > +} > + > int ipv6_flowlabel_opt(struct sock *sk, char __user *optval, int optlen) > { > int uninitialized_var(err); > diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c > index 4919a8e..9ca96c2 100644 > --- a/net/ipv6/ipv6_sockglue.c > +++ b/net/ipv6/ipv6_sockglue.c > @@ -1212,6 +1212,29 @@ static int do_ipv6_getsockopt(struct sock *sk, int level, int optname, > val = np->sndflow; > break; > > + case IPV6_FLOWLABEL_MGR: > + { > + struct in6_flowlabel_req freq; > + Can you think about other actions one might do with this getsockopt? Maybe we should copy_from_user freq and check if action specifies a GET request. So a further extension to this API won't break users which did pass uninitialized memory to this getsockopt. > + if (len < sizeof(freq)) > + return -EINVAL; > + > + len = sizeof(freq); > + memset(&freq, 0, sizeof(freq)); > + > + val = ipv6_flowlabel_opt_get(sk, &freq); > + if (val < 0) > + return val; > + > + if (put_user(len, optlen)) > + return -EFAULT; > + if (copy_to_user(optval, &freq, len)) > + return -EFAULT; > + > + return 0; > + break; I guess return 0 is enough here. The break is superfluous. Greetings, Hannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> We should take ip6_fl_lock here, otherwise expires extraction races with > the garbage collector (which can update it). There seem to be some other > unsafe places, e.g. fl6_renew. > I will fix it, and for fl6_renew too. >> + freq->flr_label = sfl->fl->label; >> + freq->flr_dst = sfl->fl->dst; >> + freq->flr_share = sfl->fl->share; >> + freq->flr_expires = (sfl->fl->expires - jiffies) / HZ; >> + freq->flr_linger = sfl->fl->linger / HZ; >> + >> + rcu_read_unlock_bh(); >> + return 0; >> + } >> + } >> + rcu_read_unlock_bh(); >> + >> + return 0; > > Maybe return -EINVAL for not found? > I don't like -EINVAL for this case, since the user can not know if there are no label or if the request has bad parameters. Could -ENOMSG be OK? >> + case IPV6_FLOWLABEL_MGR: >> + { >> + struct in6_flowlabel_req freq; >> + > > Can you think about other actions one might do with this getsockopt? Maybe > we should copy_from_user freq and check if action specifies a GET > request. So a further extension to this API won't break users which did > pass uninitialized memory to this getsockopt. > Very good idea. Regards,
On Mon, Nov 04, 2013 at 06:09:01PM +0100, Florent Fourcot wrote: > > > We should take ip6_fl_lock here, otherwise expires extraction races with > > the garbage collector (which can update it). There seem to be some other > > unsafe places, e.g. fl6_renew. > > > > I will fix it, and for fl6_renew too. Thanks! > >> + freq->flr_label = sfl->fl->label; > >> + freq->flr_dst = sfl->fl->dst; > >> + freq->flr_share = sfl->fl->share; > >> + freq->flr_expires = (sfl->fl->expires - jiffies) / HZ; > >> + freq->flr_linger = sfl->fl->linger / HZ; > >> + > >> + rcu_read_unlock_bh(); > >> + return 0; > >> + } > >> + } > >> + rcu_read_unlock_bh(); > >> + > >> + return 0; > > > > Maybe return -EINVAL for not found? > > > > I don't like -EINVAL for this case, since the user can not know if there > are no label or if the request has bad parameters. Could -ENOMSG be OK? -ENOMSG or we already return -ENOENT from the same setsockopt. I would go with ENOENT. Greetings, Hannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/net/ipv6.h b/include/net/ipv6.h index dd96638..2a5f668 100644 --- a/include/net/ipv6.h +++ b/include/net/ipv6.h @@ -250,6 +250,7 @@ struct ipv6_txoptions *fl6_merge_options(struct ipv6_txoptions *opt_space, struct ipv6_txoptions *fopt); void fl6_free_socklist(struct sock *sk); int ipv6_flowlabel_opt(struct sock *sk, char __user *optval, int optlen); +int ipv6_flowlabel_opt_get(struct sock *sk, struct in6_flowlabel_req *freq); int ip6_flowlabel_init(void); void ip6_flowlabel_cleanup(void); diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c index 819578e..34f58b7 100644 --- a/net/ipv6/ip6_flowlabel.c +++ b/net/ipv6/ip6_flowlabel.c @@ -475,6 +475,29 @@ static inline void fl_link(struct ipv6_pinfo *np, struct ipv6_fl_socklist *sfl, spin_unlock_bh(&ip6_sk_fl_lock); } +int ipv6_flowlabel_opt_get(struct sock *sk, struct in6_flowlabel_req *freq) +{ + struct ipv6_pinfo *np = inet6_sk(sk); + struct ipv6_fl_socklist *sfl; + + rcu_read_lock_bh(); + for_each_sk_fl_rcu(np, sfl) { + if (sfl->fl->label == (np->flow_label & IPV6_FLOWLABEL_MASK)) { + freq->flr_label = sfl->fl->label; + freq->flr_dst = sfl->fl->dst; + freq->flr_share = sfl->fl->share; + freq->flr_expires = (sfl->fl->expires - jiffies) / HZ; + freq->flr_linger = sfl->fl->linger / HZ; + + rcu_read_unlock_bh(); + return 0; + } + } + rcu_read_unlock_bh(); + + return 0; +} + int ipv6_flowlabel_opt(struct sock *sk, char __user *optval, int optlen) { int uninitialized_var(err); diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c index 4919a8e..9ca96c2 100644 --- a/net/ipv6/ipv6_sockglue.c +++ b/net/ipv6/ipv6_sockglue.c @@ -1212,6 +1212,29 @@ static int do_ipv6_getsockopt(struct sock *sk, int level, int optname, val = np->sndflow; break; + case IPV6_FLOWLABEL_MGR: + { + struct in6_flowlabel_req freq; + + if (len < sizeof(freq)) + return -EINVAL; + + len = sizeof(freq); + memset(&freq, 0, sizeof(freq)); + + val = ipv6_flowlabel_opt_get(sk, &freq); + if (val < 0) + return val; + + if (put_user(len, optlen)) + return -EFAULT; + if (copy_to_user(optval, &freq, len)) + return -EFAULT; + + return 0; + break; + } + case IPV6_ADDR_PREFERENCES: val = 0;
It is already possible to set/put/renew a label with IPV6_FLOWLABEL_MGR and setsockopt. This patch add the possibility to get information about this label (current value, time before expiration, etc). It helps application to take decision for a renew or a release of the label. Signed-off-by: Florent Fourcot <florent.fourcot@enst-bretagne.fr> --- include/net/ipv6.h | 1 + net/ipv6/ip6_flowlabel.c | 23 +++++++++++++++++++++++ net/ipv6/ipv6_sockglue.c | 23 +++++++++++++++++++++++ 3 files changed, 47 insertions(+)