Re: [Xen-devel] [PATCH] public/io/netif.h: change semantics of "request-multicast-control" flag

2016-01-27 Thread Paul Durrant 
> -Original Message-
> From: Ian Campbell [mailto:ian.campb...@citrix.com]
> Sent: 26 January 2016 16:51
> To: Paul Durrant
> Cc: xen-de...@lists.xenproject.org; Ian Jackson; Jan Beulich; Keir (Xen.org);
> Tim (Xen.org); Wei Liu; Roger Pau Monne
> Subject: Re: [PATCH] public/io/netif.h: change semantics of "request-
> multicast-control" flag
> 
> On Tue, 2016-01-26 at 14:17 +, Paul Durrant wrote:
> > > -Original Message-
> > > From: Wei Liu [mailto:wei.l...@citrix.com]
> > > Sent: 21 January 2016 15:46
> > > To: Ian Campbell
> > > Cc: Paul Durrant; xen-de...@lists.xenproject.org; Ian Jackson; Jan
> > > Beulich;
> > > Keir (Xen.org); Tim (Xen.org); Wei Liu; Roger Pau Monne
> > > Subject: Re: [PATCH] public/io/netif.h: change semantics of "request-
> > > multicast-control" flag
> > >
> > > On Thu, Jan 21, 2016 at 03:29:36PM +, Ian Campbell wrote:
> > > > On Wed, 2016-01-20 at 12:50 +, Paul Durrant wrote:
> > > > > My patch b2700877 "move and amend multicast control
> documentation"
> > > > > clarified use of the multicast control protocol between frontend
> > > > > and
> > > > > backend. However, it transpires that the restrictions that
> > > > > documentation
> > > > > placed on the "request-multicast-control" flag make it hard for a
> > > > > frontend to enable 'all multicast' promiscuous mode, in that to do
> > > > > so
> > > > > would require the frontend and backend to disconnect and re-
> > > > > connect.
> > > > >
> > > > > This patch adds a new "feature-dynamic-multicast-control" flag to
> > > > > allow
> > > > > a backend to advertise that it will watch "request-multicast-
> > > > > control"
> > > hence
> > > > > allowing it to be meaningfully modified by the frontend at any time
> > > > > rather
> > > > > than only when the frontend and backend are disconnected.
> > > > >
> > > > > Signed-off-by: Paul Durrant 
> > > > > Cc: Ian Campbell 
> > > > > Cc: Ian Jackson 
> > > > > Cc: Jan Beulich 
> > > > > Cc: Keir Fraser 
> > > > > Cc: Tim Deegan 
> > > >
> > > >
> > > > This looks good to me, but also adding Wei (Linux netback + BSD
> > > > stuff) and
> > > > Roger (BSD stuff) for their perspective.
> > > >
> > > > I should probably have done that for the last set of netif.h changes
> > > > too,
> > > > since apart from the nominal maintainers of xen/include/public/io/*.h
> > > > it's
> > > > worth getting input from the maintainers of the consumers. Not sure
> > > > we
> > > can
> > > > express that very well in MAINTAINERS :-(.
> > > >
> > > > Ian.
> > >
> > > LGTM
> > >
> > > Acked-by: Wei Liu 
> >
> > Ping? I notice this patch is not yet applied.
> 
> I'd have done it yesterday but the machines I needed were offline.
> 
> Now done.

Grand. Thanks,

  Paul

> 
> >
> >   Paul
> >
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] public/io/netif.h: change semantics of "request-multicast-control" flag

2016-01-26 Thread Paul Durrant 
> -Original Message-
> From: Wei Liu [mailto:wei.l...@citrix.com]
> Sent: 21 January 2016 15:46
> To: Ian Campbell
> Cc: Paul Durrant; xen-de...@lists.xenproject.org; Ian Jackson; Jan Beulich;
> Keir (Xen.org); Tim (Xen.org); Wei Liu; Roger Pau Monne
> Subject: Re: [PATCH] public/io/netif.h: change semantics of "request-
> multicast-control" flag
> 
> On Thu, Jan 21, 2016 at 03:29:36PM +, Ian Campbell wrote:
> > On Wed, 2016-01-20 at 12:50 +, Paul Durrant wrote:
> > > My patch b2700877 "move and amend multicast control documentation"
> > > clarified use of the multicast control protocol between frontend and
> > > backend. However, it transpires that the restrictions that documentation
> > > placed on the "request-multicast-control" flag make it hard for a
> > > frontend to enable 'all multicast' promiscuous mode, in that to do so
> > > would require the frontend and backend to disconnect and re-connect.
> > >
> > > This patch adds a new "feature-dynamic-multicast-control" flag to allow
> > > a backend to advertise that it will watch "request-multicast-control"
> hence
> > > allowing it to be meaningfully modified by the frontend at any time rather
> > > than only when the frontend and backend are disconnected.
> > >
> > > Signed-off-by: Paul Durrant 
> > > Cc: Ian Campbell 
> > > Cc: Ian Jackson 
> > > Cc: Jan Beulich 
> > > Cc: Keir Fraser 
> > > Cc: Tim Deegan 
> >
> >
> > This looks good to me, but also adding Wei (Linux netback + BSD stuff) and
> > Roger (BSD stuff) for their perspective.
> >
> > I should probably have done that for the last set of netif.h changes too,
> > since apart from the nominal maintainers of xen/include/public/io/*.h it's
> > worth getting input from the maintainers of the consumers. Not sure we
> can
> > express that very well in MAINTAINERS :-(.
> >
> > Ian.
> 
> LGTM
> 
> Acked-by: Wei Liu 

Ping? I notice this patch is not yet applied.

  Paul


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] public/io/netif.h: change semantics of "request-multicast-control" flag

2016-01-26 Thread Ian Campbell 
On Tue, 2016-01-26 at 14:17 +, Paul Durrant wrote:
> > -Original Message-
> > From: Wei Liu [mailto:wei.l...@citrix.com]
> > Sent: 21 January 2016 15:46
> > To: Ian Campbell
> > Cc: Paul Durrant; xen-de...@lists.xenproject.org; Ian Jackson; Jan
> > Beulich;
> > Keir (Xen.org); Tim (Xen.org); Wei Liu; Roger Pau Monne
> > Subject: Re: [PATCH] public/io/netif.h: change semantics of "request-
> > multicast-control" flag
> > 
> > On Thu, Jan 21, 2016 at 03:29:36PM +, Ian Campbell wrote:
> > > On Wed, 2016-01-20 at 12:50 +, Paul Durrant wrote:
> > > > My patch b2700877 "move and amend multicast control documentation"
> > > > clarified use of the multicast control protocol between frontend
> > > > and
> > > > backend. However, it transpires that the restrictions that
> > > > documentation
> > > > placed on the "request-multicast-control" flag make it hard for a
> > > > frontend to enable 'all multicast' promiscuous mode, in that to do
> > > > so
> > > > would require the frontend and backend to disconnect and re-
> > > > connect.
> > > > 
> > > > This patch adds a new "feature-dynamic-multicast-control" flag to
> > > > allow
> > > > a backend to advertise that it will watch "request-multicast-
> > > > control"
> > hence
> > > > allowing it to be meaningfully modified by the frontend at any time
> > > > rather
> > > > than only when the frontend and backend are disconnected.
> > > > 
> > > > Signed-off-by: Paul Durrant 
> > > > Cc: Ian Campbell 
> > > > Cc: Ian Jackson 
> > > > Cc: Jan Beulich 
> > > > Cc: Keir Fraser 
> > > > Cc: Tim Deegan 
> > > 
> > > 
> > > This looks good to me, but also adding Wei (Linux netback + BSD
> > > stuff) and
> > > Roger (BSD stuff) for their perspective.
> > > 
> > > I should probably have done that for the last set of netif.h changes
> > > too,
> > > since apart from the nominal maintainers of xen/include/public/io/*.h
> > > it's
> > > worth getting input from the maintainers of the consumers. Not sure
> > > we
> > can
> > > express that very well in MAINTAINERS :-(.
> > > 
> > > Ian.
> > 
> > LGTM
> > 
> > Acked-by: Wei Liu 
> 
> Ping? I notice this patch is not yet applied.

I'd have done it yesterday but the machines I needed were offline.

Now done.

> 
>   Paul
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] public/io/netif.h: change semantics of "request-multicast-control" flag

2016-01-21 Thread Paul Durrant 
> -Original Message-
> From: xen-devel-boun...@lists.xen.org [mailto:xen-devel-
> boun...@lists.xen.org] On Behalf Of Paul Durrant
> Sent: 20 January 2016 13:14
> To: Ian Campbell; xen-de...@lists.xenproject.org
> Cc: Ian Jackson; Keir (Xen.org); Jan Beulich; Tim (Xen.org)
> Subject: Re: [Xen-devel] [PATCH] public/io/netif.h: change semantics of
> "request-multicast-control" flag
> 
> > -Original Message-
> > From: Ian Campbell [mailto:ian.campb...@citrix.com]
> > Sent: 20 January 2016 13:06
> > To: Paul Durrant; xen-de...@lists.xenproject.org
> > Cc: Ian Jackson; Jan Beulich; Keir (Xen.org); Tim (Xen.org)
> > Subject: Re: [PATCH] public/io/netif.h: change semantics of "request-
> > multicast-control" flag
> >
> > On Wed, 2016-01-20 at 12:50 +, Paul Durrant wrote:
> > > My patch b2700877 "move and amend multicast control documentation"
> > > clarified use of the multicast control protocol between frontend and
> > > backend. However, it transpires that the restrictions that documentation
> > > placed on the "request-multicast-control" flag make it hard for a
> > > frontend to enable 'all multicast' promiscuous mode, in that to do so
> > > would require the frontend and backend to disconnect and re-connect.
> >
> > Do we therefore think that this document reflected reality, i.e. might this
> > not be "just" a documentation bug?
> >
> > (Or maybe we can't tell because the only previous implementation was
> years
> > ago in Solaris or something)
> 
> That's my concern. I hope it's just a documentation bug, but I don't know.
> Also I've already done an implementation in Linux netback according to the
> restricted semantics.
> 
> >
> > > This patch adds a new "feature-dynamic-multicast-control" flag to allow
> > > a backend to advertise that it will watch "request-multicast-control"
> hence
> > > allowing it to be meaningfully modified by the frontend at any time rather
> > > than only when the frontend and backend are disconnected.
> >
> > Would allowing XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL} to take a
> bcast
> > address
> > be easier on the backend, in that it would just need to be a static feature
> > rather than watching stuff on the fly?
> 
> The documented semantics of the list are 'exact match' so sending a bcast
> address doesn't do much good with a backend that doesn't know to treat is
> specially hence a frontend can't tell whether 'all multicast' mode is going to
> work without the extra feature flag. As for watching "request-multcast-
> control" vs. add/remove of bcast, the complexity of implementation is
> cheaper for the latter but I think the former is 'nicer'.
> 

Are you ok with the xenstore watch approach (and leaving the patch as is) or 
would you prefer to spec. the bcast address as a wildcard and submit a new 
patch?

  Paul

>   Paul
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] public/io/netif.h: change semantics of "request-multicast-control" flag

2016-01-21 Thread Ian Campbell 
On Thu, 2016-01-21 at 11:48 +, Paul Durrant wrote:
> > -Original Message-
> > From: xen-devel-boun...@lists.xen.org [mailto:xen-devel-
> > boun...@lists.xen.org] On Behalf Of Paul Durrant
> > Sent: 20 January 2016 13:14
> > To: Ian Campbell; xen-de...@lists.xenproject.org
> > Cc: Ian Jackson; Keir (Xen.org); Jan Beulich; Tim (Xen.org)
> > Subject: Re: [Xen-devel] [PATCH] public/io/netif.h: change semantics of
> > "request-multicast-control" flag
> > 
> > > -Original Message-
> > > From: Ian Campbell [mailto:ian.campb...@citrix.com]
> > > Sent: 20 January 2016 13:06
> > > To: Paul Durrant; xen-de...@lists.xenproject.org
> > > Cc: Ian Jackson; Jan Beulich; Keir (Xen.org); Tim (Xen.org)
> > > Subject: Re: [PATCH] public/io/netif.h: change semantics of "request-
> > > multicast-control" flag
> > > 
> > > On Wed, 2016-01-20 at 12:50 +, Paul Durrant wrote:
> > > > My patch b2700877 "move and amend multicast control documentation"
> > > > clarified use of the multicast control protocol between frontend
> > > > and
> > > > backend. However, it transpires that the restrictions that
> > > > documentation
> > > > placed on the "request-multicast-control" flag make it hard for a
> > > > frontend to enable 'all multicast' promiscuous mode, in that to do
> > > > so
> > > > would require the frontend and backend to disconnect and re-
> > > > connect.
> > > 
> > > Do we therefore think that this document reflected reality, i.e.
> > > might this
> > > not be "just" a documentation bug?
> > > 
> > > (Or maybe we can't tell because the only previous implementation was
> > years
> > > ago in Solaris or something)
> > 
> > That's my concern. I hope it's just a documentation bug, but I don't
> > know.
> > Also I've already done an implementation in Linux netback according to
> > the
> > restricted semantics.
> > 
> > > 
> > > > This patch adds a new "feature-dynamic-multicast-control" flag to
> > > > allow
> > > > a backend to advertise that it will watch "request-multicast-
> > > > control"
> > hence
> > > > allowing it to be meaningfully modified by the frontend at any time
> > > > rather
> > > > than only when the frontend and backend are disconnected.
> > > 
> > > Would allowing XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL} to take a
> > bcast
> > > address
> > > be easier on the backend, in that it would just need to be a static 
> > > feature
> > > rather than watching stuff on the fly?
> > 
> > The documented semantics of the list are 'exact match' so sending a bcast
> > address doesn't do much good with a backend that doesn't know to treat is
> > specially hence a frontend can't tell whether 'all multicast' mode is going 
> > to
> > work without the extra feature flag. As for watching "request-multcast-
> > control" vs. add/remove of bcast, the complexity of implementation is
> > cheaper for the latter but I think the former is 'nicer'.
> > 
> 
> Are you ok with the xenstore watch approach (and leaving the patch as is)
> or would you prefer to spec. the bcast address as a wildcard and submit a
> new patch?

I'm fine with the watch approach, was just suggesting the alternative in
case it turned out to be much easier.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] public/io/netif.h: change semantics of "request-multicast-control" flag

2016-01-21 Thread Paul Durrant 
> -Original Message-
> From: Ian Campbell [mailto:ian.campb...@citrix.com]
> Sent: 21 January 2016 11:59
> To: Paul Durrant; xen-de...@lists.xenproject.org
> Cc: Ian Jackson; Keir (Xen.org); Jan Beulich; Tim (Xen.org)
> Subject: Re: [Xen-devel] [PATCH] public/io/netif.h: change semantics of
> "request-multicast-control" flag
> 
> On Thu, 2016-01-21 at 11:48 +, Paul Durrant wrote:
> > > -Original Message-
> > > From: xen-devel-boun...@lists.xen.org [mailto:xen-devel-
> > > boun...@lists.xen.org] On Behalf Of Paul Durrant
> > > Sent: 20 January 2016 13:14
> > > To: Ian Campbell; xen-de...@lists.xenproject.org
> > > Cc: Ian Jackson; Keir (Xen.org); Jan Beulich; Tim (Xen.org)
> > > Subject: Re: [Xen-devel] [PATCH] public/io/netif.h: change semantics of
> > > "request-multicast-control" flag
> > >
> > > > -Original Message-
> > > > From: Ian Campbell [mailto:ian.campb...@citrix.com]
> > > > Sent: 20 January 2016 13:06
> > > > To: Paul Durrant; xen-de...@lists.xenproject.org
> > > > Cc: Ian Jackson; Jan Beulich; Keir (Xen.org); Tim (Xen.org)
> > > > Subject: Re: [PATCH] public/io/netif.h: change semantics of "request-
> > > > multicast-control" flag
> > > >
> > > > On Wed, 2016-01-20 at 12:50 +, Paul Durrant wrote:
> > > > > My patch b2700877 "move and amend multicast control
> documentation"
> > > > > clarified use of the multicast control protocol between frontend
> > > > > and
> > > > > backend. However, it transpires that the restrictions that
> > > > > documentation
> > > > > placed on the "request-multicast-control" flag make it hard for a
> > > > > frontend to enable 'all multicast' promiscuous mode, in that to do
> > > > > so
> > > > > would require the frontend and backend to disconnect and re-
> > > > > connect.
> > > >
> > > > Do we therefore think that this document reflected reality, i.e.
> > > > might this
> > > > not be "just" a documentation bug?
> > > >
> > > > (Or maybe we can't tell because the only previous implementation was
> > > years
> > > > ago in Solaris or something)
> > >
> > > That's my concern. I hope it's just a documentation bug, but I don't
> > > know.
> > > Also I've already done an implementation in Linux netback according to
> > > the
> > > restricted semantics.
> > >
> > > >
> > > > > This patch adds a new "feature-dynamic-multicast-control" flag to
> > > > > allow
> > > > > a backend to advertise that it will watch "request-multicast-
> > > > > control"
> > > hence
> > > > > allowing it to be meaningfully modified by the frontend at any time
> > > > > rather
> > > > > than only when the frontend and backend are disconnected.
> > > >
> > > > Would allowing XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL} to take a
> > > bcast
> > > > address
> > > > be easier on the backend, in that it would just need to be a static
> feature
> > > > rather than watching stuff on the fly?
> > >
> > > The documented semantics of the list are 'exact match' so sending a bcast
> > > address doesn't do much good with a backend that doesn't know to treat
> is
> > > specially hence a frontend can't tell whether 'all multicast' mode is 
> > > going
> to
> > > work without the extra feature flag. As for watching "request-multcast-
> > > control" vs. add/remove of bcast, the complexity of implementation is
> > > cheaper for the latter but I think the former is 'nicer'.
> > >
> >
> > Are you ok with the xenstore watch approach (and leaving the patch as is)
> > or would you prefer to spec. the bcast address as a wildcard and submit a
> > new patch?
> 
> I'm fine with the watch approach, was just suggesting the alternative in
> case it turned out to be much easier.
> 

I already have an implementation of the watch approach which is now allowing 
Windows logo testing to pass :-)

  Paul

> Ian.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] public/io/netif.h: change semantics of "request-multicast-control" flag

2016-01-21 Thread Ian Campbell 
On Wed, 2016-01-20 at 12:50 +, Paul Durrant wrote:
> My patch b2700877 "move and amend multicast control documentation"
> clarified use of the multicast control protocol between frontend and
> backend. However, it transpires that the restrictions that documentation
> placed on the "request-multicast-control" flag make it hard for a
> frontend to enable 'all multicast' promiscuous mode, in that to do so
> would require the frontend and backend to disconnect and re-connect.
> 
> This patch adds a new "feature-dynamic-multicast-control" flag to allow
> a backend to advertise that it will watch "request-multicast-control" hence
> allowing it to be meaningfully modified by the frontend at any time rather
> than only when the frontend and backend are disconnected.
> 
> Signed-off-by: Paul Durrant 
> Cc: Ian Campbell 
> Cc: Ian Jackson 
> Cc: Jan Beulich 
> Cc: Keir Fraser 
> Cc: Tim Deegan 


This looks good to me, but also adding Wei (Linux netback + BSD stuff) and
Roger (BSD stuff) for their perspective.

I should probably have done that for the last set of netif.h changes too,
since apart from the nominal maintainers of xen/include/public/io/*.h it's
worth getting input from the maintainers of the consumers. Not sure we can
express that very well in MAINTAINERS :-(.

Ian.

> ---
>  xen/include/public/io/netif.h | 34 ++
>  1 file changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/include/public/io/netif.h
> b/xen/include/public/io/netif.h
> index fe0a87f..8816e0f 100644
> --- a/xen/include/public/io/netif.h
> +++ b/xen/include/public/io/netif.h
> @@ -136,18 +136,28 @@
>   */
>  
>  /*
> - * "feature-multicast-control" advertises the capability to filter ethernet
> - * multicast packets in the backend. To enable use of this capability the
> - * frontend must set "request-multicast-control" before moving into the
> - * connected state.
> - *
> - * If "request-multicast-control" is set then the backend transmit side 
> should
> - * no longer flood multicast packets to the frontend, it should instead drop 
> any
> - * multicast packet that does not match in a filter list. The list is
> - * amended by the frontend by sending dummy transmit requests containing
> - * XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL} extra-info fragments as specified 
> below.
> - * Once enabled by the frontend, the feature cannot be disabled except by
> - * closing and re-connecting to the backend.
> + * "feature-multicast-control" and "feature-dynamic-multicast-control"
> + * advertise the capability to filter ethernet multicast packets in the
> + * backend. If the frontend wishes to take advantage of this feature then
> + * it may set "request-multicast-control". If the backend only advertises
> + * "feature-multicast-control" then "request-multicast-control" must be set
> + * before the frontend moves into the connected state. The backend will
> + * sample the value on this state transition and any subsequent change in
> + * value will have no effect. However, if the backend also advertises
> + * "feature-dynamic-multicast-control" then "request-multicast-control"
> + * may be set by the frontend at any time. In this case, the backend will
> + * watch the value and re-sample on watch events.
> + *
> + * If the sampled value of "request-multicast-control" is set then the
> + * backend transmit side should no longer flood multicast packets to the
> + * frontend, it should instead drop any multicast packet that does not
> + * match in a filter list.
> + * The list is amended by the frontend by sending dummy transmit requests
> + * containing XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL} extra-info fragments as
> + * specified below.
> + * Note that the filter list may be amended even if the sampled value of
> + * "request-multicast-control" is not set, however the filter should only
> + * be applied if it is set.
>   */
>  
>  /*

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] public/io/netif.h: change semantics of "request-multicast-control" flag

2016-01-21 Thread Wei Liu 
On Thu, Jan 21, 2016 at 03:29:36PM +, Ian Campbell wrote:
> On Wed, 2016-01-20 at 12:50 +, Paul Durrant wrote:
> > My patch b2700877 "move and amend multicast control documentation"
> > clarified use of the multicast control protocol between frontend and
> > backend. However, it transpires that the restrictions that documentation
> > placed on the "request-multicast-control" flag make it hard for a
> > frontend to enable 'all multicast' promiscuous mode, in that to do so
> > would require the frontend and backend to disconnect and re-connect.
> > 
> > This patch adds a new "feature-dynamic-multicast-control" flag to allow
> > a backend to advertise that it will watch "request-multicast-control" hence
> > allowing it to be meaningfully modified by the frontend at any time rather
> > than only when the frontend and backend are disconnected.
> > 
> > Signed-off-by: Paul Durrant 
> > Cc: Ian Campbell 
> > Cc: Ian Jackson 
> > Cc: Jan Beulich 
> > Cc: Keir Fraser 
> > Cc: Tim Deegan 
> 
> 
> This looks good to me, but also adding Wei (Linux netback + BSD stuff) and
> Roger (BSD stuff) for their perspective.
> 
> I should probably have done that for the last set of netif.h changes too,
> since apart from the nominal maintainers of xen/include/public/io/*.h it's
> worth getting input from the maintainers of the consumers. Not sure we can
> express that very well in MAINTAINERS :-(.
> 
> Ian.

LGTM

Acked-by: Wei Liu 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] public/io/netif.h: change semantics of "request-multicast-control" flag

2016-01-21 Thread Wei Liu 
On Thu, Jan 21, 2016 at 04:35:40PM +0100, Roger Pau Monné wrote:
> El 21/01/16 a les 16.29, Ian Campbell ha escrit:
> > On Wed, 2016-01-20 at 12:50 +, Paul Durrant wrote:
> >> My patch b2700877 "move and amend multicast control documentation"
> >> clarified use of the multicast control protocol between frontend and
> >> backend. However, it transpires that the restrictions that documentation
> >> placed on the "request-multicast-control" flag make it hard for a
> >> frontend to enable 'all multicast' promiscuous mode, in that to do so
> >> would require the frontend and backend to disconnect and re-connect.
> >>
> >> This patch adds a new "feature-dynamic-multicast-control" flag to allow
> >> a backend to advertise that it will watch "request-multicast-control" hence
> >> allowing it to be meaningfully modified by the frontend at any time rather
> >> than only when the frontend and backend are disconnected.
> >>
> >> Signed-off-by: Paul Durrant 
> >> Cc: Ian Campbell 
> >> Cc: Ian Jackson 
> >> Cc: Jan Beulich 
> >> Cc: Keir Fraser 
> >> Cc: Tim Deegan 
> > 
> > 
> > This looks good to me, but also adding Wei (Linux netback + BSD stuff) and
> > Roger (BSD stuff) for their perspective.
> > 
> > I should probably have done that for the last set of netif.h changes too,
> > since apart from the nominal maintainers of xen/include/public/io/*.h it's
> > worth getting input from the maintainers of the consumers. Not sure we can
> > express that very well in MAINTAINERS :-(.
> 
> I'm going to leave this one to Wei, he has more experience than me
> regarding FreeBSD netfront (and xen-net related topics).
> 
> FWIW, a quick and dirty grep on FreeBSD netfront doesn't show any
> results for "request-multicast-control", so I guess it's not implemented.
> 

No, it is not implemented.

Wei.

> Roger.
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] public/io/netif.h: change semantics of "request-multicast-control" flag

2016-01-21 Thread Roger Pau Monné 
El 21/01/16 a les 16.29, Ian Campbell ha escrit:
> On Wed, 2016-01-20 at 12:50 +, Paul Durrant wrote:
>> My patch b2700877 "move and amend multicast control documentation"
>> clarified use of the multicast control protocol between frontend and
>> backend. However, it transpires that the restrictions that documentation
>> placed on the "request-multicast-control" flag make it hard for a
>> frontend to enable 'all multicast' promiscuous mode, in that to do so
>> would require the frontend and backend to disconnect and re-connect.
>>
>> This patch adds a new "feature-dynamic-multicast-control" flag to allow
>> a backend to advertise that it will watch "request-multicast-control" hence
>> allowing it to be meaningfully modified by the frontend at any time rather
>> than only when the frontend and backend are disconnected.
>>
>> Signed-off-by: Paul Durrant 
>> Cc: Ian Campbell 
>> Cc: Ian Jackson 
>> Cc: Jan Beulich 
>> Cc: Keir Fraser 
>> Cc: Tim Deegan 
> 
> 
> This looks good to me, but also adding Wei (Linux netback + BSD stuff) and
> Roger (BSD stuff) for their perspective.
> 
> I should probably have done that for the last set of netif.h changes too,
> since apart from the nominal maintainers of xen/include/public/io/*.h it's
> worth getting input from the maintainers of the consumers. Not sure we can
> express that very well in MAINTAINERS :-(.

I'm going to leave this one to Wei, he has more experience than me
regarding FreeBSD netfront (and xen-net related topics).

FWIW, a quick and dirty grep on FreeBSD netfront doesn't show any
results for "request-multicast-control", so I guess it's not implemented.

Roger.


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] public/io/netif.h: change semantics of "request-multicast-control" flag

2016-01-20 Thread Paul Durrant 
> -Original Message-
> From: Ian Campbell [mailto:ian.campb...@citrix.com]
> Sent: 20 January 2016 13:06
> To: Paul Durrant; xen-de...@lists.xenproject.org
> Cc: Ian Jackson; Jan Beulich; Keir (Xen.org); Tim (Xen.org)
> Subject: Re: [PATCH] public/io/netif.h: change semantics of "request-
> multicast-control" flag
> 
> On Wed, 2016-01-20 at 12:50 +, Paul Durrant wrote:
> > My patch b2700877 "move and amend multicast control documentation"
> > clarified use of the multicast control protocol between frontend and
> > backend. However, it transpires that the restrictions that documentation
> > placed on the "request-multicast-control" flag make it hard for a
> > frontend to enable 'all multicast' promiscuous mode, in that to do so
> > would require the frontend and backend to disconnect and re-connect.
> 
> Do we therefore think that this document reflected reality, i.e. might this
> not be "just" a documentation bug?
> 
> (Or maybe we can't tell because the only previous implementation was years
> ago in Solaris or something)

That's my concern. I hope it's just a documentation bug, but I don't know. Also 
I've already done an implementation in Linux netback according to the 
restricted semantics.

> 
> > This patch adds a new "feature-dynamic-multicast-control" flag to allow
> > a backend to advertise that it will watch "request-multicast-control" hence
> > allowing it to be meaningfully modified by the frontend at any time rather
> > than only when the frontend and backend are disconnected.
> 
> Would allowing XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL} to take a bcast
> address
> be easier on the backend, in that it would just need to be a static feature
> rather than watching stuff on the fly?

The documented semantics of the list are 'exact match' so sending a bcast 
address doesn't do much good with a backend that doesn't know to treat is 
specially hence a frontend can't tell whether 'all multicast' mode is going to 
work without the extra feature flag. As for watching "request-multcast-control" 
vs. add/remove of bcast, the complexity of implementation is cheaper for the 
latter but I think the former is 'nicer'.

  Paul
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH] public/io/netif.h: change semantics of "request-multicast-control" flag

2016-01-20 Thread Paul Durrant 
My patch b2700877 "move and amend multicast control documentation"
clarified use of the multicast control protocol between frontend and
backend. However, it transpires that the restrictions that documentation
placed on the "request-multicast-control" flag make it hard for a
frontend to enable 'all multicast' promiscuous mode, in that to do so
would require the frontend and backend to disconnect and re-connect.

This patch adds a new "feature-dynamic-multicast-control" flag to allow
a backend to advertise that it will watch "request-multicast-control" hence
allowing it to be meaningfully modified by the frontend at any time rather
than only when the frontend and backend are disconnected.

Signed-off-by: Paul Durrant 
Cc: Ian Campbell 
Cc: Ian Jackson 
Cc: Jan Beulich 
Cc: Keir Fraser 
Cc: Tim Deegan 
---
 xen/include/public/io/netif.h | 34 ++
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h
index fe0a87f..8816e0f 100644
--- a/xen/include/public/io/netif.h
+++ b/xen/include/public/io/netif.h
@@ -136,18 +136,28 @@
  */
 
 /*
- * "feature-multicast-control" advertises the capability to filter ethernet
- * multicast packets in the backend. To enable use of this capability the
- * frontend must set "request-multicast-control" before moving into the
- * connected state.
- *
- * If "request-multicast-control" is set then the backend transmit side should
- * no longer flood multicast packets to the frontend, it should instead drop 
any
- * multicast packet that does not match in a filter list. The list is
- * amended by the frontend by sending dummy transmit requests containing
- * XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL} extra-info fragments as specified 
below.
- * Once enabled by the frontend, the feature cannot be disabled except by
- * closing and re-connecting to the backend.
+ * "feature-multicast-control" and "feature-dynamic-multicast-control"
+ * advertise the capability to filter ethernet multicast packets in the
+ * backend. If the frontend wishes to take advantage of this feature then
+ * it may set "request-multicast-control". If the backend only advertises
+ * "feature-multicast-control" then "request-multicast-control" must be set
+ * before the frontend moves into the connected state. The backend will
+ * sample the value on this state transition and any subsequent change in
+ * value will have no effect. However, if the backend also advertises
+ * "feature-dynamic-multicast-control" then "request-multicast-control"
+ * may be set by the frontend at any time. In this case, the backend will
+ * watch the value and re-sample on watch events.
+ *
+ * If the sampled value of "request-multicast-control" is set then the
+ * backend transmit side should no longer flood multicast packets to the
+ * frontend, it should instead drop any multicast packet that does not
+ * match in a filter list.
+ * The list is amended by the frontend by sending dummy transmit requests
+ * containing XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL} extra-info fragments as
+ * specified below.
+ * Note that the filter list may be amended even if the sampled value of
+ * "request-multicast-control" is not set, however the filter should only
+ * be applied if it is set.
  */
 
 /*
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH] public/io/netif.h: change semantics of "request-multicast-control" flag

2016-01-20 Thread Ian Campbell 
On Wed, 2016-01-20 at 12:50 +, Paul Durrant wrote:
> My patch b2700877 "move and amend multicast control documentation"
> clarified use of the multicast control protocol between frontend and
> backend. However, it transpires that the restrictions that documentation
> placed on the "request-multicast-control" flag make it hard for a
> frontend to enable 'all multicast' promiscuous mode, in that to do so
> would require the frontend and backend to disconnect and re-connect.

Do we therefore think that this document reflected reality, i.e. might this
not be "just" a documentation bug?

(Or maybe we can't tell because the only previous implementation was years
ago in Solaris or something)

> This patch adds a new "feature-dynamic-multicast-control" flag to allow
> a backend to advertise that it will watch "request-multicast-control" hence
> allowing it to be meaningfully modified by the frontend at any time rather
> than only when the frontend and backend are disconnected.

Would allowing XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL} to take a bcast address
be easier on the backend, in that it would just need to be a static feature
rather than watching stuff on the fly?


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel