| Message ID | 1453294249-6346-1-git-send-email-paul.durrant@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2016-01-20 at 12:50 +0000, 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?
> -----Original Message----- > From: Ian Campbell [mailto:ian.campbell@citrix.com] > Sent: 20 January 2016 13:06 > To: Paul Durrant; xen-devel@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 +0000, 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
> -----Original Message----- > From: xen-devel-bounces@lists.xen.org [mailto:xen-devel- > bounces@lists.xen.org] On Behalf Of Paul Durrant > Sent: 20 January 2016 13:14 > To: Ian Campbell; xen-devel@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.campbell@citrix.com] > > Sent: 20 January 2016 13:06 > > To: Paul Durrant; xen-devel@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 +0000, 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
On Thu, 2016-01-21 at 11:48 +0000, Paul Durrant wrote: > > -----Original Message----- > > From: xen-devel-bounces@lists.xen.org [mailto:xen-devel- > > bounces@lists.xen.org] On Behalf Of Paul Durrant > > Sent: 20 January 2016 13:14 > > To: Ian Campbell; xen-devel@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.campbell@citrix.com] > > > Sent: 20 January 2016 13:06 > > > To: Paul Durrant; xen-devel@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 +0000, 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.
> -----Original Message----- > From: Ian Campbell [mailto:ian.campbell@citrix.com] > Sent: 21 January 2016 11:59 > To: Paul Durrant; xen-devel@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 +0000, Paul Durrant wrote: > > > -----Original Message----- > > > From: xen-devel-bounces@lists.xen.org [mailto:xen-devel- > > > bounces@lists.xen.org] On Behalf Of Paul Durrant > > > Sent: 20 January 2016 13:14 > > > To: Ian Campbell; xen-devel@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.campbell@citrix.com] > > > > Sent: 20 January 2016 13:06 > > > > To: Paul Durrant; xen-devel@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 +0000, 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.
On Wed, 2016-01-20 at 12:50 +0000, 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 <paul.durrant@citrix.com> > Cc: Ian Campbell <ian.campbell@citrix.com> > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Keir Fraser <keir@xen.org> > Cc: Tim Deegan <tim@xen.org> 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. > */ > > /*
El 21/01/16 a les 16.29, Ian Campbell ha escrit: > On Wed, 2016-01-20 at 12:50 +0000, 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 <paul.durrant@citrix.com> >> Cc: Ian Campbell <ian.campbell@citrix.com> >> Cc: Ian Jackson <ian.jackson@eu.citrix.com> >> Cc: Jan Beulich <jbeulich@suse.com> >> Cc: Keir Fraser <keir@xen.org> >> Cc: Tim Deegan <tim@xen.org> > > > 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.
On Thu, Jan 21, 2016 at 03:29:36PM +0000, Ian Campbell wrote: > On Wed, 2016-01-20 at 12:50 +0000, 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 <paul.durrant@citrix.com> > > Cc: Ian Campbell <ian.campbell@citrix.com> > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > > Cc: Jan Beulich <jbeulich@suse.com> > > Cc: Keir Fraser <keir@xen.org> > > Cc: Tim Deegan <tim@xen.org> > > > 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 <wei.liu2@citrix.com>
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 +0000, 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 <paul.durrant@citrix.com> > >> Cc: Ian Campbell <ian.campbell@citrix.com> > >> Cc: Ian Jackson <ian.jackson@eu.citrix.com> > >> Cc: Jan Beulich <jbeulich@suse.com> > >> Cc: Keir Fraser <keir@xen.org> > >> Cc: Tim Deegan <tim@xen.org> > > > > > > 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. >
> -----Original Message----- > From: Wei Liu [mailto:wei.liu2@citrix.com] > Sent: 21 January 2016 15:46 > To: Ian Campbell > Cc: Paul Durrant; xen-devel@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 +0000, Ian Campbell wrote: > > On Wed, 2016-01-20 at 12:50 +0000, 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 <paul.durrant@citrix.com> > > > Cc: Ian Campbell <ian.campbell@citrix.com> > > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > > > Cc: Jan Beulich <jbeulich@suse.com> > > > Cc: Keir Fraser <keir@xen.org> > > > Cc: Tim Deegan <tim@xen.org> > > > > > > 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 <wei.liu2@citrix.com> Ping? I notice this patch is not yet applied. Paul
On Tue, 2016-01-26 at 14:17 +0000, Paul Durrant wrote: > > -----Original Message----- > > From: Wei Liu [mailto:wei.liu2@citrix.com] > > Sent: 21 January 2016 15:46 > > To: Ian Campbell > > Cc: Paul Durrant; xen-devel@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 +0000, Ian Campbell wrote: > > > On Wed, 2016-01-20 at 12:50 +0000, 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 <paul.durrant@citrix.com> > > > > Cc: Ian Campbell <ian.campbell@citrix.com> > > > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > > > > Cc: Jan Beulich <jbeulich@suse.com> > > > > Cc: Keir Fraser <keir@xen.org> > > > > Cc: Tim Deegan <tim@xen.org> > > > > > > > > > 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 <wei.liu2@citrix.com> > > 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 >
> -----Original Message----- > From: Ian Campbell [mailto:ian.campbell@citrix.com] > Sent: 26 January 2016 16:51 > To: Paul Durrant > Cc: xen-devel@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 +0000, Paul Durrant wrote: > > > -----Original Message----- > > > From: Wei Liu [mailto:wei.liu2@citrix.com] > > > Sent: 21 January 2016 15:46 > > > To: Ian Campbell > > > Cc: Paul Durrant; xen-devel@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 +0000, Ian Campbell wrote: > > > > On Wed, 2016-01-20 at 12:50 +0000, 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 <paul.durrant@citrix.com> > > > > > Cc: Ian Campbell <ian.campbell@citrix.com> > > > > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > > > > > Cc: Jan Beulich <jbeulich@suse.com> > > > > > Cc: Keir Fraser <keir@xen.org> > > > > > Cc: Tim Deegan <tim@xen.org> > > > > > > > > > > > > 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 <wei.liu2@citrix.com> > > > > 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 > >
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. */ /*
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 <paul.durrant@citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Keir Fraser <keir@xen.org> Cc: Tim Deegan <tim@xen.org> --- xen/include/public/io/netif.h | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-)