[ovs-dev] [PATCH] OVN: add the possibility to specify tunnel dst port
Ben Pfaff
blp at ovn.org
Fri Jun 28 13:23:25 UTC 2019
On Fri, Jun 28, 2019 at 03:08:31PM +0200, Lorenzo Bianconi wrote:
> >
> > On Tue, Jun 25, 2019 at 12:35:26PM +0200, Lorenzo Bianconi wrote:
> > > Introduce dst_port in options column of Encap table in order to add the
> > > capability to configure destination port used for tunnel encapsulation
> > >
> > > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> >
> > Thank you. I think that the documentation can be improved by using the
> > documentation system's ability to document individual keys. Here is my
> > suggestion; I only changed the documentation.
> >
> > --8<--------------------------cut here-------------------------->8--
> >
> > From: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> > Date: Tue, 25 Jun 2019 12:35:26 +0200
> > Subject: [PATCH] OVN: add the possibility to specify tunnel dst port
> >
> > Introduce dst_port in options column of Encap table in order to add the
> > capability to configure destination port used for tunnel encapsulation
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> > Signed-off-by: Ben Pfaff <blp at ovn.org>
> > ---
> > ovn/controller/encaps.c | 4 ++++
> > ovn/ovn-sb.xml | 28 ++++++++++++++++++++--------
> > 2 files changed, 24 insertions(+), 8 deletions(-)
> >
> > diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
> > index 3b3921a736e2..d4a436df318c 100644
> > --- a/ovn/controller/encaps.c
> > +++ b/ovn/controller/encaps.c
> > @@ -156,6 +156,7 @@ tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg,
> > struct smap options = SMAP_INITIALIZER(&options);
> > smap_add(&options, "remote_ip", encap->ip);
> > smap_add(&options, "key", "flow");
> > + const char *dst_port = smap_get(&encap->options, "dst_port");
> > const char *csum = smap_get(&encap->options, "csum");
> > char *tunnel_entry_id = NULL;
> >
> > @@ -169,6 +170,9 @@ tunnel_add(struct tunnel_ctx *tc, const struct sbrec_sb_global *sbg,
> > if (csum && (!strcmp(csum, "true") || !strcmp(csum, "false"))) {
> > smap_add(&options, "csum", csum);
> > }
> > + if (dst_port) {
> > + smap_add(&options, "dst_port", dst_port);
> > + }
> >
> > /* Add auth info if ipsec is enabled. */
> > if (sbg->ipsec) {
> > diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml
> > index 1a2bc1da9ccc..5f7783e0de8e 100644
> > --- a/ovn/ovn-sb.xml
> > +++ b/ovn/ovn-sb.xml
> > @@ -362,14 +362,14 @@
> > </column>
> >
> > <column name="options">
> > - <p>
> > - Options for configuring the encapsulation. Currently, the only
> > - option that has been defined is <code>csum</code>.
> > - </p>
> > + Options for configuring the encapsulation, which may be <ref column="type"/> specific.
> > + </column>
> >
> > + <column name="options" key="csum" type='{"type": "boolean"}'>
> > <p>
> > - <code>csum</code> indicates that encapsulation checksums can be
> > - transmitted and received with reasonable performance. It is a hint
> > + <code>csum</code> indicates whether this chassis can transmit and
> > + receive packets that include checksums with reasonable performance. It
> > + hints
> > to senders transmitting data to this chassis that they should use
> > checksums to protect OVN metadata. <code>ovn-controller</code>
> > populates this key with the value defined in
> > @@ -380,7 +380,7 @@
> > </p>
> >
> > <p>
> > - In terms of performance, this actually significantly increases
> > + In terms of performance, checksumming actually significantly increases
> > throughput in most common cases when running on Linux based hosts
> > without NICs supporting encapsulation hardware offload (around 60% for
> > bulk traffic). The reason is that generally all NICs are capable of
> > @@ -399,7 +399,7 @@
> > efficiently compute or validate full packet checksums. In addition
> > certain versions of the Linux kernel are not able to fully take
> > advantage of encapsulation NIC offloads in the presence of checksums.
> > - (This is actually a pretty narrow corner case though - earlier
> > + (This is actually a pretty narrow corner case though: earlier
> > versions of Linux don't support encapsulation offloads at all and
> > later versions support both offloads and checksums well.)
> > </p>
> > @@ -408,6 +408,18 @@
> > <code>csum</code> defaults to <code>false</code> for hardware VTEPs and
> > <code>true</code> for all other cases.
> > </p>
> > +
> > + <p>
> > + This option applies to <code>geneve</code> and <code>vxlan</code>
> > + encapsulations.
> > + </p>
> > + </column>
> > +
> > + <column name="options" key="dst_port" type='{"type": "integer"}'>
> > + <p>
> > + If set, overrides the UDP (for <code>geneve</code> and
> > + <code>vxlan</code>) or TCP (for <code>stt</code>) destination port.
> > + </p>
> > </column>
> >
> > <column name="ip">
> > --
> > 2.20.1
> >
>
> Hi Ben,
>
> I agree it is definitely better :) Do I need to resend?
No. Since you approve, I added Numan's ack and applied this to master.
Thanks again!
More information about the dev
mailing list