Hi,
On Mon, Oct 10, 2022 at 9:13 PM Alexander Aring <aahringo@xxxxxxxxxx> wrote:
>
> Hi,
>
> On Mon, Oct 10, 2022 at 9:04 PM Alexander Aring <aahringo@xxxxxxxxxx> wrote:
> >
> > Hi,
> >
> > On Fri, Oct 7, 2022 at 4:53 AM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:
> > >
> > > We have access to the address filters being theoretically applied, we
> > > also have access to the actual filtering level applied, so let's add a
> > > proper frame validation sequence in hwsim.
> > >
> > > Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> > > ---
> > > drivers/net/ieee802154/mac802154_hwsim.c | 111 ++++++++++++++++++++++-
> > > include/net/ieee802154_netdev.h | 8 ++
> > > 2 files changed, 117 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
> > > index 458be66b5195..84ee948f35bc 100644
> > > --- a/drivers/net/ieee802154/mac802154_hwsim.c
> > > +++ b/drivers/net/ieee802154/mac802154_hwsim.c
> > > @@ -18,6 +18,7 @@
> > > #include <linux/netdevice.h>
> > > #include <linux/device.h>
> > > #include <linux/spinlock.h>
> > > +#include <net/ieee802154_netdev.h>
> > > #include <net/mac802154.h>
> > > #include <net/cfg802154.h>
> > > #include <net/genetlink.h>
> > > @@ -139,6 +140,113 @@ static int hwsim_hw_addr_filt(struct ieee802154_hw *hw,
> > > return 0;
> > > }
> > >
> > > +static void hwsim_hw_receive(struct ieee802154_hw *hw, struct sk_buff *skb,
> > > + u8 lqi)
> > > +{
> > > + struct ieee802154_hdr hdr;
> > > + struct hwsim_phy *phy = hw->priv;
> > > + struct hwsim_pib *pib;
> > > +
> > > + rcu_read_lock();
> > > + pib = rcu_dereference(phy->pib);
> > > +
> > > + if (!pskb_may_pull(skb, 3)) {
> > > + dev_dbg(hw->parent, "invalid frame\n");
> > > + goto drop;
> > > + }
> > > +
> > > + memcpy(&hdr, skb->data, 3);
> > > +
> > > + /* Level 4 filtering: Frame fields validity */
> > > + if (hw->phy->filtering == IEEE802154_FILTERING_4_FRAME_FIELDS) {
>
> I see, there is this big if handling. But it accesses the
> hw->phy->filtering value. It should be part of the hwsim pib setting
> set by the driver callback. It is a question here of mac802154 layer
> setting vs driver layer setting. We should do what the mac802154 tells
> the driver to do, this way we do what the mac802154 layer is set to.
>
> However it's a minor thing and it's okay to do it so...
* whereas we never let the driver know at any time of what different
filter levels exist _currently_ we have only the promiscuous mode
on/off switch which is do nothing or 4_FRAME_FIELDS.
It will work for now, changing anything in the mac802154 filtering
fields or something will end in probably breakage in this handling. In
my point of view as the current state is it should not do that, as
remember that hwsim will "simulate" hardware it should not be able to
access mac802154 fields (especially when doing receiving of frames) as
other hardware will only set register bits (as hwsim pib values is
there for)...
Still I think it's fine for now.
- Alex
