[PATCH v2] public/io/netif.h: move and amend multicast control documentation

[PATCH v2] public/io/netif.h: move and amend multicast control documentation

[Paul Durrant]

netif.h contains a specification of the XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL}
extra info messages require to manipulate a multicast filter list maintained
by a backend and specifies the xenstore negotiation protocol in a comment
just above the structure defintion, which is easy to miss.

This patch moves the documentation of the xenstore negotiation to be
co-located with the documentation for other features and also amends the
wording to be clearer.

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 |   22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h
index 353eab7..dfd0412 100644
--- a/xen/include/public/io/netif.h
+++ b/xen/include/public/io/netif.h
@@ -136,6 +136,20 @@
  */
 
 /*
+ * "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.
+ */
+
+/*
  * This is the 'wire' format for packets:
  *  Request 1: netif_tx_request_t -- NETTXF_* (any flags)
  * [Request 2: netif_extra_info_t] (only if request 1 has NETTXF_extra_info)
@@ -341,14 +355,6 @@ struct netif_extra_info {
 
         /*
          * XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL}:
-         * Backend advertises availability via 'feature-multicast-control'
-         * xenbus node containing value '1'.
-         * Frontend requests this feature by advertising
-         * 'request-multicast-control' xenbus node containing value '1'.
-         * If multicast control is requested then multicast flooding is
-         * disabled and the frontend must explicitly register its interest
-         * in multicast groups using dummy transmit requests containing
-         * MCAST_{ADD,DEL} extra-info fragments.
          */
         struct {
             uint8_t addr[6]; /* Address to add/remove. */
-- 
1.7.10.4

Re: [PATCH v2] public/io/netif.h: move and amend multicast control documentation

[Wei Liu]

On Wed, Sep 02, 2015 at 12:17:05PM +0100, Paul Durrant wrote:
> netif.h contains a specification of the XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL}
> extra info messages require to manipulate a multicast filter list maintained
> by a backend and specifies the xenstore negotiation protocol in a comment
> just above the structure defintion, which is easy to miss.
> 
> This patch moves the documentation of the xenstore negotiation to be
> co-located with the documentation for other features and also amends the
> wording to be clearer.
> 
> 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>

Acked-by: Wei Liu <wei.liu2@citrix.com>

> ---
>  xen/include/public/io/netif.h |   22 ++++++++++++++--------
>  1 file changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/include/public/io/netif.h b/xen/include/public/io/netif.h
> index 353eab7..dfd0412 100644
> --- a/xen/include/public/io/netif.h
> +++ b/xen/include/public/io/netif.h
> @@ -136,6 +136,20 @@
>   */
>  
>  /*
> + * "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.

I would prefer adding a blank line here if possible.

> + * 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.
> + */
> +
> +/*
>   * This is the 'wire' format for packets:
>   *  Request 1: netif_tx_request_t -- NETTXF_* (any flags)
>   * [Request 2: netif_extra_info_t] (only if request 1 has NETTXF_extra_info)
> @@ -341,14 +355,6 @@ struct netif_extra_info {
>  
>          /*
>           * XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL}:
> -         * Backend advertises availability via 'feature-multicast-control'
> -         * xenbus node containing value '1'.
> -         * Frontend requests this feature by advertising
> -         * 'request-multicast-control' xenbus node containing value '1'.
> -         * If multicast control is requested then multicast flooding is
> -         * disabled and the frontend must explicitly register its interest
> -         * in multicast groups using dummy transmit requests containing
> -         * MCAST_{ADD,DEL} extra-info fragments.
>           */
>          struct {
>              uint8_t addr[6]; /* Address to add/remove. */
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH v2] public/io/netif.h: move and amend multicast control documentation

[Ian Campbell]

On Fri, 2015-09-04 at 14:22 +0100, Wei Liu wrote:
> On Wed, Sep 02, 2015 at 12:17:05PM +0100, Paul Durrant wrote:
> > netif.h contains a specification of the 
> > XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL}
> > extra info messages require to manipulate a multicast filter list 
> > maintained
> > by a backend and specifies the xenstore negotiation protocol in a 
> > comment
> > just above the structure defintion, which is easy to miss.
> > 
> > This patch moves the documentation of the xenstore negotiation to be
> > co-located with the documentation for other features and also amends 
> > the
> > wording to be clearer.
> > 
> > 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>
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>

Applied, with:

> > + * connected state.
> 
> I would prefer adding a blank line here if possible.

"\n *" added.

Re: [PATCH v2] public/io/netif.h: move and amend multicast control documentation

[Paul Durrant]

> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: 04 September 2015 15:42
> To: Wei Liu; Paul Durrant
> Cc: xen-devel@lists.xenproject.org; Keir (Xen.org); Tim (Xen.org); Ian
> Jackson; Jan Beulich
> Subject: Re: [Xen-devel] [PATCH v2] public/io/netif.h: move and amend
> multicast control documentation
> 
> On Fri, 2015-09-04 at 14:22 +0100, Wei Liu wrote:
> > On Wed, Sep 02, 2015 at 12:17:05PM +0100, Paul Durrant wrote:
> > > netif.h contains a specification of the
> > > XEN_NETIF_EXTRA_TYPE_MCAST_{ADD,DEL}
> > > extra info messages require to manipulate a multicast filter list
> > > maintained
> > > by a backend and specifies the xenstore negotiation protocol in a
> > > comment
> > > just above the structure defintion, which is easy to miss.
> > >
> > > This patch moves the documentation of the xenstore negotiation to be
> > > co-located with the documentation for other features and also amends
> > > the
> > > wording to be clearer.
> > >
> > > 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>
> >
> > Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> Applied, with:
> 
> > > + * connected state.
> >
> > I would prefer adding a blank line here if possible.
> 
> "\n *" added.

Cool. Thanks,

  Paul