On Tue, 12 May 2015 14:22:56 +0200
Alexander Aring <alex.aring@xxxxxxxxx> wrote:
> On Tue, May 12, 2015 at 01:52:58PM +0200, Alexander Aring wrote:
> > Hi,
> >
> > ...
> > > >
> > > > Introduce a percpu counter for the sequence numbers,
> > > > incrementation of this counter is an atomic operation then and
> > > > we are sure that we don't sending the same sequence number when
> > > > calling this function at the same time.
> > >
> > > With this, two threads running on the same interface can send
> > > different packets with the same sequence number back to back.
> > > Maybe better make it atomic instead of percpu instead to avoid
> > > that?
> > >
> >
> > Yes you are right, that's not correct. Because per_cpu is a local
> > variable what's the name said _per_ _cpu_. For this kind of very
> > global mib value which needs to be incremented after each transmit
> > a atomic_t should be correct here and that's also what the comment
> > said.
> >
> > Damn, why I thought that a percpu variable should be correct here.
> >
>
> So I updated the draft with this, I hope that is more correct than the
> previous one, which really makes no sense:
>
> diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
> index c6aa1d2..4de59aa 100644
> --- a/include/net/cfg802154.h
> +++ b/include/net/cfg802154.h
> @@ -177,9 +177,9 @@ struct wpan_dev {
> __le64 extended_addr;
>
> /* MAC BSN field */
> - u8 bsn;
> + atomic_t bsn;
> /* MAC DSN field */
> - u8 dsn;
> + atomic_t dsn;
>
> u8 min_be;
> u8 max_be;
> diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
> index 866d27f..b99a6f6 100644
> --- a/net/mac802154/iface.c
> +++ b/net/mac802154/iface.c
> @@ -365,10 +365,7 @@ static int mac802154_header_create(struct
> sk_buff *skb, hdr.fc.type = cb->type;
> hdr.fc.security_enabled = cb->secen;
> hdr.fc.ack_request = cb->ackreq;
> - /* TODO: use atomic_t as dsn, dsn need to be locked when
> AF_IEEE802154
> - * and IEEE802154 6LoWPAN call this at the same time.
> - */
> - hdr.seq = dev->ieee802154_ptr->dsn++;
> + hdr.seq = atomic_inc_return(&dev->ieee802154_ptr->dsn) & 0xFF;
>
> if (mac802154_set_header_security(sdata, &hdr, cb) < 0)
> return -EINVAL;
> @@ -464,13 +461,16 @@ ieee802154_setup_sdata(struct
> ieee802154_sub_if_data *sdata, enum nl802154_iftype type)
> {
> struct wpan_dev *wpan_dev = &sdata->wpan_dev;
> + u8 tmp;
>
> /* set some type-dependent values */
> sdata->vif.type = type;
> sdata->wpan_dev.iftype = type;
>
> - get_random_bytes(&wpan_dev->bsn, 1);
> - get_random_bytes(&wpan_dev->dsn, 1);
> + get_random_bytes(&tmp, 1);
> + atomic_set(&wpan_dev->bsn, tmp);
> + get_random_bytes(&tmp, 1);
> + atomic_set(&wpan_dev->dsn, tmp);
>
> /* defaults per 802.15.4-2011 */
> wpan_dev->min_be = 3;
>
> - Alex
LGTM
--
To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
