> There wasn't such suggestion, we were talking about renaming interface
> state from IEEE80211_MESH to something more specific. I suggested
> _ACTIVE, _ON or something like that... any preferences? As for the
> interface name, we have discussed about how MAP would be a different
> interface type, so it would be a good idea to be more precise with the
> it. I would go for MESH_POINT though, to follow the nomenclature in the
> standard.
Hah, ok, I misunderstood the thread then. Sorry. MESH_POINT does sound
better than just MESH though.
> > > /* FIX: what would be proper limits for MTU?
> > > * This interface uses 802.3 frames. */
> > > - if (new_mtu < 256 || new_mtu > IEEE80211_MAX_DATA_LEN - 24 - 6) {
> > > + if (new_mtu < 256 ||
> > > + new_mtu > IEEE80211_MAX_DATA_LEN - 24 - 6 - meshhdrlen) {
> > > printk(KERN_WARNING "%s: invalid MTU %d\n",
> > > dev->name, new_mtu);
> > > return -EINVAL;
> >
> > Are you sure this is appropriate? As the comment says, interface is
> > 802.3 framed. Don't the 802.11 specifications usually keep the frame
> > payload length and require the device to handle longer frames?
>
> 802.11s does not modify the PHY at all, and AFAIK does not impose any
> additional requirement on the devices. So regardless if the code already
> there was needed, it is necessary to decrease the MTU by meshhdrlen
> bytes. Another problem is that mesh headers do not always have the same
> length, but that will be dealt with when support for different length
> headers is added.
Ok, thanks for the explanation.
> > > + /* TODO: check for expired neighbors, paths and frames to non resolved
> > > + * hw addresses
> > > + */
> >
> > Seems dangerous to actually run the code w/o this, no?
>
> No, right now we only add paths by manually from user space.
Oh ok. Never bothered to read that long part of the draft copy I have :)
> > > +static int o80211s_getmeshpath(struct sk_buff *skb, struct nlmsghdr *nlh,
> > > + void *arg)
> > > +{
> > > + return 0;
> > > +}
> >
> > Uh huh. Why is this here? Should it be filled?
>
> Of course. Please note this is work in progress. I guess I should return
> a ENOTSUPP instead.
That'd indeed be better.
> > > @@ -230,6 +231,9 @@ ieee80211_tx_h_check_assoc(struct ieee80211_txrx_data *tx)
> > > (tx->fc & IEEE80211_FCTL_STYPE) != IEEE80211_STYPE_PROBE_REQ))
> > > return TXRX_DROP;
> > >
> > > + if (tx->sdata->type == IEEE80211_IF_TYPE_MESH)
> > > + return TXRX_CONTINUE;
> >
> > Is this right or should there be some other "is forwarded frame" check
> > to go with this? I'm not sure but it seems that frames we send via the
> > netdev need to be still checked? Or not because they're just injected
> > into the mesh? I think I'm confused.
>
> I'm confused too. ieee80211_subif_start_xmit checks if the frame is
> forwarded, ieee80211_tx_h_check_assoc does not care about if the frame
> is forwarded or not. Since there is no interface-wide "association
> state" in mesh, the association check would always be true.
Ok.
> > I think you should use RCU for the locking in the neighbour table. This
> > code should (IIRC) be under rcu read-side lock already so the neighbour
> > table reading would be lockless. Since updates are infrequent, RCU is a
> > good tradeoff.
>
> Updates are not that infrequent, every mesh path might be updated every
> few seconds (about 5 or 10). Would RCU be a good trade off in this
> situation?
I think so, yes. After all, a few seconds is nothing compared to how
many frames could be going through (many per second). In fact, the path
update might lock out the tx/rx paths for quite a while which is highly
undesirable.
johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
