Re: hvn(4): don't input mbufs if interface is not running

2021-06-11 Thread Mike Belopuhov
On 12/05/2021 15:15, Patrick Wildt wrote:
> Hi,
> 
> when hvn(4) attaches it sends commands and waits for replies to come
> back in, hence the interrupt function is being polled.  Unfortunately
> it seems that the 'receive pipe' has both command completion and data
> packets.  As it turns out, while hvn(4) is just setting up the pipes,
> it can already receive packets, which I have seen happening on Hyper-V.
> 
> This essentially means that if_input() is being called *before* the
> card is set up (or UP).  This seems wrong.  Apparently on drivers like
> em(4) we only read packets if IFF_RUNNING is set.  I think in the case
> of hvn(4), we should drop packets unless IFF_RUNNING is set.
> 
> Opinions?
> 

Hi Patrick,

You're right that hvn needs to have the receiving path setup to exchange
commands with the hypervisor. This diff LGTM and should be committed if
it wasn't.

Cheers,
Mike

> Patrick
> 
> diff --git a/sys/dev/pv/if_hvn.c b/sys/dev/pv/if_hvn.c
> index f12e2f935ca..4306f717baf 100644
> --- a/sys/dev/pv/if_hvn.c
> +++ b/sys/dev/pv/if_hvn.c
> @@ -1470,7 +1470,10 @@ hvn_rndis_input(struct hvn_softc *sc, uint64_t tid, 
> void *arg)
>   }
>   hvn_nvs_ack(sc, tid);
>  
> - if_input(ifp, &ml);
> + if (ifp->if_flags & IFF_RUNNING)
> + if_input(ifp, &ml);
> + else
> + ml_purge(&ml);
>  }
>  
>  static inline struct mbuf *
> 



hvn(4): don't input mbufs if interface is not running

2021-05-12 Thread Patrick Wildt
Hi,

when hvn(4) attaches it sends commands and waits for replies to come
back in, hence the interrupt function is being polled.  Unfortunately
it seems that the 'receive pipe' has both command completion and data
packets.  As it turns out, while hvn(4) is just setting up the pipes,
it can already receive packets, which I have seen happening on Hyper-V.

This essentially means that if_input() is being called *before* the
card is set up (or UP).  This seems wrong.  Apparently on drivers like
em(4) we only read packets if IFF_RUNNING is set.  I think in the case
of hvn(4), we should drop packets unless IFF_RUNNING is set.

Opinions?

Patrick

diff --git a/sys/dev/pv/if_hvn.c b/sys/dev/pv/if_hvn.c
index f12e2f935ca..4306f717baf 100644
--- a/sys/dev/pv/if_hvn.c
+++ b/sys/dev/pv/if_hvn.c
@@ -1470,7 +1470,10 @@ hvn_rndis_input(struct hvn_softc *sc, uint64_t tid, void 
*arg)
}
hvn_nvs_ack(sc, tid);
 
-   if_input(ifp, &ml);
+   if (ifp->if_flags & IFF_RUNNING)
+   if_input(ifp, &ml);
+   else
+   ml_purge(&ml);
 }
 
 static inline struct mbuf *