Re: [ovs-dev] [PATCH 1/3] Add OVS_VSWITCHD_STOP to bfd unit tests
On Mon, Jun 22, 2015 at 5:21 PM, Alin Serdean wrote: > I could send another patch with the following: > -tskill $i > +taskkill //PID $i //F >/dev/null > > Alin. Your current patch is something that I think we should apply because from Windows' perspective it does a clean kill instead of the force kill. Do you know why your system does not have tskill? The code originally had taskkill instead of tskill. One thing I observed was that taskkill cannot kill deadlocked processes and unit tests would hang whenever processes deadlock. That is the reason I changed it to tskill. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/3] Add OVS_VSWITCHD_STOP to bfd unit tests
I applied this patch as-is. We still need to figure out the issue around tskill. On Tue, Jun 23, 2015 at 7:05 AM, Gurucharan Shetty wrote: > On Mon, Jun 22, 2015 at 5:21 PM, Alin Serdean > wrote: >> I could send another patch with the following: >> -tskill $i >> +taskkill //PID $i //F >/dev/null >> >> Alin. > > Your current patch is something that I think we should apply because > from Windows' perspective it does a clean kill instead of the force > kill. > > Do you know why your system does not have tskill? The code originally > had taskkill instead of tskill. One thing I observed was that taskkill > cannot kill deadlocked processes and unit tests would hang whenever > processes deadlock. That is the reason I changed it to tskill. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 2/3] Remove the windows service in case it failed to start
On Mon, Jun 22, 2015 at 3:45 PM, Alin Serdean wrote: > In case the ovsdb-server failed to start, the defined service was not > properly cleaned. > > Add a run-if-false command in case the service failed to start. > > Signed-off-by: Alin Gabriel Serdean Applied, thanks. > --- > tests/daemon.at | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/daemon.at b/tests/daemon.at > index dd7a3f4..51d56c5 100644 > --- a/tests/daemon.at > +++ b/tests/daemon.at > @@ -172,7 +172,7 @@ AT_CHECK([sc create ovsdb-server > binpath="$abs_path/ovsdb-server `pwd`/db --log- > [0], [[[SC]] CreateService SUCCESS > ]) > > -AT_CHECK([sc start ovsdb-server], [0], [ignore]) > +AT_CHECK([sc start ovsdb-server], [0], [ignore], [ignore], [sc delete > ovsdb-server]) > OVS_WAIT_UNTIL([test -s pid]) > OVS_WAIT_UNTIL([sc query ovsdb-server | grep STATE | grep RUNNING > > /dev/null 2>&1]) > AT_CHECK([kill -0 `cat pid`], [0], [ignore]) > -- > 1.9.5.msysgit.0 > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 3/3] Update windows test documentation
On Mon, Jun 22, 2015 at 3:45 PM, Alin Serdean wrote: > Describe explictly where to add the pthread library to the PATH variable. > > In case the pthread library directory was added to the user PATH variable > the service failed to start. > > Signed-off-by: Alin Gabriel Serdean > --- > INSTALL.Windows.md | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/INSTALL.Windows.md b/INSTALL.Windows.md > index 6d870ed..52acf6e 100644 > --- a/INSTALL.Windows.md > +++ b/INSTALL.Windows.md > @@ -92,6 +92,9 @@ or from a distribution tar ball. > > % make check TESTSUITEFLAGS="-j8" > > + For "daemon --service" test make sure to have the pthread libaries in > + the the Windows' SYSTEM PATH environment variable > + How about the following wording instead? (One of the unit tests starts Open vSwitch as a system wide service. This test expects the pthread libraries path to be in the Windows' SYSTEM PATH environment variable. If while running unit tests, you do not have the permissions to start a system wide service or if you already have a Open vSwitch service running, you can disable the unit test with: make check TESTSUITEFLAGS="-k \!windows-service –j8”) If you think above is correct, I will update your patch before committing. > * To install all the compiled executables on the local machine, run: > > % make install > -- > 1.9.5.msysgit.0 > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH]: bfd: display last wall clock time of last flap
Extend bfd to save wall clock time of the last flap in bfd_forwarding__,
and display it throught bfd/show. This information is also exported out
to ovsdb in bfd_get_status.
Signed-off-by: Sabyasachi Sengupta
diff --git a/lib/bfd.c b/lib/bfd.c
index 92fdbd8..c8122b0 100644
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -223,6 +223,7 @@ struct bfd {
long long int decay_detect_time; /* Decay detection time. */
uint64_t flap_count; /* Counts bfd forwarding flaps. */
+long long int flap_time; /* Contains timestamp of last flap */
/* True when the variables returned by bfd_get_status() are changed
* since last check. */
@@ -321,6 +322,12 @@ bfd_get_status(const struct bfd *bfd, struct smap *smap)
smap_add(smap, "state", bfd_state_str(bfd->state));
smap_add(smap, "diagnostic", bfd_diag_str(bfd->diag));
smap_add_format(smap, "flap_count", "%"PRIu64, bfd->flap_count);
+if (bfd->flap_time) {
+char *s;
+s = xastrftime_msec("%Y-%m-%d %H:%M:%S.###", bfd->flap_time, false);
+smap_add(smap, "flap_time", s);
+free(s);
+}
if (bfd->state != STATE_DOWN) {
smap_add(smap, "remote_state", bfd_state_str(bfd->rmt_state));
@@ -380,6 +387,7 @@ bfd_configure(struct bfd *bfd, const char *name, const
struct smap *cfg,
bfd->rx_packets = bfd_rx_packets(bfd);
bfd->in_decay = false;
bfd->flap_count = 0;
+bfd->flap_time = 0;
/* RFC 5881 section 4
* The source port MUST be in the range 49152 through 65535. The same
@@ -937,6 +945,7 @@ bfd_forwarding__(struct bfd *bfd) OVS_REQUIRES(mutex)
&& bfd->rmt_diag != DIAG_RCPATH_DOWN;
if (bfd->last_forwarding != last_forwarding) {
bfd->flap_count++;
+bfd->flap_time = time_wall_msec();
bfd_status_changed(bfd);
}
return bfd->last_forwarding;
@@ -1269,6 +1278,12 @@ bfd_put_details(struct ds *ds, const struct bfd *bfd)
OVS_REQUIRES(mutex)
time_msec() - bfd->next_tx);
ds_put_format(ds, "\tLast TX Time: now %+lldms\n",
time_msec() - bfd->last_tx);
+if (bfd->flap_time) {
+char *s;
+s = xastrftime_msec("%Y-%m-%d %H:%M:%S.###", bfd->flap_time, false);
+ds_put_format(ds, "\tLast Flap Time: %s\n", s);
+free(s);
+}
ds_put_cstr(ds, "\n");
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 7/7] db-ctl-base: Improve show command.
On Thu, Jun 11, 2015 at 07:37:02PM -0700, Alex Wang wrote: > This commit adds improvement to 'show' command logic and allows it > to print key->table_ref maps. The direct effect can be observed > from the tests/vtep-ctl.at change. The improvement will also be > used in the ovn-sbctl implementation. > > Signed-off-by: Alex Wang I'm happy with this whole series, you can take this for all the patches: Acked-by: Ben Pfaff I see places for improvement, but since so much of this series is moving code around, most of those improvements could have been made to the original code and can still be made later, so I'd really rather just get it in. Thanks for doing this! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] Add a few OVN files as part of DISTCLEANFILES.
Reported-by: Ian Stokes Signed-off-by: Gurucharan Shetty --- ovn/automake.mk|1 + ovn/controller/automake.mk |1 + ovn/utilities/automake.mk |3 +++ utilities/automake.mk |2 ++ 4 files changed, 7 insertions(+) diff --git a/ovn/automake.mk b/ovn/automake.mk index 7eb9c1d..ee20ce7 100644 --- a/ovn/automake.mk +++ b/ovn/automake.mk @@ -68,6 +68,7 @@ ovn/ovn-nb.5: \ man_MANS += ovn/ovn-architecture.7 ovn/ovn-nbctl.8 EXTRA_DIST += ovn/ovn-architecture.7.xml ovn/ovn-nbctl.8.xml +DISTCLEANFILES += ovn/ovn-nbctl.8 ovn/ovn-architecture.7 SUFFIXES += .xml %: %.xml diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk index 12a3606..25b5f8b 100644 --- a/ovn/controller/automake.mk +++ b/ovn/controller/automake.mk @@ -15,3 +15,4 @@ ovn_controller_ovn_controller_SOURCES = \ ovn_controller_ovn_controller_LDADD = ovn/lib/libovn.la lib/libopenvswitch.la man_MANS += ovn/controller/ovn-controller.8 EXTRA_DIST += ovn/controller/ovn-controller.8.xml +DISTCLEANFILES += ovn/controller/ovn-controller.8 diff --git a/ovn/utilities/automake.mk b/ovn/utilities/automake.mk index a55c1ad..1373864 100644 --- a/ovn/utilities/automake.mk +++ b/ovn/utilities/automake.mk @@ -7,3 +7,6 @@ man_MANS += \ EXTRA_DIST += \ ovn/utilities/ovn-ctl \ ovn/utilities/ovn-ctl.8.xml + +DISTCLEANFILES += \ +ovn/utilities/ovn-ctl.8 diff --git a/utilities/automake.mk b/utilities/automake.mk index 3b43cb4..e7fcf4f 100644 --- a/utilities/automake.mk +++ b/utilities/automake.mk @@ -93,6 +93,8 @@ DISTCLEANFILES += \ utilities/ovs-pcap.1 \ utilities/ovs-pki \ utilities/ovs-pki.8 \ + utilities/ovs-sim \ + utilities/ovs-sim.1 \ utilities/ovs-tcpundump \ utilities/ovs-tcpundump.1 \ utilities/ovs-test \ -- 1.7.9.5 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] Add a few OVN files as part of DISTCLEANFILES.
On 06/23/2015 10:49 AM, Gurucharan Shetty wrote: > Reported-by: Ian Stokes > Signed-off-by: Gurucharan Shetty > --- > ovn/automake.mk|1 + > ovn/controller/automake.mk |1 + > ovn/utilities/automake.mk |3 +++ > utilities/automake.mk |2 ++ > 4 files changed, 7 insertions(+) Acked-by: Russell Bryant -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] Add a few OVN files as part of DISTCLEANFILES.
> -Original Message- > From: Gurucharan Shetty [mailto:shet...@nicira.com] > Sent: Tuesday, June 23, 2015 3:49 PM > To: dev@openvswitch.org > Cc: Stokes, Ian; Gurucharan Shetty > Subject: [PATCH] Add a few OVN files as part of DISTCLEANFILES. > > Reported-by: Ian Stokes > Signed-off-by: Gurucharan Shetty > --- > ovn/automake.mk|1 + > ovn/controller/automake.mk |1 + > ovn/utilities/automake.mk |3 +++ > utilities/automake.mk |2 ++ > 4 files changed, 7 insertions(+) > > diff --git a/ovn/automake.mk b/ovn/automake.mk > index 7eb9c1d..ee20ce7 100644 > --- a/ovn/automake.mk > +++ b/ovn/automake.mk > @@ -68,6 +68,7 @@ ovn/ovn-nb.5: \ > > man_MANS += ovn/ovn-architecture.7 ovn/ovn-nbctl.8 > EXTRA_DIST += ovn/ovn-architecture.7.xml ovn/ovn-nbctl.8.xml > +DISTCLEANFILES += ovn/ovn-nbctl.8 ovn/ovn-architecture.7 > > SUFFIXES += .xml > %: %.xml > diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk > index 12a3606..25b5f8b 100644 > --- a/ovn/controller/automake.mk > +++ b/ovn/controller/automake.mk > @@ -15,3 +15,4 @@ ovn_controller_ovn_controller_SOURCES = \ > ovn_controller_ovn_controller_LDADD = ovn/lib/libovn.la > lib/libopenvswitch.la > man_MANS += ovn/controller/ovn-controller.8 > EXTRA_DIST += ovn/controller/ovn-controller.8.xml > +DISTCLEANFILES += ovn/controller/ovn-controller.8 > diff --git a/ovn/utilities/automake.mk b/ovn/utilities/automake.mk > index a55c1ad..1373864 100644 > --- a/ovn/utilities/automake.mk > +++ b/ovn/utilities/automake.mk > @@ -7,3 +7,6 @@ man_MANS += \ > EXTRA_DIST += \ > ovn/utilities/ovn-ctl \ > ovn/utilities/ovn-ctl.8.xml > + > +DISTCLEANFILES += \ > +ovn/utilities/ovn-ctl.8 > diff --git a/utilities/automake.mk b/utilities/automake.mk > index 3b43cb4..e7fcf4f 100644 > --- a/utilities/automake.mk > +++ b/utilities/automake.mk > @@ -93,6 +93,8 @@ DISTCLEANFILES += \ > utilities/ovs-pcap.1 \ > utilities/ovs-pki \ > utilities/ovs-pki.8 \ > + utilities/ovs-sim \ > + utilities/ovs-sim.1 \ > utilities/ovs-tcpundump \ > utilities/ovs-tcpundump.1 \ > utilities/ovs-test \ > -- > 1.7.9.5 Thanks for the quick response. I've tested this and it works as expected now. Tested-by: Ian Stokes ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] Add a few OVN files as part of DISTCLEANFILES.
Thank you Russell and Ian. I applied this to master. On Tue, Jun 23, 2015 at 9:30 AM, Stokes, Ian wrote: > > >> -Original Message- >> From: Gurucharan Shetty [mailto:shet...@nicira.com] >> Sent: Tuesday, June 23, 2015 3:49 PM >> To: dev@openvswitch.org >> Cc: Stokes, Ian; Gurucharan Shetty >> Subject: [PATCH] Add a few OVN files as part of DISTCLEANFILES. >> >> Reported-by: Ian Stokes >> Signed-off-by: Gurucharan Shetty >> --- >> ovn/automake.mk|1 + >> ovn/controller/automake.mk |1 + >> ovn/utilities/automake.mk |3 +++ >> utilities/automake.mk |2 ++ >> 4 files changed, 7 insertions(+) >> >> diff --git a/ovn/automake.mk b/ovn/automake.mk >> index 7eb9c1d..ee20ce7 100644 >> --- a/ovn/automake.mk >> +++ b/ovn/automake.mk >> @@ -68,6 +68,7 @@ ovn/ovn-nb.5: \ >> >> man_MANS += ovn/ovn-architecture.7 ovn/ovn-nbctl.8 >> EXTRA_DIST += ovn/ovn-architecture.7.xml ovn/ovn-nbctl.8.xml >> +DISTCLEANFILES += ovn/ovn-nbctl.8 ovn/ovn-architecture.7 >> >> SUFFIXES += .xml >> %: %.xml >> diff --git a/ovn/controller/automake.mk b/ovn/controller/automake.mk >> index 12a3606..25b5f8b 100644 >> --- a/ovn/controller/automake.mk >> +++ b/ovn/controller/automake.mk >> @@ -15,3 +15,4 @@ ovn_controller_ovn_controller_SOURCES = \ >> ovn_controller_ovn_controller_LDADD = ovn/lib/libovn.la >> lib/libopenvswitch.la >> man_MANS += ovn/controller/ovn-controller.8 >> EXTRA_DIST += ovn/controller/ovn-controller.8.xml >> +DISTCLEANFILES += ovn/controller/ovn-controller.8 >> diff --git a/ovn/utilities/automake.mk b/ovn/utilities/automake.mk >> index a55c1ad..1373864 100644 >> --- a/ovn/utilities/automake.mk >> +++ b/ovn/utilities/automake.mk >> @@ -7,3 +7,6 @@ man_MANS += \ >> EXTRA_DIST += \ >> ovn/utilities/ovn-ctl \ >> ovn/utilities/ovn-ctl.8.xml >> + >> +DISTCLEANFILES += \ >> +ovn/utilities/ovn-ctl.8 >> diff --git a/utilities/automake.mk b/utilities/automake.mk >> index 3b43cb4..e7fcf4f 100644 >> --- a/utilities/automake.mk >> +++ b/utilities/automake.mk >> @@ -93,6 +93,8 @@ DISTCLEANFILES += \ >> utilities/ovs-pcap.1 \ >> utilities/ovs-pki \ >> utilities/ovs-pki.8 \ >> + utilities/ovs-sim \ >> + utilities/ovs-sim.1 \ >> utilities/ovs-tcpundump \ >> utilities/ovs-tcpundump.1 \ >> utilities/ovs-test \ >> -- >> 1.7.9.5 > Thanks for the quick response. > > I've tested this and it works as expected now. > > Tested-by: Ian Stokes ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 7/7] db-ctl-base: Improve show command.
Thx a lot for the review! Applied series to master, On Tue, Jun 23, 2015 at 8:44 AM, Ben Pfaff wrote: > On Thu, Jun 11, 2015 at 07:37:02PM -0700, Alex Wang wrote: > > This commit adds improvement to 'show' command logic and allows it > > to print key->table_ref maps. The direct effect can be observed > > from the tests/vtep-ctl.at change. The improvement will also be > > used in the ovn-sbctl implementation. > > > > Signed-off-by: Alex Wang > > I'm happy with this whole series, you can take this for all the patches: > Acked-by: Ben Pfaff > > I see places for improvement, but since so much of this series is moving > code around, most of those improvements could have been made to the > original code and can still be made later, so I'd really rather just get > it in. Thanks for doing this! > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2 2/2] Makefiles: Stop distributing files because building them requires Python.
Thanks for the reviews. I applied these to master. On Mon, Jun 22, 2015 at 11:13:39PM -0700, Alex Wang wrote: > Thx for the explanation, > > Looks good to me, > > On Mon, Jun 22, 2015 at 3:06 PM, Ben Pfaff wrote: > > > On Mon, Jun 22, 2015 at 10:50:55AM -0700, Alex Wang wrote: > > > On Wed, Jun 10, 2015 at 9:04 AM, Ben Pfaff wrote: > > > > > > > A long time ago, the Open vSwitch build did not depend on Python > > (whereas > > > > the runtime did), so the "make dist" based distribution included the > > > > results of Python build tools. Later, the build began building on > > Python, > > > > but the distribution still included some of those results, because no > > one > > > > had gone to the trouble of changing them. This commit changes the > > > > Makefiles not to distribute Python-generated files but instead to just > > > > generate them at build time. > > > > > > Could you explain more on "Later, the build began building on Python"? > > > > That's bad phrasing. I mean that, later, the build did start to require > > direct use of Python. One example is the extract-ofp-actions program, > > which always runs during the build (its build products are not > > distributed). > > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] ofp-actions: Support mixing "conjunction" and "note" actions.
I'd appreciate a review for this because I'd like to get it into OVS
2.4. The "conjunction match" feature is new in 2.4 and I'd like to have
it have this feature there.
On Mon, Jun 15, 2015 at 01:58:32PM -0700, Ben Pfaff wrote:
> It doesn't make sense to mix "conjunction" actions with most other kinds
> of actions. That's because flows with "conjunction" actions aren't ever
> actually executed, so any actions mixed up with them would never do
> anything useful. "note" actions are a little different because they never
> do anything useful anyway: they are just there to allow a controller to
> annotate flows. It makes as much sense to annotate a flow with
> "conjunction" actions as it does to annotate any other flow, so this
> commit makes this possible.
>
> Requested-by: Soner Sevinc
> Signed-off-by: Ben Pfaff
> ---
> lib/ofp-actions.c| 7 ---
> ofproto/ofproto.c| 36 +++-
> tests/classifier.at | 17 +
> utilities/ovs-ofctl.8.in | 5 +++--
> 4 files changed, 43 insertions(+), 22 deletions(-)
>
> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> index 10e2a92..eef3389 100644
> --- a/lib/ofp-actions.c
> +++ b/lib/ofp-actions.c
> @@ -5737,9 +5737,10 @@ ofpacts_verify(const struct ofpact ofpacts[], size_t
> ofpacts_len,
>
> if (a->type == OFPACT_CONJUNCTION) {
> OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
> -if (a->type != OFPACT_CONJUNCTION) {
> -VLOG_WARN("when conjunction action is present, it must
> be "
> - "the only kind of action used (saw '%s'
> action)",
> +if (a->type != OFPACT_CONJUNCTION && a->type != OFPACT_NOTE)
> {
> +VLOG_WARN("\"conjunction\" actions may be used along
> with "
> + "\"note\" but not any other kind of action "
> + "(such as the \"%s\" action used here)",
>ofpact_name(a->type));
> return OFPERR_NXBAC_BAD_CONJUNCTION;
> }
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 08ba043..2a5d831 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -4406,12 +4406,6 @@ evict_rules_from_table(struct oftable *table)
> return error;
> }
>
> -static bool
> -is_conjunction(const struct ofpact *ofpacts, size_t ofpacts_len)
> -{
> -return ofpacts_len > 0 && ofpacts->type == OFPACT_CONJUNCTION;
> -}
> -
> static void
> get_conjunctions(const struct ofputil_flow_mod *fm,
> struct cls_conjunction **conjsp, size_t *n_conjsp)
> @@ -4420,23 +4414,31 @@ get_conjunctions(const struct ofputil_flow_mod *fm,
> struct cls_conjunction *conjs = NULL;
> int n_conjs = 0;
>
> -if (is_conjunction(fm->ofpacts, fm->ofpacts_len)) {
> -const struct ofpact *ofpact;
> -int i;
> +n_conjs = 0;
>
> -n_conjs = 0;
> -OFPACT_FOR_EACH (ofpact, fm->ofpacts, fm->ofpacts_len) {
> +const struct ofpact *ofpact;
> +OFPACT_FOR_EACH (ofpact, fm->ofpacts, fm->ofpacts_len) {
> +if (ofpact->type == OFPACT_CONJUNCTION) {
> n_conjs++;
> +} else if (ofpact->type != OFPACT_NOTE) {
> +/* "conjunction" may appear with "note" actions but not with any
> + * other type of actions. */
> +ovs_assert(!n_conjs);
> +break;
> }
> +}
> +if (n_conjs) {
> +int i = 0;
>
> conjs = xzalloc(n_conjs * sizeof *conjs);
> -i = 0;
> OFPACT_FOR_EACH (ofpact, fm->ofpacts, fm->ofpacts_len) {
> -struct ofpact_conjunction *oc = ofpact_get_CONJUNCTION(ofpact);
> -conjs[i].clause = oc->clause;
> -conjs[i].n_clauses = oc->n_clauses;
> -conjs[i].id = oc->id;
> -i++;
> +if (ofpact->type == OFPACT_CONJUNCTION) {
> +struct ofpact_conjunction *oc =
> ofpact_get_CONJUNCTION(ofpact);
> +conjs[i].clause = oc->clause;
> +conjs[i].n_clauses = oc->n_clauses;
> +conjs[i].id = oc->id;
> +i++;
> +}
> }
> }
>
> diff --git a/tests/classifier.at b/tests/classifier.at
> index 1e75123..3520acd 100644
> --- a/tests/classifier.at
> +++ b/tests/classifier.at
> @@ -289,3 +289,20 @@ for src in 0 1 2 3; do
> done
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> +
> +AT_SETUP([conjunctive match and other actions])
> +OVS_VSWITCHD_START
> +# It's OK to use "conjunction" actions with "note" actions.
> +AT_CHECK([ovs-ofctl add-flow br0
> 'actions=conjunction(3,1/2),note:41.42.43.44.45.46'])
> +AT_CHECK([ovs-ofctl add-flow br0
> 'actions=note:41.42.43.44.45.46,conjunction(3,1/2)'])
> +# It's not OK to use "conjunction" actions with other types of actions.
> +AT_CHECK([ovs-ofctl '-vPATTERN:console:%c|%p|%m' add-flow br0
> 'actions=output:1,conjunction(3,
[ovs-dev] [PATCH 0/2] ovn: Add administrative state to logical ports.
While working on OpenStack Neutron integration for OVN, I came across a small feature gap. The Neutron API supports setting a port as administratively down. One way to implement that would be to delete ports from OVN while administratively down. However, it seemed to me that it would be nice to keep the list of ports that currently exist in sync between the CMS (Neutron in this case) and OVN. This patch is a first attempt at supporting this OVN by updating the Pipeline to drop packets to/from a port that is administratively down. [PATCH 1/2] ovn: Add logical port 'enabled' state. [PATCH 2/2] ovn: Add get/set-enabled to ovn-nbctl. northd/ovn-northd.c | 14 +--- ovn-nb.ovsschema|1 ovn-nb.xml |7 ++ ovn-nbctl.8.xml | 13 +++ ovn-nbctl.c | 59 5 files changed, 91 insertions(+), 3 deletions(-) -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/2] ovn: Add get/set-enabled to ovn-nbctl.
This patch adds support for getting and setting the 'enabled' column
for logical ports using ovn-nbctl.
Signed-off-by: Russell Bryant
---
ovn/ovn-nbctl.8.xml | 13
ovn/ovn-nbctl.c | 59 +
2 files changed, 72 insertions(+)
diff --git a/ovn/ovn-nbctl.8.xml b/ovn/ovn-nbctl.8.xml
index 0e89229..39ffb35 100644
--- a/ovn/ovn-nbctl.8.xml
+++ b/ovn/ovn-nbctl.8.xml
@@ -179,6 +179,19 @@
down.
+ lport-set-enabled lport state
+
+Set the administrative state of lport, either
enabled
+or disabled. When a port is disabled, no traffic is
allowed into
+or out of the port.
+
+
+ lport-get-enabled lport
+
+Prints the administrative state of lport, either
enabled
+or disabled.
+
+
Options
diff --git a/ovn/ovn-nbctl.c b/ovn/ovn-nbctl.c
index a8e5d08..6a35a1a 100644
--- a/ovn/ovn-nbctl.c
+++ b/ovn/ovn-nbctl.c
@@ -81,6 +81,11 @@ Logical port commands:\n\
set port security addresses for LPORT.\n\
lport-get-port-security LPORTget LPORT's port security addresses\n\
lport-get-up LPORTget state of LPORT ('up' or 'down')\n\
+ lport-set-enabled LPORT STATE\n\
+set administrative state LPORT\n\
+('enabled' or 'disabled')\n\
+ lport-get-enabled LPORT get administrative state LPORT\n\
+('enabled' or 'disabled')\n\
\n\
Options:\n\
--db=DATABASE connect to DATABASE\n\
@@ -566,6 +571,46 @@ do_lport_get_up(struct ovs_cmdl_context *ctx)
printf("%s\n", (lport->up && *lport->up) ? "up" : "down");
}
+
+static void
+do_lport_set_enabled(struct ovs_cmdl_context *ctx)
+{
+struct nbctl_context *nb_ctx = ctx->pvt;
+const char *id = ctx->argv[1];
+const char *state = ctx->argv[2];
+const struct nbrec_logical_port *lport;
+
+lport = lport_by_name_or_uuid(nb_ctx, id);
+if (!lport) {
+return;
+}
+
+if (!strcasecmp(state, "enabled")) {
+bool enabled = true;
+nbrec_logical_port_set_enabled(lport, &enabled, 1);
+} else if (!strcasecmp(state, "disabled")) {
+bool enabled = false;
+nbrec_logical_port_set_enabled(lport, &enabled, 1);
+} else {
+VLOG_ERR("Invalid state '%s' provided to lport-set-enabled", state);
+}
+}
+
+static void
+do_lport_get_enabled(struct ovs_cmdl_context *ctx)
+{
+struct nbctl_context *nb_ctx = ctx->pvt;
+const char *id = ctx->argv[1];
+const struct nbrec_logical_port *lport;
+
+lport = lport_by_name_or_uuid(nb_ctx, id);
+if (!lport) {
+return;
+}
+
+printf("%s\n",
+ (!lport->enabled || *lport->enabled) ? "enabled" : "disabled");
+}
static void
parse_options(int argc, char *argv[])
@@ -753,6 +798,20 @@ static const struct ovs_cmdl_command all_commands[] = {
.max_args = 1,
.handler = do_lport_get_up,
},
+{
+.name = "lport-set-enabled",
+.usage = "LPORT STATE",
+.min_args = 2,
+.max_args = 2,
+.handler = do_lport_set_enabled,
+},
+{
+.name = "lport-get-enabled",
+.usage = "LPORT",
+.min_args = 1,
+.max_args = 1,
+.handler = do_lport_get_enabled,
+},
{
/* sentinel */
--
2.4.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 1/2] ovn: Add logical port 'enabled' state.
This patch adds a new column to the Logical_Port table of the
OVN_Northbound database called 'enabled'. The purpose is to allow a
port to be administratively enabled or disabled. It is sometimes
useful to keep a port and its related configuration, but temporarily
disable it, which means no traffic is allowed in or out of the port.
The implementation is fairly non-invasive as it only required minor
changes to the logical pipeline.
Signed-off-by: Russell Bryant
---
ovn/northd/ovn-northd.c | 14 +++---
ovn/ovn-nb.ovsschema| 1 +
ovn/ovn-nb.xml | 7 +++
3 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 39df3b5..f37df77 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -235,6 +235,12 @@ build_port_security(const char *eth_addr_field,
}
}
+static bool
+lport_is_enabled(const struct nbrec_logical_port *lport)
+{
+return !lport->enabled || *lport->enabled;
+}
+
/* Updates the Pipeline table in the OVN_SB database, constructing its contents
* based on the OVN_NB database. */
static void
@@ -283,7 +289,8 @@ build_pipeline(struct northd_context *ctx)
build_port_security("eth.src",
lport->port_security, lport->n_port_security,
&match);
-pipeline_add(&pc, lport->lswitch, 0, 50, ds_cstr(&match), "next;");
+pipeline_add(&pc, lport->lswitch, 0, 50, ds_cstr(&match),
+ lport_is_enabled(lport) ? "next;" : "drop;");
ds_destroy(&match);
}
@@ -294,7 +301,7 @@ build_pipeline(struct northd_context *ctx)
ds_init(&actions);
NBREC_LOGICAL_PORT_FOR_EACH (lport, ctx->ovnnb_idl) {
-if (lport->lswitch == lswitch) {
+if (lport->lswitch == lswitch && lport_is_enabled(lport)) {
ds_put_cstr(&actions, "outport = ");
json_string_escape(lport->name, &actions);
ds_put_cstr(&actions, "; next; ");
@@ -403,7 +410,8 @@ build_pipeline(struct northd_context *ctx)
lport->port_security, lport->n_port_security,
&match);
-pipeline_add(&pc, lport->lswitch, 3, 50, ds_cstr(&match), "output;");
+pipeline_add(&pc, lport->lswitch, 3, 50, ds_cstr(&match),
+ lport_is_enabled(lport) ? "output;" : "drop;");
ds_destroy(&match);
}
diff --git a/ovn/ovn-nb.ovsschema b/ovn/ovn-nb.ovsschema
index fe69d31..bcbd94b 100644
--- a/ovn/ovn-nb.ovsschema
+++ b/ovn/ovn-nb.ovsschema
@@ -30,6 +30,7 @@
"min": 0,
"max": "unlimited"}},
"up": {"type": {"key": "boolean", "min": 0, "max": 1}},
+"enabled": {"type": {"key": "boolean", "min": 0, "max": 1}},
"external_ids": {
"type": {"key": "string", "value": "string",
"min": 0, "max": "unlimited"}}},
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index b15aeac..a74bf4d 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -126,6 +126,13 @@
become active before it allows the VM (or container) to start.
+
+ This column is used to administratively set port state. If this column
is
+ empty or is set to true, the port is enabled. If this
column
+ is set to false, the port is disabled. A disabled port has
all
+ ingress and egress traffic dropped.
+
+
The logical port's own Ethernet address or addresses, each in the form
xx:xx:xx:xx:xx:xx.
--
2.4.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH net-next V11 0/4] openvswitch: Add support for 802.1AD
V11: Add inner tpid to flow key. Fix separate inner encap attribute when parsing netlink attributes. Merge 2 patches to consolidate qinq changes. V10: Implement reviewer comments: Consolidate vlan parsing functions. Splits netlink parsing and flow conversion into a separate patch. Uses double encap attribute encapsulation for 802.1ad. Netlink attributes now look like this: eth_type(0x88a8),vlan(vid=100),encap(eth_type(0x8100), vlan(vid=200), encap(eth_type(0x0800), ...)) The double encap atributes in this version of the patch is incompatible with old versions of the user level 802.1ad patch. A new user level patch which is also being submitted simultaneously to openvswitch dev mailing list. V9: Includes changes suggested by reviewers V8: Includes changes suggested by reviewers V7: Includes changes suggested by reviewers V6: Rebased to net-next V5: Use encapsulated attributes Although the Open Flow specification specified support for 802.1AD (qinq) as well as push and pop vlan headers, So far Open vSwitch has only supported a single tag header. This patch accompanies version 10 of the user level openvswitch patch submitted to openvswitch dev list. For discussion, history and previous versions of the kernel module patch and the user code patch see the OVS dev mailing list, openvswitch.org/pipermail/dev/.. Thomas F Herbert (3): openvswitch: 802.1ad uapi changes. Check for vlan ethernet types for 8021.q or 802.1ad 802.1AD: Flow handling, actions, vlan parsing and netlink attributes include/linux/if_vlan.h | 9 ++ include/uapi/linux/openvswitch.h | 17 ++-- net/openvswitch/flow.c | 84 ++--- net/openvswitch/flow.h | 5 + net/openvswitch/flow_netlink.c | 195 +-- 5 files changed, 260 insertions(+), 50 deletions(-) -- 2.1.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH net-next V11 1/3] openvswitch: 802.1ad uapi changes.
openvswitch: Add support for 8021.AD
Change the description of the VLAN tpid field.
Signed-off-by: Thomas F Herbert
---
include/uapi/linux/openvswitch.h | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index bbd49a0..f2ccdef 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -559,13 +559,13 @@ struct ovs_action_push_mpls {
* @vlan_tci: Tag control identifier (TCI) to push. The CFI bit must be set
* (but it will not be set in the 802.1Q header that is pushed).
*
- * The @vlan_tpid value is typically %ETH_P_8021Q. The only acceptable TPID
- * values are those that the kernel module also parses as 802.1Q headers, to
- * prevent %OVS_ACTION_ATTR_PUSH_VLAN followed by %OVS_ACTION_ATTR_POP_VLAN
- * from having surprising results.
+ * The @vlan_tpid value is typically %ETH_P_8021Q or %ETH_P_8021AD.
+ * The only acceptable TPID values are those that the kernel module also parses
+ * as 802.1Q or 802.1AD headers, to prevent %OVS_ACTION_ATTR_PUSH_VLAN followed
+ * by %OVS_ACTION_ATTR_POP_VLAN from having surprising results.
*/
struct ovs_action_push_vlan {
- __be16 vlan_tpid; /* 802.1Q TPID. */
+ __be16 vlan_tpid; /* 802.1Q or 802.1ad TPID. */
__be16 vlan_tci;/* 802.1Q TCI (VLAN ID and priority). */
};
@@ -605,9 +605,10 @@ struct ovs_action_hash {
* is copied from the value to the packet header field, rest of the bits are
* left unchanged. The non-masked value bits must be passed in as zeroes.
* Masking is not supported for the %OVS_KEY_ATTR_TUNNEL attribute.
- * @OVS_ACTION_ATTR_PUSH_VLAN: Push a new outermost 802.1Q header onto the
- * packet.
- * @OVS_ACTION_ATTR_POP_VLAN: Pop the outermost 802.1Q header off the packet.
+ * @OVS_ACTION_ATTR_PUSH_VLAN: Push a new outermost 802.1Q or 802.1ad header
+ * onto the packet.
+ * @OVS_ACTION_ATTR_POP_VLAN: Pop the outermost 802.1Q or 802.1ad header
+ * from the packet.
* @OVS_ACTION_ATTR_SAMPLE: Probabilitically executes actions, as specified in
* the nested %OVS_SAMPLE_ATTR_* attributes.
* @OVS_ACTION_ATTR_PUSH_MPLS: Push a new MPLS label stack entry onto the
--
2.1.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH net-next V11 2/3] Check for vlan ethernet types for 8021.q or 802.1ad
Signed-off-by: Thomas F Herbert
---
include/linux/if_vlan.h | 9 +
1 file changed, 9 insertions(+)
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 920e445..3713454 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -627,5 +627,14 @@ static inline netdev_features_t vlan_features_check(const
struct sk_buff *skb,
return features;
}
+/**
+ * Check for legal valid vlan ether type.
+ */
+static inline bool eth_type_vlan(__be16 ethertype)
+{
+ if (ethertype == htons(ETH_P_8021Q) || ethertype == htons(ETH_P_8021AD))
+ return true;
+ return false;
+}
#endif /* !(_LINUX_IF_VLAN_H_) */
--
2.1.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH net-next 3/3] openvswitch: 802.1AD: Flow handling, actions, and vlan parsing
Add support for 802.1ad including the ability to push and pop double
tagged vlans. Add support for 802.1ad to netlink parsing and flow
conversion. Uses double nested encap attributes to represent double
tagged vlan. Inner TPID encoded along with ctci in nested attributes.
Signed-off-by: Thomas F Herbert
---
net/openvswitch/flow.c | 84 +++---
net/openvswitch/flow.h | 5 ++
net/openvswitch/flow_netlink.c | 195 ++---
3 files changed, 242 insertions(+), 42 deletions(-)
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 2dacc7b..e268865 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -298,21 +298,80 @@ static bool icmp6hdr_ok(struct sk_buff *skb)
static int parse_vlan(struct sk_buff *skb, struct sw_flow_key *key)
{
struct qtag_prefix {
- __be16 eth_type; /* ETH_P_8021Q */
+ __be16 eth_type; /* ETH_P_8021Q or ETH_P_8021AD */
__be16 tci;
};
- struct qtag_prefix *qp;
+ struct qtag_prefix *qp = (struct qtag_prefix *)skb->data;
- if (unlikely(skb->len < sizeof(struct qtag_prefix) + sizeof(__be16)))
+ struct qinqtag_prefix {
+ __be16 eth_type; /* ETH_P_8021Q or ETH_P_8021AD */
+ __be16 tci;
+ __be16 inner_tpid; /* ETH_P_8021Q */
+ __be16 ctci;
+ };
+
+ if (likely(skb_vlan_tag_present(skb))) {
+ key->eth.tci = htons(skb->vlan_tci);
+
+ /* Case where upstream
+* processing has already stripped the outer vlan tag.
+*/
+ if (unlikely(skb->vlan_proto == htons(ETH_P_8021AD))) {
+ if (unlikely(skb->len < sizeof(struct qtag_prefix) +
+ sizeof(__be16))) {
+ key->eth.tci = 0;
+ return 0;
+ }
+
+ if (unlikely(!pskb_may_pull(skb,
+ sizeof(struct qtag_prefix) +
+ sizeof(__be16 {
+ return -ENOMEM;
+ }
+
+ if (likely(qp->eth_type == htons(ETH_P_8021Q))) {
+ key->eth.cvlan.ctci =
+ qp->tci | htons(VLAN_TAG_PRESENT);
+ key->eth.cvlan.c_tpid = skb->vlan_proto;
+ __skb_pull(skb, sizeof(struct qtag_prefix));
+ }
+ }
return 0;
+ }
- if (unlikely(!pskb_may_pull(skb, sizeof(struct qtag_prefix) +
-sizeof(__be16
- return -ENOMEM;
- qp = (struct qtag_prefix *) skb->data;
- key->eth.tci = qp->tci | htons(VLAN_TAG_PRESENT);
- __skb_pull(skb, sizeof(struct qtag_prefix));
+ if (qp->eth_type == htons(ETH_P_8021AD)) {
+ struct qinqtag_prefix *qinqp =
+ (struct qinqtag_prefix *)skb->data;
+
+ if (unlikely(skb->len < sizeof(struct qinqtag_prefix) +
+ sizeof(__be16)))
+ return 0;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(struct qinqtag_prefix) +
+ sizeof(__be16 {
+ return -ENOMEM;
+ }
+ key->eth.tci = qinqp->tci | htons(VLAN_TAG_PRESENT);
+ key->eth.cvlan.ctci = qinqp->ctci | htons(VLAN_TAG_PRESENT);
+ key->eth.cvlan.c_tpid = qinqp->inner_tpid;
+
+ __skb_pull(skb, sizeof(struct qinqtag_prefix));
+
+ return 0;
+ }
+ if (qp->eth_type == htons(ETH_P_8021Q)) {
+ if (unlikely(skb->len < sizeof(struct qtag_prefix) +
+ sizeof(__be16)))
+ return -ENOMEM;
+
+ if (unlikely(!pskb_may_pull(skb, sizeof(struct qtag_prefix) +
+ sizeof(__be16
+ return 0;
+ key->eth.tci = qp->tci | htons(VLAN_TAG_PRESENT);
+
+ __skb_pull(skb, sizeof(struct qtag_prefix));
+ }
return 0;
}
@@ -474,9 +533,10 @@ static int key_extract(struct sk_buff *skb, struct
sw_flow_key *key)
*/
key->eth.tci = 0;
- if (skb_vlan_tag_present(skb))
- key->eth.tci = htons(skb->vlan_tci);
- else if (eth->h_proto == htons(ETH_P_8021Q))
+ key->eth.cvlan.ctci = 0;
+ if ((skb_vlan_tag_present(skb)) ||
+ (eth->h_proto == htons(ETH_P_8021Q)) ||
+ (eth->h_proto == htons(ETH_P_8021AD)))
if (unlikely(parse_vlan(skb, key)))
return -ENOMEM;
diff --git a/net/openvswitch/flow.h b/net/op
[ovs-dev] [PATCH V11 0/4] Add 802.1ad (qinq) support
From: "Thomas F. Herbert" Version 11: Encode inner tpid in with customer tci into netlink in inner encapsulation. Some cleanup in code and comments. The patch adding 802.1ad support functions, which is already merged is removed from this series. This patch accompanies V11 kernel module patch submitted to linux net-next. Version 10: Use doubly nested encap attributes to encode 802.1ad. Rebase to master. V8 and V9 were skipped and this version was called V10 to accompany V10 kernel module patch simultaneously to net-next. This patch will is incopatible with old versions of the kernel module and will require V10 of the kernel module patch because of the change in netlink attribute encoding of 802.1ad. Netlink attributes now look like this: eth_type(0x88a8),vlan(vid=100),encap(eth_type(0x8100), vlan(vid=200), encap(eth_type(0x0800), ...)) Version 7 is rebased to latest master and incorporates changes from reviewers. The accompanying kernel changes have been submitted to the linux net-dev mailing list. Version 6 split the changes into separate patches to make it easier on the eyes of reviewers but otherwise is functionally the same as version 5. Version 5 updated the patch to properly use nested attributes, fixed some whitespace problems and allows TPID 0x88a8 tag be pushed with either single or outer tag because Open Flow does not specifically prohibit it. This patch has been tested along with the Linux Kernel datapath patch in a VM based test network as well as real-world provider test environment. Thanks to Robert Peterson for inspiring and leading the effort of separating customer from provider traffic which led to this work and thanks to the people at ATC for helping with configuring the test lab and running perf tests. Also, thanks to Entry Point LLC who were involved in this project from the beginning. Thanks to Ben Pfaff, Pravin Shelar and others that have commented on previous versions of this patch and thanks to Dave Benson who contributed the test included in this patch. Thomas F. Herbert (4): Flow and action changes for 802.1AD Flow key and netlink parsing changes for 802.1AD 802.1AD: Describe 802.1ad changes Test push and pop of vlan with both 802.1AD and 802.1Q NEWS | 2 + lib/flow.c | 14 +-- lib/flow.h | 16 ++- lib/match.c | 2 +- lib/nx-match.c | 2 +- lib/odp-util.c | 289 +-- lib/odp-util.h | 2 +- lib/ofp-actions.c| 32 +++-- lib/ofp-actions.h| 14 ++- lib/ofp-util.c | 2 +- ofproto/ofproto-dpif-rid.h | 2 +- ofproto/ofproto-dpif-xlate.c | 13 +- tests/ofproto-dpif.at| 40 ++ utilities/ovs-ofctl.8.in | 3 +- 14 files changed, 359 insertions(+), 74 deletions(-) -- 2.1.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH V11 2/4] Flow key and netlink parsing changes for 802.1AD
From: "Thomas F. Herbert"
Netlink parsing and flow key conversion. Netlink attribute encoding is done
with double encap attributes. Netlink attributes for 802.1ad look like
the following:
eth_type(0x88a8),vlan(vid=100),encap(eth_type(0x8100), vlan(vid=200),
encap(eth_type(0x0800), ...))
V11 fixes issues in the inner vlan encapsulation.
Signed-off-by: Thomas F Herbert
---
lib/odp-util.c | 289 +
lib/odp-util.h | 2 +-
2 files changed, 252 insertions(+), 39 deletions(-)
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 56979ac..bcd33c8 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -3421,7 +3421,8 @@ odp_flow_key_from_flow__(const struct odp_flow_key_parms
*parms,
bool export_mask, struct ofpbuf *buf)
{
struct ovs_key_ethernet *eth_key;
-size_t encap;
+size_t encap = 0;
+size_t inner_encap = 0;
const struct flow *flow = parms->flow;
const struct flow *data = export_mask ? parms->mask : parms->flow;
@@ -3448,19 +3449,45 @@ odp_flow_key_from_flow__(const struct
odp_flow_key_parms *parms,
sizeof *eth_key);
get_ethernet_key(data, eth_key);
-if (flow->vlan_tci != htons(0) || flow->dl_type == htons(ETH_TYPE_VLAN)) {
+if ((flow->vlan_tci != htons(0)) || (flow->dl_type ==
+htons(ETH_TYPE_VLAN_8021Q))) {
if (export_mask) {
nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, OVS_BE16_MAX);
} else {
-nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, htons(ETH_TYPE_VLAN));
+nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE,
+htons(ETH_TYPE_VLAN_8021Q));
+}
+nl_msg_put_be16(buf, OVS_KEY_ATTR_VLAN, data->vlan_tci);
+encap = nl_msg_start_nested(buf, OVS_KEY_ATTR_ENCAP);
+if (flow->vlan_tci == htons(0)) {
+goto unencap;
+}
+} else if ((flow->vlan_ctci != htons(0)) || (flow->dl_type ==
+ htons(ETH_TYPE_VLAN_8021AD))) {
+if (export_mask) {
+nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, OVS_BE16_MAX);
+} else {
+nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE,
+htons(ETH_TYPE_VLAN_8021AD));
+}
+nl_msg_put_be16(buf, OVS_KEY_ATTR_VLAN, data->vlan_ctci);
+inner_encap = nl_msg_start_nested(buf, OVS_KEY_ATTR_ENCAP);
+if (flow->vlan_ctci == htons(0)) {
+goto unencap;
+}
+/* Now, code the inner customer vlan and then another layer of
+ * nesting for the payload. */
+if (export_mask) {
+nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE, OVS_BE16_MAX);
+} else {
+nl_msg_put_be16(buf, OVS_KEY_ATTR_ETHERTYPE,
+htons(ETH_TYPE_VLAN_8021Q));
}
nl_msg_put_be16(buf, OVS_KEY_ATTR_VLAN, data->vlan_tci);
encap = nl_msg_start_nested(buf, OVS_KEY_ATTR_ENCAP);
if (flow->vlan_tci == htons(0)) {
goto unencap;
}
-} else {
-encap = 0;
}
if (ntohs(flow->dl_type) < ETH_TYPE_MIN) {
@@ -3575,6 +3602,9 @@ odp_flow_key_from_flow__(const struct odp_flow_key_parms
*parms,
}
unencap:
+if (inner_encap) {
+nl_msg_end_nested(buf, inner_encap);
+}
if (encap) {
nl_msg_end_nested(buf, encap);
}
@@ -4096,9 +4126,36 @@ done:
key, key_len);
}
-/* Parse 802.1Q header then encapsulated L3 attributes. */
static enum odp_key_fitness
-parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
+parse_encap_and_vlan(uint64_t present_attrs, int out_of_range_attr,
+ uint64_t expected_attrs, struct flow *flow,
+ const struct nlattr *key, size_t key_len,
+ const struct flow *src_flow)
+{
+bool is_mask = src_flow != flow;
+enum odp_key_fitness fitness;
+
+/* Calculate fitness of vlan and encap attributes. */
+if (!is_mask) {
+expected_attrs |= ((UINT64_C(1) << OVS_KEY_ATTR_VLAN) |
+ (UINT64_C(1) << OVS_KEY_ATTR_ENCAP));
+} else {
+if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_VLAN)) {
+expected_attrs |= (UINT64_C(1) << OVS_KEY_ATTR_VLAN);
+}
+if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_ENCAP)) {
+expected_attrs |= (UINT64_C(1) << OVS_KEY_ATTR_ENCAP);
+}
+}
+fitness = check_expectations(present_attrs, out_of_range_attr,
+ expected_attrs, key, key_len);
+
+return fitness;
+}
+
+/* Parse 802.1AD inner vlan and then parse encapsulated Attributes. */
+static enum odp_key_fitness
+parse_cvlan_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
uint64_t present_attrs, int out_of_range_attr,
uint64_t expected_attrs, struct
[ovs-dev] [PATCH V11 1/4] Flow and action changes for 802.1AD
From: "Thomas F. Herbert"
The flow structure is updated to hold the customer tci.
Flow key extraction is changed to add support for ctci and both TPIDs.
Parsing to support pushing and popping with both single and double
tagged vlans. In response to reviewers comments on V6, all changes
affected by the flow structure are gathered in this patch.
Signed-off-by: Thomas F Herbert
---
lib/flow.c | 14 +++---
lib/flow.h | 16 +++-
lib/match.c | 2 +-
lib/nx-match.c | 2 +-
lib/ofp-actions.c| 32 +++-
lib/ofp-actions.h| 14 +++---
lib/ofp-util.c | 2 +-
ofproto/ofproto-dpif-rid.h | 2 +-
ofproto/ofproto-dpif-xlate.c | 13 -
9 files changed, 64 insertions(+), 33 deletions(-)
diff --git a/lib/flow.c b/lib/flow.c
index 3e99d5e..3bfcb26 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -123,7 +123,7 @@ struct mf_ctx {
* away. Some GCC versions gave warnings on ALWAYS_INLINE, so these are
* defined as macros. */
-#if (FLOW_WC_SEQ != 31)
+#if (FLOW_WC_SEQ != 32)
#define MINIFLOW_ASSERT(X) ovs_assert(X)
BUILD_MESSAGE("FLOW_WC_SEQ changed: miniflow_extract() will have runtime "
"assertions enabled. Consider updating FLOW_WC_SEQ after "
@@ -291,7 +291,7 @@ parse_vlan(const void **datap, size_t *sizep)
data_pull(datap, sizep, ETH_ADDR_LEN * 2);
-if (eth->eth_type == htons(ETH_TYPE_VLAN)) {
+if (eth_type_vlan(eth->eth_type)) {
if (OVS_LIKELY(*sizep
>= sizeof(struct qtag_prefix) + sizeof(ovs_be16))) {
const struct qtag_prefix *qp = data_pull(datap, sizep, sizeof *qp);
@@ -766,7 +766,7 @@ flow_get_metadata(const struct flow *flow, struct match
*flow_metadata)
{
int i;
-BUILD_ASSERT_DECL(FLOW_WC_SEQ == 31);
+BUILD_ASSERT_DECL(FLOW_WC_SEQ == 32);
match_init_catchall(flow_metadata);
if (flow->tunnel.tun_id != htonll(0)) {
@@ -942,7 +942,7 @@ void flow_wildcards_init_for_packet(struct flow_wildcards
*wc,
memset(&wc->masks, 0x0, sizeof wc->masks);
/* Update this function whenever struct flow changes. */
-BUILD_ASSERT_DECL(FLOW_WC_SEQ == 31);
+BUILD_ASSERT_DECL(FLOW_WC_SEQ == 32);
if (flow->tunnel.ip_dst) {
if (flow->tunnel.flags & FLOW_TNL_F_KEY) {
@@ -1041,7 +1041,7 @@ uint64_t
flow_wc_map(const struct flow *flow)
{
/* Update this function whenever struct flow changes. */
-BUILD_ASSERT_DECL(FLOW_WC_SEQ == 31);
+BUILD_ASSERT_DECL(FLOW_WC_SEQ == 32);
uint64_t map = (flow->tunnel.ip_dst) ? MINIFLOW_MAP(tunnel) : 0;
@@ -1093,7 +1093,7 @@ void
flow_wildcards_clear_non_packet_fields(struct flow_wildcards *wc)
{
/* Update this function whenever struct flow changes. */
-BUILD_ASSERT_DECL(FLOW_WC_SEQ == 31);
+BUILD_ASSERT_DECL(FLOW_WC_SEQ == 32);
memset(&wc->masks.metadata, 0, sizeof wc->masks.metadata);
memset(&wc->masks.regs, 0, sizeof wc->masks.regs);
@@ -1648,7 +1648,7 @@ flow_push_mpls(struct flow *flow, int n, ovs_be16
mpls_eth_type,
flow->mpls_lse[0] = set_mpls_lse_values(ttl, tc, 1, htonl(label));
/* Clear all L3 and L4 fields and dp_hash. */
-BUILD_ASSERT(FLOW_WC_SEQ == 31);
+BUILD_ASSERT(FLOW_WC_SEQ == 32);
memset((char *) flow + FLOW_SEGMENT_2_ENDS_AT, 0,
sizeof(struct flow) - FLOW_SEGMENT_2_ENDS_AT);
flow->dp_hash = 0;
diff --git a/lib/flow.h b/lib/flow.h
index 70554e4..9d34775 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -39,7 +39,7 @@ struct match;
/* This sequence number should be incremented whenever anything involving flows
* or the wildcarding of flows changes. This will cause build assertion
* failures in places which likely need to be updated. */
-#define FLOW_WC_SEQ 31
+#define FLOW_WC_SEQ 32
/* Number of Open vSwitch extension 32-bit registers. */
#define FLOW_N_REGS 8
@@ -114,7 +114,13 @@ struct flow {
uint8_t dl_dst[ETH_ADDR_LEN]; /* Ethernet destination address. */
uint8_t dl_src[ETH_ADDR_LEN]; /* Ethernet source address. */
ovs_be16 dl_type; /* Ethernet frame type. */
-ovs_be16 vlan_tci; /* If 802.1Q, TCI | VLAN_CFI; otherwise 0. */
+ovs_be16 vlan_tci; /* If 802.1Q, TCI | VLAN_CFI; If 802.1ad,
+ * outer tag, otherwise 0. */
+ovs_be16 vlan_ctci; /* If 802.1ad, Customer TCI | VLAN_CFI,
+ * inner tag, otherwise 0. */
+ovs_be16 vlan_tpid; /* Vlan protocol type,
+ * either 802.1ad or 802.1q. */
+uint32_t pad2; /* Pad to 64 bits. */
ovs_be32 mpls_lse[ROUND_UP(FLOW_MAX_MPLS_LABELS, 2)]; /* MPLS label stack
(with padding). */
/* L3 (64-bit aligned) */
@@ -131,7 +137,7 @@ struct flow {
uint8_t arp_sha[ETH_AD
[ovs-dev] [PATCH] ofproto: Fix use-after-free in bridge destruction with groups.
Groups were not destroyed until after lots of other important bridge
data had been destroyed, including the connection manager. There was an
indirect dependency on the connection manager for bridge destruction
because destroying a group also destroys all of the flows that reference
the group, which in turn causes the ofmonitor to be invoked to report that
the flows had been destroyed. This commit fixes the problem by destroying
groups earlier.
The problem can be observed by reverting the code changes in this commit
then running "make check-valgrind" with the test that this commit
introduces.
Reported-by: Simon Horman
Signed-off-by: Ben Pfaff
---
ofproto/ofproto-dpif.c | 1 +
ofproto/ofproto-provider.h | 5 -
ofproto/ofproto.c | 11 ++-
tests/ofproto.at | 13 +
4 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 05a80b7..ac3e48a 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1403,6 +1403,7 @@ destruct(struct ofproto *ofproto_)
ofproto_rule_delete(&ofproto->up, &rule->up);
}
}
+ofproto_group_delete_all(&ofproto->up);
guarded_list_pop_all(&ofproto->pins, &pins);
LIST_FOR_EACH_POP (pin, list_node, &pins) {
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 527823a..e5739b0 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -521,6 +521,8 @@ bool ofproto_group_lookup(const struct ofproto *ofproto,
uint32_t group_id,
void ofproto_group_ref(struct ofgroup *);
void ofproto_group_unref(struct ofgroup *);
+void ofproto_group_delete_all(struct ofproto *);
+
/* ofproto class structure, to be defined by each ofproto implementation.
*
*
@@ -742,7 +744,8 @@ struct ofproto_class {
* ===
*
* ->destruct() must also destroy all remaining rules in the ofproto's
- * tables, by passing each remaining rule to ofproto_rule_delete().
+ * tables, by passing each remaining rule to ofproto_rule_delete(), then
+ * destroy all remaining groups by calling ofproto_group_delete_all().
*
* The client will destroy the flow tables themselves after ->destruct()
* returns.
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 08ba043..f0e6c9a 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1530,7 +1530,6 @@ ofproto_destroy__(struct ofproto *ofproto)
struct oftable *table;
destroy_rule_executes(ofproto);
-delete_group(ofproto, OFPG_ALL);
guarded_list_destroy(&ofproto->rule_executes);
ovs_rwlock_destroy(&ofproto->groups_rwlock);
@@ -6504,6 +6503,16 @@ delete_group(struct ofproto *ofproto, uint32_t group_id)
ovs_rwlock_unlock(&ofproto->groups_rwlock);
}
+/* Delete all groups from 'ofproto'.
+ *
+ * This is intended for use within an ofproto provider's 'destruct'
+ * function. */
+void
+ofproto_group_delete_all(struct ofproto *ofproto)
+{
+delete_group(ofproto, OFPG_ALL);
+}
+
static enum ofperr
handle_group_mod(struct ofconn *ofconn, const struct ofp_header *oh)
{
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 08585d1..6cf422f 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -679,6 +679,19 @@ OFPST_GROUP reply (OF1.5):
OVS_VSWITCHD_STOP
AT_CLEANUP
+dnl This found a use-after-free error in bridge destruction in the
+dnl presence of groups.
+AT_SETUP([ofproto - group add then bridge delete (OpenFlow 1.3)])
+OVS_VSWITCHD_START
+AT_DATA([groups.txt], [dnl
+group_id=1234,type=all,bucket=output:10
+group_id=1235,type=all,bucket=output:10
+])
+AT_CHECK([ovs-ofctl -O OpenFlow13 -vwarn add-groups br0 groups.txt])
+AT_CHECK([ovs-vsctl del-br br0])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
AT_SETUP([ofproto - mod-port (OpenFlow 1.0)])
OVS_VSWITCHD_START
for command_config_state in \
--
2.1.3
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH V11 3/4] 802.1AD: Describe 802.1ad changes
From: "Thomas F. Herbert" Signed-off-by: Thomas F Herbert --- NEWS | 2 ++ utilities/ovs-ofctl.8.in | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index a0664dc..581da59 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,8 @@ Post-v2.4.0 v2.4.0 - xx xxx - + - Add support for 8021.AD as specified in OpenFlow 1.1+. Adds push and pop + of 802.1AD Ethertype and handling of QinQ or double stacked Vlans. - Flow table modifications are now atomic, meaning that each packet now sees a coherent version of the OpenFlow pipeline. For example, if a controller removes all flows with a single OpenFlow diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in index 63c2ecc..5ab252d 100644 --- a/utilities/ovs-ofctl.8.in +++ b/utilities/ovs-ofctl.8.in @@ -1327,8 +1327,7 @@ Strips the VLAN tag from a packet if it is present. . .IP \fBpush_vlan\fR:\fIethertype\fR Push a new VLAN tag onto the packet. Ethertype is used as the the Ethertype -for the tag. Only ethertype 0x8100 should be used. (0x88a8 which the spec -allows isn't supported at the moment.) +for the tag. Ethertypes 0x8100 or 0x88a8 may be used. A priority of zero and the tag of zero are used for the new tag. . .IP \fBpush_mpls\fR:\fIethertype\fR -- 2.1.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH/RFC] connmgr: clear ofproto's pointer to connmgr when the latter is destroyed
On Wed, Jun 17, 2015 at 02:22:32PM +0900, Simon Horman wrote: > As per the testcase included in this patch it has been observed > that ovs-vswtichd may segfault when deleting a bridge. > > Analysing the output of valgrind and gdb it appears that > this is caused by the connmgr of a ofproto being accessed > after the latter has been freed. > > It appears that this is occurring in different threads and > is the result of the following postponement arrangement in ofproto_destroy(); > > /* We should not postpone this because it involves deleting a listening > * socket which we may want to reopen soon. 'connmgr' should not be used > * by other threads */ > connmgr_destroy(p->connmgr); > > /* Destroying rules is deferred, must have 'ofproto' around for them. */ > ovsrcu_postpone(ofproto_destroy__, p); Thanks a lot for the report and the fix. I found what I think is a cleaner fix: http://openvswitch.org/pipermail/dev/2015-June/056716.html Will you review it? Thanks! ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] INSTALL.DPDK: remove experimental statement
Do you two have an opinion on this? If DPDK support is pretty solid now then it makes sense to apply this to master and backport it to branch-2.4. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH V11 4/4] Test push and pop of vlan with both 802.1AD and 802.1Q
From: "Thomas F. Herbert" This test tests the user space actions for 802.1q and 802.1ad. Signed-off-by: Thomas F Herbert Signed-off-by: Dave Benson --- tests/ofproto-dpif.at | 40 1 file changed, 40 insertions(+) diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 7f20786..37d67ff 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -1541,6 +1541,46 @@ NXST_FLOW reply: OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([ofproto-dpif - VLAN with 802.1AD Ethertype]) +OVS_VSWITCHD_START +ADD_OF_PORTS([br0], [1], [2], [3], [4]) + +AT_DATA([flows1.txt], [dnl +table=0 in_port=1 dl_type=0x8100 actions=pop_vlan,output:2 +table=0 in_port=1 dl_type=0x88a8 actions=pop_vlan,output:3 +]) +AT_CHECK([ovs-ofctl add-flows br0 flows1.txt]) +AT_DATA([flows2.txt], [dnl +table=0 in_port=2 dl_type=0x0800 actions=push_vlan:0x8100,mod_vlan_vid:9,output:1 +table=0 in_port=3 dl_type=0x0800 actions=push_vlan:0x88a8,set_field:0x1006->vlan_tci,output:1 +table=0 in_port=4 dl_type=0x0800 actions=push_vlan:0x88a8,mod_vlan_vid:11,output:1 +]) +AT_CHECK([ovs-ofctl -O OpenFlow11 add-flows br0 flows2.txt]) + +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x8100,dl_vlan=9'], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: pop_vlan,2 +]) +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=1,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x88a8,dl_vlan=9'], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: pop_vlan,3 +]) +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=2,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x0800'], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: push_vlan(vid=9,pcp=0),1 +]) +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=3,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x0800'], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: push_vlan(tpid=0x88a8,vid=6,pcp=0),1 +]) +AT_CHECK([ovs-appctl ofproto/trace br0 'in_port=4,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x0800'], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], + [Datapath actions: push_vlan(tpid=0x88a8,vid=11,pcp=0),1 +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([ofproto-dpif - MPLS handling]) OVS_VSWITCHD_START([dnl add-port br0 p1 -- set Interface p1 type=dummy -- 2.1.0 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH net-next V11 2/3] Check for vlan ethernet types for 8021.q or 802.1ad
Hello.
On 06/23/2015 09:26 PM, Thomas F Herbert wrote:
Signed-off-by: Thomas F Herbert
[...]
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 920e445..3713454 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -627,5 +627,14 @@ static inline netdev_features_t vlan_features_check(const
struct sk_buff *skb,
return features;
}
+/**
+ * Check for legal valid vlan ether type.
The comment doesn't follow the kernel-doc format determined by /**.
+ */
+static inline bool eth_type_vlan(__be16 ethertype)
+{
+ if (ethertype == htons(ETH_P_8021Q) || ethertype == htons(ETH_P_8021AD))
+ return true;
+ return false;
Perhaps *switch*?
[...]
WBR, Sergei
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] INSTALL.DPDK: remove experimental statement
On 6/17/15 2:41 PM, Flavio Leitner wrote: Signed-off-by: Flavio Leitner --- INSTALL.DPDK.md | 3 --- 1 file changed, 3 deletions(-) It looks like a good time to promote DPDK support before 2.4 is branched off. What do you think? +1 diff --git a/INSTALL.DPDK.md b/INSTALL.DPDK.md index cdef6cf..a2d012e 100644 --- a/INSTALL.DPDK.md +++ b/INSTALL.DPDK.md @@ -5,9 +5,6 @@ Open vSwitch can use Intel(R) DPDK lib to operate entirely in userspace. This file explains how to install and use Open vSwitch in such a mode. -The DPDK support of Open vSwitch is considered experimental. -It has not been thoroughly tested. - This version of Open vSwitch should be built manually with `configure` and `make`. -- Thomas F. Herbert ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] datapath-windows: Return success for already existing WFP objects
On Tue, Jun 23, 2015 at 12:02:44AM +, Nithin Raju wrote: > > On Jun 22, 2015, at 10:58 AM, Sorin Vinturis > > wrote: > > > > Hi Nithin, > > > > I didn't thought about your idea. I have added instrumentation code in > > order to force the EEXIST error and test the patch and the code performs as > > expected. The extension is able to be activated, in case of both the > > sublayer and callout are already present, and everything works as fine. > > Sounds good. I guess when we cannot replicate real failures, we need to get > creative to test the code added :) This sounded like a positive review, so I applied this to master and branch-2.4. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] datapath-windows: Return success for already existing WFP objects
> On Jun 23, 2015, at 11:46 AM, Ben Pfaff wrote: > > On Tue, Jun 23, 2015 at 12:02:44AM +, Nithin Raju wrote: >>> On Jun 22, 2015, at 10:58 AM, Sorin Vinturis >>> wrote: >>> >>> Hi Nithin, >>> >>> I didn't thought about your idea. I have added instrumentation code in >>> order to force the EEXIST error and test the patch and the code performs as >>> expected. The extension is able to be activated, in case of both the >>> sublayer and callout are already present, and everything works as fine. >> >> Sounds good. I guess when we cannot replicate real failures, we need to get >> creative to test the code added :) > > This sounded like a positive review, so I applied this to master and > branch-2.4. Thanks Ben. For some reason, I thought this was already applied and hence didn’t ACK. Yes, we are good to go with this patch. -- Nithin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH net-next V11 2/3] Check for vlan ethernet types for 8021.q or 802.1ad
On 6/23/15 2:43 PM, Sergei Shtylyov wrote:
Hello.
On 06/23/2015 09:26 PM, Thomas F Herbert wrote:
Signed-off-by: Thomas F Herbert
[...]
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 920e445..3713454 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -627,5 +627,14 @@ static inline netdev_features_t
vlan_features_check(const struct sk_buff *skb,
return features;
}
+/**
+ * Check for legal valid vlan ether type.
The comment doesn't follow the kernel-doc format determined by /**.
OK, I will change it to the proper kernel doc format.
+ */
+static inline bool eth_type_vlan(__be16 ethertype)
+{
+if (ethertype == htons(ETH_P_8021Q) || ethertype ==
htons(ETH_P_8021AD))
+return true;
+return false;
Perhaps *switch*?
I have no objection to changing this to a switch statement.
[...]
WBR, Sergei
--
Thomas F. Herbert
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 0/2] ovn: Add administrative state to logical ports.
On 06/23/2015 02:22 PM, Russell Bryant wrote: > While working on OpenStack Neutron integration for OVN, I came across a small > feature gap. The Neutron API supports setting a port as administratively > down. > One way to implement that would be to delete ports from OVN while > administratively down. However, it seemed to me that it would be nice to keep > the list of ports that currently exist in sync between the CMS (Neutron in > this > case) and OVN. This patch is a first attempt at supporting this OVN by > updating > the Pipeline to drop packets to/from a port that is administratively down. > > [PATCH 1/2] ovn: Add logical port 'enabled' state. > [PATCH 2/2] ovn: Add get/set-enabled to ovn-nbctl. > > northd/ovn-northd.c | 14 +--- > ovn-nb.ovsschema|1 > ovn-nb.xml |7 ++ > ovn-nbctl.8.xml | 13 +++ > ovn-nbctl.c | 59 > > 5 files changed, 91 insertions(+), 3 deletions(-) > After some more looking around, it seems the appropriate way to do this is with the equivalent of "ovs-ofctl mod-port". I'll look into how to do that some more. Consider this a RFC on the idea and approach, though. :-) -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath-windows: Remove the external/internal port only if it is removed on the Hyper-V switch
Should I take that as a positive review? On Fri, Jun 19, 2015 at 06:51:17AM +, Nithin Raju wrote: > Thanks Alin. > > > On Jun 18, 2015, at 2:00 PM, Alin Serdean > > wrote: > > > > Delete internal port > > Test connectivity > > Add internal port > > Test connectivity > > > > Delete external port > > Test connectivity > > Add external port > > Test connectivity > > > > Delete internal port > > Delete external port > > Test connectivity > > Add internal port > > Test connectivity > > Add external port > > Test connectivity > > > > Uninstall extension verify port deletion. > > > > Alin. > > > > -Mesaj original- > > De la: dev [mailto:dev-boun...@openvswitch.org] În numele Nithin Raju > > Trimis: Thursday, June 18, 2015 11:30 PM > > Către: Sorin Vinturis > > Cc: dev@openvswitch.org > > Subiect: Re: [ovs-dev] [PATCH] datapath-windows: Remove the > > external/internal port only if it is removed on the Hyper-V switch > > > >> On Jun 18, 2015, at 11:52 AM, Sorin Vinturis > >> wrote: > >> > >> Disconnecting the NIC from HV switch and then unloading the driver > >> hits an ASSERT (ASSERT(switchContext->numPhysicalNics);). > >> > >> Also if the external or internal port is deleted from the userspace, > >> the latter ports would not be re-added until the extension would be > >> reloaded. > >> > >> This patch ensures that the internal or external ports are being > >> deleted only by the Hyper-V switch mechanisms, which also solves the > >> issue raised by the above assert. > >> > >> Signed-off-by: Sorin Vinturis > >> Co-authored-by: Alin Gabriel Serdean > >> Reported-by: Sorin Vinturis > > > > Just curious, was any unit testing performed to test the specific scenario > > we are claiming to fix? > > > > thanks, > > -- Nithin > > ___ > > dev mailing list > > dev@openvswitch.org > > https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=q1cb-T2pctLQnNJ2OwKlv_RsKbtpJ8FPjBJ-W5p_9vU&s=nWK_z-UHuGNF1B0MBF0RTfbwqrzr85wc920kbDvQWCU&e= > > > > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] datapath-windows: Remove the external/internal port only if it is removed on the Hyper-V switch
On Thu, Jun 18, 2015 at 09:05:32PM +, Nithin Raju wrote: > > On Jun 18, 2015, at 1:57 PM, Alin Serdean > > wrote: > > > > I have no issue with it if it is tested. > > > > Yes Nithin you are right we agreed that you will work on it but it was in > > the backlog for some time and it is a show stopper because it also > > fixes(https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_62&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=mGRgSLlhBsu9jwYpHagt2xLxS1GSklQgTGXxMSuEL-g&s=YIhRLJ49Q4cetS_HYw_jccyA5VQFWGMPvnX6bq7yGKY&e= > > ). > > I’ll send out my fix today. I ran into a crash while testing > creation/deletion of vxlan port. Debugging that right now. > > Thanks for your understanding. Does this mean that you're working on a different fix? Or is this patch one that should be applied? Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] datapath-windows: Remove the external/internal port only if it is removed on the Hyper-V switch
Oh I see there's a v2. On Tue, Jun 23, 2015 at 12:40:32PM -0700, Ben Pfaff wrote: > Should I take that as a positive review? > > On Fri, Jun 19, 2015 at 06:51:17AM +, Nithin Raju wrote: > > Thanks Alin. > > > > > On Jun 18, 2015, at 2:00 PM, Alin Serdean > > > wrote: > > > > > > Delete internal port > > > Test connectivity > > > Add internal port > > > Test connectivity > > > > > > Delete external port > > > Test connectivity > > > Add external port > > > Test connectivity > > > > > > Delete internal port > > > Delete external port > > > Test connectivity > > > Add internal port > > > Test connectivity > > > Add external port > > > Test connectivity > > > > > > Uninstall extension verify port deletion. > > > > > > Alin. > > > > > > -Mesaj original- > > > De la: dev [mailto:dev-boun...@openvswitch.org] În numele Nithin Raju > > > Trimis: Thursday, June 18, 2015 11:30 PM > > > Către: Sorin Vinturis > > > Cc: dev@openvswitch.org > > > Subiect: Re: [ovs-dev] [PATCH] datapath-windows: Remove the > > > external/internal port only if it is removed on the Hyper-V switch > > > > > >> On Jun 18, 2015, at 11:52 AM, Sorin Vinturis > > >> wrote: > > >> > > >> Disconnecting the NIC from HV switch and then unloading the driver > > >> hits an ASSERT (ASSERT(switchContext->numPhysicalNics);). > > >> > > >> Also if the external or internal port is deleted from the userspace, > > >> the latter ports would not be re-added until the extension would be > > >> reloaded. > > >> > > >> This patch ensures that the internal or external ports are being > > >> deleted only by the Hyper-V switch mechanisms, which also solves the > > >> issue raised by the above assert. > > >> > > >> Signed-off-by: Sorin Vinturis > > >> Co-authored-by: Alin Gabriel Serdean > > >> Reported-by: Sorin Vinturis > > > > > > Just curious, was any unit testing performed to test the specific > > > scenario we are claiming to fix? > > > > > > thanks, > > > -- Nithin > > > ___ > > > dev mailing list > > > dev@openvswitch.org > > > https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailman_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=q1cb-T2pctLQnNJ2OwKlv_RsKbtpJ8FPjBJ-W5p_9vU&s=nWK_z-UHuGNF1B0MBF0RTfbwqrzr85wc920kbDvQWCU&e= > > > > > > > ___ > > dev mailing list > > dev@openvswitch.org > > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3] ovs-ofctl: replace-flows and diff-flows support for multiple tables
On Thu, Jun 18, 2015 at 02:31:18PM -0700, Shashank Shanbhag wrote: > From: Shashank Shanbhag > > Fix replace-flows and diff-flows to modify/diff flows in multiple tables. > Add a --tables(-T) option that allows the user to specify a comma-separated > list of table indexes to replace/diff. > Fix replace-flows to support OF1.3 and above. > Add/change ovs-ofctl replace-flows and diff-flows tests. > > Signed-off-by: Shashank Shanbhag > Acked-by: Romain Lenglet This needs a rebase due to some patch rejects. Please retain the double blank line between versions in NEWS. Does this really drop support for OF1.0 through OF1.2 in these commands? That's a regression, we can't allow that. I think the NEWS item should be shortened to just: - ovs-ofctl: replace-flows and diff-flows now operate on multiple tables. Users can refer to ovs-ofctl(8) for the rest. I'll do some detailed review when this gets rebased. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3] ovs-ofctl: replace-flows and diff-flows support for multiple tables
On Thu, Jun 18, 2015 at 02:39:07PM -0700, Shashank Shanbhag wrote: > You also suggested that we move to using an array rather than a hash. > http://openvswitch.org/pipermail/dev/2015-June/055998.html > > An array of 255 pointers to classifiers would lead to 2040 bytes (64-bit > system). I'd like to avoid that especially when there are a handful of > tables. I do not understand why you are continuing to argue with me about this. Here is the data structure I want you to use: static uint8_t tables[255]; static int n_tables; I don't care that it uses 255 or so bytes. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v3] ovs-ofctl: replace-flows and diff-flows support for multiple tables
On Fri, Jun 19, 2015 at 05:32:06PM -0700, Romain Lenglet wrote: > > On June 18, 2015 at 2:39:31 PM, Shashank Shanbhag > (shashank.shanb...@gmail.com(mailto:shashank.shanb...@gmail.com)) wrote: > > > Hi Ben, > > > > Please review when you get a chance. We would really like this to be in the > > 2.4 release. :-) > > > > This patch has the following changes suggested by you in the last review: > > 1. ovs-ofctl replace-flow and diff-flow queries the switch for visible > > tables > > > > You also suggested that we move to using an array rather than a hash. > > http://openvswitch.org/pipermail/dev/2015-June/055998.html > > > > An array of 255 pointers to classifiers would lead to 2040 bytes (64-bit > > system). I'd like to avoid that especially when there are a handful of > > tables. > > I believe you meant something like 2MB here? Seriously, you guys can't figure out how to use an array of 8-bit integers instead of a linked list of 8-bit integers? WTF? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 2/3] mcast-snooping: Use IPv6 address for MDB
Use IPv6 internally for storing multicast addresses. IPv4 addresses are
translated to their IPv4-mapped equivalent.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/mcast-snooping.c | 69 +++-
lib/mcast-snooping.h | 24 +++
ofproto/ofproto-dpif-xlate.c | 6 ++--
ofproto/ofproto-dpif.c | 5 ++--
4 files changed, 79 insertions(+), 25 deletions(-)
diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
index 7b927aa..f2684f3 100644
--- a/lib/mcast-snooping.c
+++ b/lib/mcast-snooping.c
@@ -87,10 +87,11 @@ mcast_bundle_age(const struct mcast_snooping *ms,
}
static uint32_t
-mcast_table_hash(const struct mcast_snooping *ms, ovs_be32 grp_ip4,
- uint16_t vlan)
+mcast_table_hash(const struct mcast_snooping *ms,
+ const struct in6_addr *grp_addr, uint16_t vlan)
{
-return hash_3words((OVS_FORCE uint32_t) grp_ip4, vlan, ms->secret);
+return hash_words((const uint32_t *) grp_addr->s6_addr, 4,
+ hash_2words(ms->secret, vlan));
}
static struct mcast_group_bundle *
@@ -108,8 +109,8 @@ mcast_group_from_lru_node(struct ovs_list *list)
/* Searches 'ms' for and returns an mcast group for destination address
* 'dip' in 'vlan'. */
struct mcast_group *
-mcast_snooping_lookup(const struct mcast_snooping *ms, ovs_be32 dip,
- uint16_t vlan)
+mcast_snooping_lookup(const struct mcast_snooping *ms,
+ const struct in6_addr *dip, uint16_t vlan)
OVS_REQ_RDLOCK(ms->rwlock)
{
struct mcast_group *grp;
@@ -117,13 +118,32 @@ mcast_snooping_lookup(const struct mcast_snooping *ms,
ovs_be32 dip,
hash = mcast_table_hash(ms, dip, vlan);
HMAP_FOR_EACH_WITH_HASH (grp, hmap_node, hash, &ms->table) {
-if (grp->vlan == vlan && grp->ip4 == dip) {
+if (grp->vlan == vlan && ipv6_addr_equals(&grp->addr, dip)) {
return grp;
}
}
return NULL;
}
+static inline void
+in6_addr_set_mapped_ipv4(struct in6_addr *addr, ovs_be32 ip4)
+{
+union ovs_16aligned_in6_addr *taddr = (void *) addr;
+memset(taddr->be16, 0, sizeof(taddr->be16));
+taddr->be16[5] = 0x;
+put_16aligned_be32(&taddr->be32[3], ip4);
+}
+
+struct mcast_group *
+mcast_snooping_lookup4(const struct mcast_snooping *ms, ovs_be32 ip4,
+ uint16_t vlan)
+OVS_REQ_RDLOCK(ms->rwlock)
+{
+struct in6_addr addr;
+in6_addr_set_mapped_ipv4(&addr, ip4);
+return mcast_snooping_lookup(ms, &addr, vlan);
+}
+
/* If the LRU list is not empty, stores the least-recently-used entry
* in '*e' and returns true. Otherwise, if the LRU list is empty,
* stores NULL in '*e' and return false. */
@@ -376,7 +396,8 @@ mcast_snooping_prune_expired(struct mcast_snooping *ms,
* move to the last position in the LRU list.
*/
bool
-mcast_snooping_add_group(struct mcast_snooping *ms, ovs_be32 ip4,
+mcast_snooping_add_group(struct mcast_snooping *ms,
+ const struct in6_addr *addr,
uint16_t vlan, void *port)
OVS_REQ_WRLOCK(ms->rwlock)
{
@@ -390,9 +411,9 @@ mcast_snooping_add_group(struct mcast_snooping *ms,
ovs_be32 ip4,
}
learned = false;
-grp = mcast_snooping_lookup(ms, ip4, vlan);
+grp = mcast_snooping_lookup(ms, addr, vlan);
if (!grp) {
-uint32_t hash = mcast_table_hash(ms, ip4, vlan);
+uint32_t hash = mcast_table_hash(ms, addr, vlan);
if (hmap_count(&ms->table) >= ms->max_entries) {
group_get_lru(ms, &grp);
@@ -401,7 +422,7 @@ mcast_snooping_add_group(struct mcast_snooping *ms,
ovs_be32 ip4,
grp = xmalloc(sizeof *grp);
hmap_insert(&ms->table, &grp->hmap_node, hash);
-grp->ip4 = ip4;
+memcpy(grp->addr.s6_addr, addr->s6_addr, sizeof(addr->s6_addr));
grp->vlan = vlan;
list_init(&grp->bundle_lru);
learned = true;
@@ -417,6 +438,16 @@ mcast_snooping_add_group(struct mcast_snooping *ms,
ovs_be32 ip4,
return learned;
}
+bool
+mcast_snooping_add_group4(struct mcast_snooping *ms, ovs_be32 ip4,
+ uint16_t vlan, void *port)
+OVS_REQ_WRLOCK(ms->rwlock)
+{
+struct in6_addr addr;
+in6_addr_set_mapped_ipv4(&addr, ip4);
+return mcast_snooping_add_group(ms, &addr, vlan, port);
+}
+
int
mcast_snooping_add_report(struct mcast_snooping *ms,
const struct dp_packet *p,
@@ -455,9 +486,9 @@ mcast_snooping_add_report(struct mcast_snooping *ms,
if (ntohs(record->nsrcs) == 0
&& (record->type == IGMPV3_MODE_IS_INCLUDE
|| record->type == IGMPV3_CHANGE_TO_INCLUDE_MODE)) {
-ret = mcast_snooping_leave_group(ms, ip4, vlan, port);
+ret = mcast_snooping_leave_group4(ms, ip4, vlan, port);
} else {
-ret = mcast_snooping_add_group(ms, ip4, vlan, port);
+ret = mcast_snooping_add_group4(ms,
[ovs-dev] [PATCH 1/3] ofproto/ofproto-dpif.c: fix coding style
Identation was one extra level at ofproto_unixctl_mcast_snooping_show. Signed-off-by: Thadeu Lima de Souza Cascardo --- ofproto/ofproto-dpif.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 05a80b7..04f6229 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -4442,7 +4442,7 @@ ofproto_unixctl_mcast_snooping_show(struct unixctl_conn *conn, bundle = mrouter->port; ofputil_port_to_string(ofbundle_get_a_port(bundle)->up.ofp_port, name, sizeof name); -ds_put_format(&ds, "%5s %4d querier %3d\n", +ds_put_format(&ds, "%5s %4d querier %3d\n", name, mrouter->vlan, mcast_mrouter_age(ofproto->ms, mrouter)); } -- 2.4.2 ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH 3/3] Multicast Listener Discovery support
Add support for MLDv1 and MLDv2. The behavior is not that different from
IGMP. Packets to all-hosts address and queries are always flooded,
reports go to routers, routers are added when a query is observed, and
all MLD packets go through slow path.
Signed-off-by: Thadeu Lima de Souza Cascardo
---
lib/flow.h | 25 +
lib/mcast-snooping.c | 67 ++
lib/mcast-snooping.h | 4 +++
lib/packets.c| 1 +
lib/packets.h| 40 +
ofproto/ofproto-dpif-xlate.c | 85 +---
6 files changed, 209 insertions(+), 13 deletions(-)
diff --git a/lib/flow.h b/lib/flow.h
index 70554e4..e68dd74 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -758,6 +758,31 @@ static inline bool is_icmpv6(const struct flow *flow)
&& flow->nw_proto == IPPROTO_ICMPV6);
}
+static inline bool is_igmp(const struct flow *flow)
+{
+return (flow->dl_type == htons(ETH_TYPE_IP)
+&& flow->nw_proto == IPPROTO_IGMP);
+}
+
+static inline bool is_mld(const struct flow *flow)
+{
+return is_icmpv6(flow)
+ && (flow->tp_src == htons(MLD_QUERY)
+ || flow->tp_src == htons(MLD_REPORT)
+ || flow->tp_src == htons(MLD_DONE)
+ || flow->tp_src == htons(MLD2_REPORT));
+}
+
+static inline bool is_mld_query(const struct flow *flow)
+{
+return is_icmpv6(flow) && flow->tp_src == htons(MLD_QUERY);
+}
+
+static inline bool is_mld_report(const struct flow *flow)
+{
+return is_mld(flow) && !is_mld_query(flow);
+}
+
static inline bool is_stp(const struct flow *flow)
{
return (eth_addr_equals(flow->dl_dst, eth_addr_stp)
diff --git a/lib/mcast-snooping.c b/lib/mcast-snooping.c
index f2684f3..aee80b1 100644
--- a/lib/mcast-snooping.c
+++ b/lib/mcast-snooping.c
@@ -499,6 +499,73 @@ mcast_snooping_add_report(struct mcast_snooping *ms,
return count;
}
+int
+mcast_snooping_add_mld(struct mcast_snooping *ms,
+ const struct dp_packet *p,
+ uint16_t vlan, void *port)
+{
+const struct in6_addr *addr;
+size_t offset;
+const struct mld_header *mld;
+const struct mld2_record *record;
+int count = 0;
+int ngrp;
+bool ret;
+
+offset = (char *) dp_packet_l4(p) - (char *) dp_packet_data(p);
+mld = dp_packet_at(p, offset, MLD_HEADER_LEN);
+if (!mld) {
+return 0;
+}
+ngrp = ntohs(mld->ngrp);
+offset += MLD_HEADER_LEN;
+addr = dp_packet_at(p, offset, sizeof(struct in6_addr));
+
+switch (mld->type) {
+case MLD_REPORT:
+ret = mcast_snooping_add_group(ms, addr, vlan, port);
+if (ret)
+count++;
+break;
+case MLD_DONE:
+ret = mcast_snooping_leave_group(ms, addr, vlan, port);
+if (ret)
+count++;
+break;
+case MLD2_REPORT:
+while (ngrp--) {
+record = dp_packet_at(p, offset, sizeof(struct mld2_record));
+if (!record) {
+break;
+}
+/* Only consider known record types. */
+if (record->type < IGMPV3_MODE_IS_INCLUDE
+|| record->type > IGMPV3_BLOCK_OLD_SOURCES) {
+continue;
+}
+addr = &record->maddr;
+/*
+ * If record is INCLUDE MODE and there are no sources, it's
equivalent
+ * to a LEAVE.
+ */
+if (ntohs(record->nsrcs) == 0
+&& (record->type == IGMPV3_MODE_IS_INCLUDE
+|| record->type == IGMPV3_CHANGE_TO_INCLUDE_MODE)) {
+ret = mcast_snooping_leave_group(ms, addr, vlan, port);
+} else {
+ret = mcast_snooping_add_group(ms, addr, vlan, port);
+}
+if (ret) {
+count++;
+}
+offset += sizeof(*record)
+ + ntohs(record->nsrcs) * sizeof(struct in6_addr) +
record->aux_len;
+}
+}
+
+return count;
+}
+
bool
mcast_snooping_leave_group(struct mcast_snooping *ms,
const struct in6_addr *addr,
diff --git a/lib/mcast-snooping.h b/lib/mcast-snooping.h
index e3d15e4..99c314d 100644
--- a/lib/mcast-snooping.h
+++ b/lib/mcast-snooping.h
@@ -194,6 +194,10 @@ int mcast_snooping_add_report(struct mcast_snooping *ms,
const struct dp_packet *p,
uint16_t vlan, void *port)
OVS_REQ_WRLOCK(ms->rwlock);
+int mcast_snooping_add_mld(struct mcast_snooping *ms,
+ const struct dp_packet *p,
+ uint16_t vlan, void *port)
+OVS_REQ_WRLOCK(ms->rwlock);
bool mcast_snooping_leave_group(struct mcast_snooping *ms,
const struct in6_addr *addr,
uint16_t vlan, void *port)
diff --git a/lib/packets.c b/lib/
Re: [ovs-dev] [PATCH v3] ovs-ofctl: replace-flows and diff-flows support for multiple tables
Ben, Thanks for taking a look. I’ll move to using an array, and will send out the patch as soon as possible. Shashank On Jun 23, 2015, at 12:58 PM, Ben Pfaff wrote: > On Thu, Jun 18, 2015 at 02:39:07PM -0700, Shashank Shanbhag wrote: >> You also suggested that we move to using an array rather than a hash. >> http://openvswitch.org/pipermail/dev/2015-June/055998.html >> >> An array of 255 pointers to classifiers would lead to 2040 bytes (64-bit >> system). I'd like to avoid that especially when there are a handful of >> tables. > > I do not understand why you are continuing to argue with me about this. > Here is the data structure I want you to use: > >static uint8_t tables[255]; >static int n_tables; > > I don't care that it uses 255 or so bytes. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH v2] datapath-windows: Remove the external/internal port only if it is removed on the Hyper-V switch
> On Jun 23, 2015, at 12:42 PM, Ben Pfaff wrote: > > On Thu, Jun 18, 2015 at 09:05:32PM +, Nithin Raju wrote: >>> On Jun 18, 2015, at 1:57 PM, Alin Serdean >>> wrote: >>> >>> I have no issue with it if it is tested. >>> >>> Yes Nithin you are right we agreed that you will work on it but it was in >>> the backlog for some time and it is a show stopper because it also >>> fixes(https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openvswitch_ovs-2Dissues_issues_62&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=pNHQcdr7B40b4h6Yb7FIedI1dnBsxdDuTLBYD3JqV80&m=mGRgSLlhBsu9jwYpHagt2xLxS1GSklQgTGXxMSuEL-g&s=YIhRLJ49Q4cetS_HYw_jccyA5VQFWGMPvnX6bq7yGKY&e= >>> ). >> >> I’ll send out my fix today. I ran into a crash while testing >> creation/deletion of vxlan port. Debugging that right now. >> >> Thanks for your understanding. > > Does this mean that you're working on a different fix? Or is this patch > one that should be applied? hi Ben, Alin, Sorin and I agreed that we’ll use my patch that I send out (later) here: http://openvswitch.org/pipermail/dev/2015-June/056579.html Alin found some issues with the patch while validating it, and I am working with him offline to get the patch validated. Once I get an ACK from him offline, I’ll post a new revision of the patch. As of now, we don’t have any “datapath-windows” patches pending to be committed. thanks, -- Nithin ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OVN and OpenStack Provider Networks
On Mon, Jun 22, 2015 at 02:34:07PM -0400, Russell Bryant wrote: > On 06/15/2015 08:00 PM, Ben Pfaff wrote: > > On Wed, Jun 10, 2015 at 03:13:54PM -0400, Russell Bryant wrote: > >> Provider Networks > >> = > >> > >> OpenStack Neutron currently has a feature referred to as "provider > >> networks". This is used as a way to define existing physical networks > >> that you would like to integrate into your environment. > >> > >> In the simplest case, it can be used in environments where they have no > >> interest in tenant networks. Instead, they want all VMs hooked up > >> directly to a pre-defined network in their environment. This use case > >> is actually popular for private OpenStack deployments. > >> > >> Neutron's current OVS agent that runs on network nodes and hypervisors > >> has this configuration entry: > >> > >> bridge_mappings = physnet1:br-eth1,physnet2:br-eth2[...] > >> > >> This is used to name your physical networks and the bridge used to > >> access that physical network from the local node. > >> > >> Defining a provider network via the Neutron API via the neutron > >> command looks like this: > >> > >> $ neutron net-create physnet1 --shared \ > >> > --provider:physical_network external \ > >> > --provider:network_type flat > >> > >> A provider network can also be defined with a VLAN id: > >> > >> $ neutron net-create physnet1-101 --shared \ > >> > --provider:physical_network external \ > >> > --provider:network_type vlan \ > >> > --provider:segmentation_id 101 > > > > I'm trying to understand what degree of sophistication these provider > > networks have. Are they just an interface to a MAC-learning switch > > (possibly VLAN-tagged)? Or do provider networks go beyond that, with > > the features that one would expect from an OVN logical network > > (e.g. port security, ACLs, distributed routing and firewalling, ...)? > > (Kyle and Salvatore, please sanity check me on this.) > > AFAIK, it is simply an interface to a MAC-learning switch, possibly VLAN > tagged. > > It is not expected that a provider network would provide port security > or ACLs (security groups). Those would still be the responsibility of > OVN in this case. > > A provider network *may* (but usually does) handle routing and SNAT/DNAT > if necessary. In that case it is managed externally to Neutron. The > only knowledge Neutron has is about the address space on the provider > network, since Neutron provides IPAM. Continuing with the example > above, we can define a subnet on that provider network with: > > $ neutron subnet-create provider-101 203.0.113.0/24 \ > > --enable-dhcp --gateway 203.0.113.1 > > Neutron would do address assignment and provide the DHCP server for this > network. 203.0.113.1 would be the router. > > Neutron (and thus, OVN) would provide port-level firewalls (Neutron > security groups) using OVN ACLs. Additional firewalls (such as at the > router) may exist, but Neutron doesn't need to know about it as it's > expected to be managed externally. I had to read this several times, but maybe I understand it now. Let me recap for verification. A "tenant network" is what OVN calls a logical network. OVN can construct it as an L2-over-L3 overlay with STT or Geneve or whatever. Tenant networks can be connected to physical networks via OVN gateways. A "provider network" is just a physical L2 network (possibly VLAN-tagged). In such a network, on the sending side, OVN would rely on normal L2 switching for packets to reach their destinations, and on the receiving side, OVN would not have a reliable way to determine the source of a packet (it would have to infer it from the source MAC). Is that accurate? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OVN and OpenStack Provider Networks
On 06/23/2015 04:17 PM, Ben Pfaff wrote: > On Mon, Jun 22, 2015 at 02:34:07PM -0400, Russell Bryant wrote: >> On 06/15/2015 08:00 PM, Ben Pfaff wrote: >>> On Wed, Jun 10, 2015 at 03:13:54PM -0400, Russell Bryant wrote: Provider Networks = OpenStack Neutron currently has a feature referred to as "provider networks". This is used as a way to define existing physical networks that you would like to integrate into your environment. In the simplest case, it can be used in environments where they have no interest in tenant networks. Instead, they want all VMs hooked up directly to a pre-defined network in their environment. This use case is actually popular for private OpenStack deployments. Neutron's current OVS agent that runs on network nodes and hypervisors has this configuration entry: bridge_mappings = physnet1:br-eth1,physnet2:br-eth2[...] This is used to name your physical networks and the bridge used to access that physical network from the local node. Defining a provider network via the Neutron API via the neutron command looks like this: $ neutron net-create physnet1 --shared \ > --provider:physical_network external \ > --provider:network_type flat A provider network can also be defined with a VLAN id: $ neutron net-create physnet1-101 --shared \ > --provider:physical_network external \ > --provider:network_type vlan \ > --provider:segmentation_id 101 >>> >>> I'm trying to understand what degree of sophistication these provider >>> networks have. Are they just an interface to a MAC-learning switch >>> (possibly VLAN-tagged)? Or do provider networks go beyond that, with >>> the features that one would expect from an OVN logical network >>> (e.g. port security, ACLs, distributed routing and firewalling, ...)? >> >> (Kyle and Salvatore, please sanity check me on this.) >> >> AFAIK, it is simply an interface to a MAC-learning switch, possibly VLAN >> tagged. >> >> It is not expected that a provider network would provide port security >> or ACLs (security groups). Those would still be the responsibility of >> OVN in this case. >> >> A provider network *may* (but usually does) handle routing and SNAT/DNAT >> if necessary. In that case it is managed externally to Neutron. The >> only knowledge Neutron has is about the address space on the provider >> network, since Neutron provides IPAM. Continuing with the example >> above, we can define a subnet on that provider network with: >> >> $ neutron subnet-create provider-101 203.0.113.0/24 \ >> > --enable-dhcp --gateway 203.0.113.1 >> >> Neutron would do address assignment and provide the DHCP server for this >> network. 203.0.113.1 would be the router. >> >> Neutron (and thus, OVN) would provide port-level firewalls (Neutron >> security groups) using OVN ACLs. Additional firewalls (such as at the >> router) may exist, but Neutron doesn't need to know about it as it's >> expected to be managed externally. > > I had to read this several times, but maybe I understand it now. Let me > recap for verification. > > A "tenant network" is what OVN calls a logical network. OVN can > construct it as an L2-over-L3 overlay with STT or Geneve or whatever. > Tenant networks can be connected to physical networks via OVN gateways. > > A "provider network" is just a physical L2 network (possibly > VLAN-tagged). In such a network, on the sending side, OVN would rely on > normal L2 switching for packets to reach their destinations, and on the > receiving side, OVN would not have a reliable way to determine the > source of a packet (it would have to infer it from the source MAC). Is > that accurate? > Yes, all of that matches my understanding of things. I worry that not being able to explain it well might mean I don't have it all right, so I hope some other Neutron devs chime in to confirm, as well. -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OVN and OpenStack Provider Networks
On Tue, Jun 23, 2015 at 04:54:20PM -0400, Russell Bryant wrote: > On 06/23/2015 04:17 PM, Ben Pfaff wrote: > > On Mon, Jun 22, 2015 at 02:34:07PM -0400, Russell Bryant wrote: > >> On 06/15/2015 08:00 PM, Ben Pfaff wrote: > >>> On Wed, Jun 10, 2015 at 03:13:54PM -0400, Russell Bryant wrote: > Provider Networks > = > > OpenStack Neutron currently has a feature referred to as "provider > networks". This is used as a way to define existing physical networks > that you would like to integrate into your environment. > > In the simplest case, it can be used in environments where they have no > interest in tenant networks. Instead, they want all VMs hooked up > directly to a pre-defined network in their environment. This use case > is actually popular for private OpenStack deployments. [...] > > I had to read this several times, but maybe I understand it now. Let me > > recap for verification. > > > > A "tenant network" is what OVN calls a logical network. OVN can > > construct it as an L2-over-L3 overlay with STT or Geneve or whatever. > > Tenant networks can be connected to physical networks via OVN gateways. > > > > A "provider network" is just a physical L2 network (possibly > > VLAN-tagged). In such a network, on the sending side, OVN would rely on > > normal L2 switching for packets to reach their destinations, and on the > > receiving side, OVN would not have a reliable way to determine the > > source of a packet (it would have to infer it from the source MAC). Is > > that accurate? > > Yes, all of that matches my understanding of things. > > I worry that not being able to explain it well might mean I don't have > it all right, so I hope some other Neutron devs chime in to confirm, as > well. OK, let's go on then. Some more recap, on the reason why this would need to be in OVN. If I'm following, that's because users are likely to want to have VMs that connect both to provider networks and to tenant networks on the same hypervisor, and that means that they need Neutron plugins for each of those, and there's naturally a reluctance to install the bits for two different plugins on every hypervisor. Is that correct? If it is, then I'll go back and reread the ideas we had elsewhere in this thread; I'm better equipped to understand them now. Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OVN and OpenStack Provider Networks
On 06/23/2015 05:10 PM, Ben Pfaff wrote: > On Tue, Jun 23, 2015 at 04:54:20PM -0400, Russell Bryant wrote: >> On 06/23/2015 04:17 PM, Ben Pfaff wrote: >>> On Mon, Jun 22, 2015 at 02:34:07PM -0400, Russell Bryant wrote: On 06/15/2015 08:00 PM, Ben Pfaff wrote: > On Wed, Jun 10, 2015 at 03:13:54PM -0400, Russell Bryant wrote: >> Provider Networks >> = >> >> OpenStack Neutron currently has a feature referred to as "provider >> networks". This is used as a way to define existing physical networks >> that you would like to integrate into your environment. >> >> In the simplest case, it can be used in environments where they have no >> interest in tenant networks. Instead, they want all VMs hooked up >> directly to a pre-defined network in their environment. This use case >> is actually popular for private OpenStack deployments. > > [...] > >>> I had to read this several times, but maybe I understand it now. Let me >>> recap for verification. >>> >>> A "tenant network" is what OVN calls a logical network. OVN can >>> construct it as an L2-over-L3 overlay with STT or Geneve or whatever. >>> Tenant networks can be connected to physical networks via OVN gateways. >>> >>> A "provider network" is just a physical L2 network (possibly >>> VLAN-tagged). In such a network, on the sending side, OVN would rely on >>> normal L2 switching for packets to reach their destinations, and on the >>> receiving side, OVN would not have a reliable way to determine the >>> source of a packet (it would have to infer it from the source MAC). Is >>> that accurate? >> >> Yes, all of that matches my understanding of things. >> >> I worry that not being able to explain it well might mean I don't have >> it all right, so I hope some other Neutron devs chime in to confirm, as >> well. > > OK, let's go on then. > > Some more recap, on the reason why this would need to be in OVN. If I'm > following, that's because users are likely to want to have VMs that > connect both to provider networks and to tenant networks on the same > hypervisor, and that means that they need Neutron plugins for each of > those, and there's naturally a reluctance to install the bits for two > different plugins on every hypervisor. Is that correct? If it is, then > I'll go back and reread the ideas we had elsewhere in this thread; I'm > better equipped to understand them now. That is correct, yes. -- Russell Bryant ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] dpif-netdev: Check for PKT_RX_RSS_HASH flag.
On Mon, Jun 22, 2015 at 8:08 PM, Pravin Shelar wrote: > On Fri, Jun 19, 2015 at 11:24 AM, Daniele Di Proietto > wrote: >> >> >> On 18/06/2015 23:57, "Traynor, Kevin" wrote: >> >>> >>> -Original Message- >>> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Daniele Di >>> Proietto >>> Sent: Tuesday, June 16, 2015 7:39 PM >>> To: dev@openvswitch.org >>> Subject: [ovs-dev] [PATCH] dpif-netdev: Check for PKT_RX_RSS_HASH flag. >>> >>> DPDK mbufs contain a valid RSS hash only if PKT_RX_RSS_HASH is >>> set in 'ol_flags'. Otherwise the hash is garbage and doesn't >>> relate to the packet. >>> >>> This fixes an issue with vhost, which, being a virtual NIC, doesn't >>> compute the hash. >>> >>> Unfortunately the ixgbe vPMD doesn't set the PKT_RX_RSS_HASH, forcing >>> OVS to compute an hash is software. This has a significant impact on >>> performance (-30% throughput in a single flow setup) which can be >>> mitigated in the CPU supports crc32c instructions. >>> >>> >>> >>>As per the other thread on this I'm a bit concerned about the performance >>> >>>drop from this patch, so I did some testing of this and alternative/ >>> >>>complimentary solutions. >>> >>> >>> >>>Here's the options I looked at and some comments: >>> >>>1. This patch in isolation: vhost drops about ~15% vhost-vhost and >>> >>>phy-vhost-phy (because of sw hash) but also there is drops of ~25% for >>> >>>phy-phy and ~15% drop for phy-ivshmem-phy. >>> >>> >>> >>>2. Leave the code as is and let EMC misses happen for vhost rx pkts: >>> >>>I measure this at ~35% drop if missed *everytime* for vhost-vhost. We >>> >>>see in testing that it can also never happen, but this is not realistic. >>> >>>There should be no impact to other DPDK interfaces. >>> >>> >>> >>>3. Add hash reset for packets from vhost: This is another way of forcing >>> >>>the software hash for vhost rx and it is roughly equivalent in performance >>> >>>to 1. for vhost-vhost (~15% drop). While there is a no significant drop >>> >>>for phy-vhost-phy. There should be no impact to other DPDK interfaces. >>> >>> >>> >>>4. Apply this patch and turn off Rx Vectorisation. vhost-vhost will drop >>> >>>~15% as per 1. and there should be nothing significant for phy-vhost-phy. >>> >>>We would lose the 10% gain that rx vectorisation gave us for phy-phy. >>> >>>There should be no impact for dpdkr ports. >>> >>> >>> >>> >>> >>>In terms of not knowing whether the hw hash is valid or not if the flag is >>> >>>not checked, I would have expected the pmd to return an error on config if >>> >>>the hash wasn't supported, but I'm not sure that it does. >>> >>>In the worst case where there was an incorrect hash, it would miss the EMC >>> >>>which is about a 45% drop for phy-phy. I would think it's pretty safe that >>> >>>if we configure it, the hash will be correct but I guess there is a >>> >>>possibility it wouldn't be. >>> >>> >>> >>>Even if it is possible to get a smaller patch to fix the underlying issue >>> >>>in DPDK, it would be in DPDK 2.1 at the earliest meaning the performance >>> >>>would remain low until sometime in August. If it's DPDK 2.2, then it would >>> >>>be sometime in December. This would mean any performance drops would be >>> >>>present in OVS 2.4 and possibly OVS 2.5. >>> >>> >>> >>>Sorry :( but based on the performance drop with this patch in isolation it >>> >>>would be a NAK from me. My preference would be 3 which gives best >>>performance, >>> >>>or 4 which is a bit lower for phy-phy but safer. >>> >>> >>> >>>Kevin. >> >> Thanks for all the testing. I guess it might make sense to stretch our >> interpretation of the API in this case, because it wouldn't affect >> correctness. >> >> Unless there any other objection I'm fine with the 3rd approach. >> > > We can use 3rd approach to fix issue on branch 2.4. Then have patch to > check the PKT_RX_RSS_HASH flag on master. By the time we release > branch 2.5 we will have proper fix in DPDK and performance will bounce > back. I think this is probably a reasonable compromise. I think it's better to not keep a workaround in for an unbounded amount of time, otherwise we'll forget about it and it will come back to bite us in the future. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] OVN and OpenStack Provider Networks
I'm afraid I have to start bike shedding on this thread too. Apparently that's what I do best. More inline, Salvatore On 23 June 2015 at 23:23, Russell Bryant wrote: > On 06/23/2015 05:10 PM, Ben Pfaff wrote: > > On Tue, Jun 23, 2015 at 04:54:20PM -0400, Russell Bryant wrote: > >> On 06/23/2015 04:17 PM, Ben Pfaff wrote: > >>> On Mon, Jun 22, 2015 at 02:34:07PM -0400, Russell Bryant wrote: > On 06/15/2015 08:00 PM, Ben Pfaff wrote: > > On Wed, Jun 10, 2015 at 03:13:54PM -0400, Russell Bryant wrote: > >> Provider Networks > >> = > >> > >> OpenStack Neutron currently has a feature referred to as "provider > >> networks". This is used as a way to define existing physical > networks > >> that you would like to integrate into your environment. > >> > >> In the simplest case, it can be used in environments where they > have no > >> interest in tenant networks. Instead, they want all VMs hooked up > >> directly to a pre-defined network in their environment. This use > case > >> is actually popular for private OpenStack deployments. > > > > [...] > > > >>> I had to read this several times, but maybe I understand it now. Let > me > >>> recap for verification. > >>> > >>> A "tenant network" is what OVN calls a logical network. OVN can > >>> construct it as an L2-over-L3 overlay with STT or Geneve or whatever. > >>> Tenant networks can be connected to physical networks via OVN gateways. > >>> > >>> A "provider network" is just a physical L2 network (possibly > >>> VLAN-tagged). In such a network, on the sending side, OVN would rely > on > >>> normal L2 switching for packets to reach their destinations, and on the > >>> receiving side, OVN would not have a reliable way to determine the > >>> source of a packet (it would have to infer it from the source MAC). Is > >>> that accurate? > >> > While this is correct, it is also restrictive - as that would imply that a "provider network" is just a physical L2 segment on the data centre. Therefore logical ports on a provider networks would be pretty much pass through to the physical network. While it is correct that they might be mapped to OVS ports on a bridge doings plain L2 forwarding onto a physical network, this does not mean that L2 forwarding is the only thing that one can do on provider networks. A provider network is, from the neutron perspective, exactly like any other logical network, including tenant networks. What changes are bindings (or mappings, I don't know what's the correct OVN terminology). These bindings define three aspects: 1 - the transport type (VLAN, GRE, STT, VxLAN, etc) 2 - the physical network, if any 3 - the segmentation id on the physical network, if any, For tenant networks, bindings are implicit and depend on what the control plan defaults to. As Ben was suggesting this could STT or Geneve. For provider networks, these bindings are explicit, as the admin defines them. For instance I want this network to be mapped to VLAN 666 on physical network MEH. In practical terms with provider networks the control plane must honour the specification made in the neutron request concerning transport bindings for the logical networks. If it can't honour these mapping - for instance if it does not support the select transport type - it must return an error. Nevertheless the control plane still treats provider networks like any other network. You can have services like DHCP on them (even if often is not a great idea), apply security groups to its ports, uplink them to logical routers, and so on. > >> Yes, all of that matches my understanding of things. > >> > >> I worry that not being able to explain it well might mean I don't have > >> it all right, so I hope some other Neutron devs chime in to confirm, as > >> well. > > > > OK, let's go on then. > > > > Some more recap, on the reason why this would need to be in OVN. If I'm > > following, that's because users are likely to want to have VMs that > > connect both to provider networks and to tenant networks on the same > > hypervisor, and that means that they need Neutron plugins for each of > > those, and there's naturally a reluctance to install the bits for two > > different plugins on every hypervisor. Is that correct? If it is, then > > I'll go back and reread the ideas we had elsewhere in this thread; I'm > > better equipped to understand them now. > I believe people would love the idea of being able to deploy multiple plugins on the same neutron deployments and handle some kind of networks with one plugin and other kind of networks with the other plugin. Unfortunately Neutron cannot yet quite do that, unless we add some machinery into the ML2 plugin. One reason I see for having them in OVN is that these providers networks are not isolated from the rest of the logical network topology. You should still be able to apply security groups to them or uplink them a logical router, as per my previous comment. This is not
Re: [ovs-dev] OVN and OpenStack Provider Networks
On Tue, Jun 23, 2015 at 11:58:25PM +0200, Salvatore Orlando wrote: > I'm afraid I have to start bike shedding on this thread too. > Apparently that's what I do best. These are important clarifications, not bikeshedding. More below (with lots of trimming and reflowing for clarity): > On 06/23/2015 05:10 PM, Ben Pfaff wrote: > > A "tenant network" is what OVN calls a logical network. OVN can > > construct it as an L2-over-L3 overlay with STT or Geneve or > > whatever. Tenant networks can be connected to physical networks via > > OVN gateways. > > > > A "provider network" is just a physical L2 network (possibly > > VLAN-tagged). In such a network, on the sending side, OVN would > > rely on normal L2 switching for packets to reach their destinations, > > and on the receiving side, OVN would not have a reliable way to > > determine the source of a packet (it would have to infer it from the > > source MAC). Is that accurate? > > While this is correct, it is also restrictive - as that would imply that a > "provider network" is just a physical L2 segment on the data centre. > Therefore logical ports on a provider networks would be pretty much pass > through to the physical network. While it is correct that they might be > mapped to OVS ports on a bridge doings plain L2 forwarding onto a physical > network, this does not mean that L2 forwarding is the only thing that one > can do on provider networks. > > A provider network is, from the neutron perspective, exactly like any other > logical network, including tenant networks. What changes are bindings (or > mappings, I don't know what's the correct OVN terminology). These bindings > define three aspects: > 1 - the transport type (VLAN, GRE, STT, VxLAN, etc) > 2 - the physical network, if any > 3 - the segmentation id on the physical network, if any, > > For tenant networks, bindings are implicit and depend on what the control > plan defaults to. As Ben was suggesting this could STT or Geneve. > For provider networks, these bindings are explicit, as the admin defines > them. For instance I want this network to be mapped to VLAN 666 on physical > network MEH. > > In practical terms with provider networks the control plane must honour the > specification made in the neutron request concerning transport bindings for > the logical networks. If it can't honour these mapping - for instance if it > does not support the select transport type - it must return an error. > Nevertheless the control plane still treats provider networks like any > other network. You can have services like DHCP on them (even if often is > not a great idea), apply security groups to its ports, uplink them to > logical routers, and so on. OK, let me take another stab at a recap, then. For a tenant network, it is outside the scope of Neutron to dictate or configure how packets are transported among VMs. Instead, that is delegated to the plugin itself or to whatever the plugin configures (e.g. in this case, to OVN). For a provider network, the administrator (via Neutron) configures use of a specific transport in a specific way. A Neutron plugin must operate over that configured transport in that way, or if cannot then it must refuse to operate at all. OK, if that all that is correct, does the following logical extension also hold? A provider network implementation is expected to transparently interoperate with preexisting software that shares a given transport. For example, if I set up a provider network with Neutron on a particular Ethernet network, and I have a bunch of physical machines attached to the same Ethernet network, then I would expect my Neutron VMs attached to the physical network to be able to communicate back and forth with those physical machines. If that is the case, then I guess one description of the two different types of network is this: a Neutron plugin may *define* a tenant network, but a Neutron plugin only *participates* in a provider network. Is that fair? (I apologize if all this should be obvious to Neutron veterans. I'm new to this!) Thanks, Ben. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] INSTALL.DPDK: remove experimental statement
On Tue, Jun 23, 2015 at 11:42:15AM -0700, Ben Pfaff wrote: > Do you two have an opinion on this? If DPDK support is pretty solid now > then it makes sense to apply this to master and backport it to > branch-2.4. Daniele, what is your opinion? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 3/3] Update windows test documentation
How about the following: Before running the unit tests make sure to add the pthread libraries to your PATH environment variable. One of the unit tests starts Open vSwitch as a system wide service. This test expects the pthread libraries path to be in the Windows' SYSTEM PATH environment variable. If while running unit tests, you do not have the permissions to start a system wide service or if you already have a Open vSwitch service running, you can disable the unit test with: make check TESTSUITEFLAGS="-k \!windows-service –j8” Alin. -Mesaj original- De la: Gurucharan Shetty [mailto:shet...@nicira.com] Trimis: Tuesday, June 23, 2015 5:51 PM Către: Alin Serdean Cc: dev@openvswitch.org Subiect: Re: [ovs-dev] [PATCH 3/3] Update windows test documentation On Mon, Jun 22, 2015 at 3:45 PM, Alin Serdean wrote: > Describe explictly where to add the pthread library to the PATH variable. > > In case the pthread library directory was added to the user PATH > variable the service failed to start. > > Signed-off-by: Alin Gabriel Serdean > --- > INSTALL.Windows.md | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/INSTALL.Windows.md b/INSTALL.Windows.md index > 6d870ed..52acf6e 100644 > --- a/INSTALL.Windows.md > +++ b/INSTALL.Windows.md > @@ -92,6 +92,9 @@ or from a distribution tar ball. > > % make check TESTSUITEFLAGS="-j8" > > + For "daemon --service" test make sure to have the pthread libaries > + in the the Windows' SYSTEM PATH environment variable > + How about the following wording instead? (One of the unit tests starts Open vSwitch as a system wide service. This test expects the pthread libraries path to be in the Windows' SYSTEM PATH environment variable. If while running unit tests, you do not have the permissions to start a system wide service or if you already have a Open vSwitch service running, you can disable the unit test with: make check TESTSUITEFLAGS="-k \!windows-service –j8”) If you think above is correct, I will update your patch before committing. > * To install all the compiled executables on the local machine, run: > > % make install > -- > 1.9.5.msysgit.0 > ___ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH 1/3] Add OVS_VSWITCHD_STOP to bfd unit tests
Think I found why tskill is missing on my system. It is part of the role remote desktop services role https://technet.microsoft.com/en-us/library/cc725766.aspx while taskkill is part of the default installation. Theoretically taskkill with option /f "Specifies that processes be forcefully terminated. This parameter is ignored for remote processes; all remote processes are forcefully terminated." and /t " Terminates the specified process and any child processes started by it." would suffice. Alin. -Mesaj original- De la: Gurucharan Shetty [mailto:shet...@nicira.com] Trimis: Tuesday, June 23, 2015 5:40 PM Către: Alin Serdean Cc: dev@openvswitch.org Subiect: Re: [ovs-dev] [PATCH 1/3] Add OVS_VSWITCHD_STOP to bfd unit tests I applied this patch as-is. We still need to figure out the issue around tskill. On Tue, Jun 23, 2015 at 7:05 AM, Gurucharan Shetty wrote: > On Mon, Jun 22, 2015 at 5:21 PM, Alin Serdean > wrote: >> I could send another patch with the following: >> -tskill $i >> +taskkill //PID $i //F >/dev/null >> >> Alin. > > Your current patch is something that I think we should apply because > from Windows' perspective it does a clean kill instead of the force > kill. > > Do you know why your system does not have tskill? The code originally > had taskkill instead of tskill. One thing I observed was that taskkill > cannot kill deadlocked processes and unit tests would hang whenever > processes deadlock. That is the reason I changed it to tskill. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] INSTALL.DPDK: remove experimental statement
On Tue, Jun 23, 2015 at 11:42 AM, Ben Pfaff wrote: > Do you two have an opinion on this? If DPDK support is pretty solid now > then it makes sense to apply this to master and backport it to > branch-2.4. Personally I would like to have better testing with vhost, tunneling and PMD thread management before removing experimental status. Once that is done I will be more comfortable with it. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] dpif-netdev: Check for PKT_RX_RSS_HASH flag.
On Tue, Jun 23, 2015 at 2:51 PM, Jesse Gross wrote: > On Mon, Jun 22, 2015 at 8:08 PM, Pravin Shelar wrote: >> On Fri, Jun 19, 2015 at 11:24 AM, Daniele Di Proietto >> wrote: >>> >>> >>> On 18/06/2015 23:57, "Traynor, Kevin" wrote: >>> > -Original Message- > From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Daniele Di > Proietto > Sent: Tuesday, June 16, 2015 7:39 PM > To: dev@openvswitch.org > Subject: [ovs-dev] [PATCH] dpif-netdev: Check for PKT_RX_RSS_HASH flag. > > DPDK mbufs contain a valid RSS hash only if PKT_RX_RSS_HASH is > set in 'ol_flags'. Otherwise the hash is garbage and doesn't > relate to the packet. > > This fixes an issue with vhost, which, being a virtual NIC, doesn't > compute the hash. > > Unfortunately the ixgbe vPMD doesn't set the PKT_RX_RSS_HASH, forcing > OVS to compute an hash is software. This has a significant impact on > performance (-30% throughput in a single flow setup) which can be > mitigated in the CPU supports crc32c instructions. As per the other thread on this I'm a bit concerned about the performance drop from this patch, so I did some testing of this and alternative/ complimentary solutions. Here's the options I looked at and some comments: 1. This patch in isolation: vhost drops about ~15% vhost-vhost and phy-vhost-phy (because of sw hash) but also there is drops of ~25% for phy-phy and ~15% drop for phy-ivshmem-phy. 2. Leave the code as is and let EMC misses happen for vhost rx pkts: I measure this at ~35% drop if missed *everytime* for vhost-vhost. We see in testing that it can also never happen, but this is not realistic. There should be no impact to other DPDK interfaces. 3. Add hash reset for packets from vhost: This is another way of forcing the software hash for vhost rx and it is roughly equivalent in performance to 1. for vhost-vhost (~15% drop). While there is a no significant drop for phy-vhost-phy. There should be no impact to other DPDK interfaces. 4. Apply this patch and turn off Rx Vectorisation. vhost-vhost will drop ~15% as per 1. and there should be nothing significant for phy-vhost-phy. We would lose the 10% gain that rx vectorisation gave us for phy-phy. There should be no impact for dpdkr ports. In terms of not knowing whether the hw hash is valid or not if the flag is not checked, I would have expected the pmd to return an error on config if the hash wasn't supported, but I'm not sure that it does. In the worst case where there was an incorrect hash, it would miss the EMC which is about a 45% drop for phy-phy. I would think it's pretty safe that if we configure it, the hash will be correct but I guess there is a possibility it wouldn't be. Even if it is possible to get a smaller patch to fix the underlying issue in DPDK, it would be in DPDK 2.1 at the earliest meaning the performance would remain low until sometime in August. If it's DPDK 2.2, then it would be sometime in December. This would mean any performance drops would be present in OVS 2.4 and possibly OVS 2.5. Sorry :( but based on the performance drop with this patch in isolation it would be a NAK from me. My preference would be 3 which gives best performance, or 4 which is a bit lower for phy-phy but safer. Kevin. >>> >>> Thanks for all the testing. I guess it might make sense to stretch our >>> interpretation of the API in this case, because it wouldn't affect >>> correctness. >>> >>> Unless there any other objection I'm fine with the 3rd approach. >>> >> >> We can use 3rd approach to fix issue on branch 2.4. Then have patch to >> check the PKT_RX_RSS_HASH flag on master. By the time we release >> branch 2.5 we will have proper fix in DPDK and performance will bounce >> back. > > I think this is probably a reasonable compromise. I think it's better > to not keep a workaround in for an unbounded amount of time, otherwise > we'll forget about it and it will come back to bite us in the future. ok, Once the DPDK fix is backported to DPDK 2.0, we can remove the workaround. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] dpif-netdev: Check for PKT_RX_RSS_HASH flag.
On Tue, Jun 23, 2015 at 7:06 PM, Pravin Shelar wrote: > On Tue, Jun 23, 2015 at 2:51 PM, Jesse Gross wrote: >> On Mon, Jun 22, 2015 at 8:08 PM, Pravin Shelar wrote: >>> On Fri, Jun 19, 2015 at 11:24 AM, Daniele Di Proietto >>> wrote: On 18/06/2015 23:57, "Traynor, Kevin" wrote: > > >> -Original Message- > >> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Daniele Di > >> Proietto > >> Sent: Tuesday, June 16, 2015 7:39 PM > >> To: dev@openvswitch.org > >> Subject: [ovs-dev] [PATCH] dpif-netdev: Check for PKT_RX_RSS_HASH flag. > >> > >> DPDK mbufs contain a valid RSS hash only if PKT_RX_RSS_HASH is > >> set in 'ol_flags'. Otherwise the hash is garbage and doesn't > >> relate to the packet. > >> > >> This fixes an issue with vhost, which, being a virtual NIC, doesn't > >> compute the hash. > >> > >> Unfortunately the ixgbe vPMD doesn't set the PKT_RX_RSS_HASH, forcing > >> OVS to compute an hash is software. This has a significant impact on > >> performance (-30% throughput in a single flow setup) which can be > >> mitigated in the CPU supports crc32c instructions. > > > >As per the other thread on this I'm a bit concerned about the performance > >drop from this patch, so I did some testing of this and alternative/ > >complimentary solutions. > > > >Here's the options I looked at and some comments: > >1. This patch in isolation: vhost drops about ~15% vhost-vhost and > >phy-vhost-phy (because of sw hash) but also there is drops of ~25% for > >phy-phy and ~15% drop for phy-ivshmem-phy. > > > >2. Leave the code as is and let EMC misses happen for vhost rx pkts: > >I measure this at ~35% drop if missed *everytime* for vhost-vhost. We > >see in testing that it can also never happen, but this is not realistic. > >There should be no impact to other DPDK interfaces. > > > >3. Add hash reset for packets from vhost: This is another way of forcing > >the software hash for vhost rx and it is roughly equivalent in performance > >to 1. for vhost-vhost (~15% drop). While there is a no significant drop > >for phy-vhost-phy. There should be no impact to other DPDK interfaces. > > > >4. Apply this patch and turn off Rx Vectorisation. vhost-vhost will drop > >~15% as per 1. and there should be nothing significant for phy-vhost-phy. > >We would lose the 10% gain that rx vectorisation gave us for phy-phy. > >There should be no impact for dpdkr ports. > > > > > >In terms of not knowing whether the hw hash is valid or not if the flag is > >not checked, I would have expected the pmd to return an error on config if > >the hash wasn't supported, but I'm not sure that it does. > >In the worst case where there was an incorrect hash, it would miss the EMC > >which is about a 45% drop for phy-phy. I would think it's pretty safe that > >if we configure it, the hash will be correct but I guess there is a > >possibility it wouldn't be. > > > >Even if it is possible to get a smaller patch to fix the underlying issue > >in DPDK, it would be in DPDK 2.1 at the earliest meaning the performance > >would remain low until sometime in August. If it's DPDK 2.2, then it would > >be sometime in December. This would mean any performance drops would be > >present in OVS 2.4 and possibly OVS 2.5. > > > >Sorry :( but based on the performance drop with this patch in isolation it > >would be a NAK from me. My preference would be 3 which gives best >performance, > >or 4 which is a bit lower for phy-phy but safer. > > > >Kevin. Thanks for all the testing. I guess it might make sense to stretch our interpretation of the API in this case, because it wouldn't affect correctness. Unless there any other objection I'm fine with the 3rd approach. >>> >>> We can use 3rd approach to fix issue on branch 2.4. Then have patch to >>> check the PKT_RX_RSS_HASH flag on master. By the time we release >>> branch 2.5 we will have proper fix in DPDK and performance will bounce >>> back. >> >> I think this is probably a reasonable compromise. I think it's better >> to not keep a workaround in for an unbounded amount of time, otherwise >> we'll forget about it and it will come back to bite us in the future. > > ok, Once the DPDK fix is backported to DPDK 2.0, we can remove the workaround. Oh, I was just providing my justification for agreeing with you. I was considering putting the check for the RSS flag on master to be removing the workaround, so I don't think there is anything to be done beyon
Re: [ovs-dev] [PATCH] dpif-netdev: Check for PKT_RX_RSS_HASH flag.
On Tue, Jun 23, 2015 at 7:09 PM, Jesse Gross wrote: > On Tue, Jun 23, 2015 at 7:06 PM, Pravin Shelar wrote: >> On Tue, Jun 23, 2015 at 2:51 PM, Jesse Gross wrote: >>> On Mon, Jun 22, 2015 at 8:08 PM, Pravin Shelar wrote: On Fri, Jun 19, 2015 at 11:24 AM, Daniele Di Proietto wrote: > > > On 18/06/2015 23:57, "Traynor, Kevin" wrote: > >> >> >>> -Original Message- >> >>> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Daniele Di >> >>> Proietto >> >>> Sent: Tuesday, June 16, 2015 7:39 PM >> >>> To: dev@openvswitch.org >> >>> Subject: [ovs-dev] [PATCH] dpif-netdev: Check for PKT_RX_RSS_HASH flag. >> >>> >> >>> DPDK mbufs contain a valid RSS hash only if PKT_RX_RSS_HASH is >> >>> set in 'ol_flags'. Otherwise the hash is garbage and doesn't >> >>> relate to the packet. >> >>> >> >>> This fixes an issue with vhost, which, being a virtual NIC, doesn't >> >>> compute the hash. >> >>> >> >>> Unfortunately the ixgbe vPMD doesn't set the PKT_RX_RSS_HASH, forcing >> >>> OVS to compute an hash is software. This has a significant impact on >> >>> performance (-30% throughput in a single flow setup) which can be >> >>> mitigated in the CPU supports crc32c instructions. >> >> >> >>As per the other thread on this I'm a bit concerned about the performance >> >>drop from this patch, so I did some testing of this and alternative/ >> >>complimentary solutions. >> >> >> >>Here's the options I looked at and some comments: >> >>1. This patch in isolation: vhost drops about ~15% vhost-vhost and >> >>phy-vhost-phy (because of sw hash) but also there is drops of ~25% for >> >>phy-phy and ~15% drop for phy-ivshmem-phy. >> >> >> >>2. Leave the code as is and let EMC misses happen for vhost rx pkts: >> >>I measure this at ~35% drop if missed *everytime* for vhost-vhost. We >> >>see in testing that it can also never happen, but this is not realistic. >> >>There should be no impact to other DPDK interfaces. >> >> >> >>3. Add hash reset for packets from vhost: This is another way of forcing >> >>the software hash for vhost rx and it is roughly equivalent in performance >> >>to 1. for vhost-vhost (~15% drop). While there is a no significant drop >> >>for phy-vhost-phy. There should be no impact to other DPDK interfaces. >> >> >> >>4. Apply this patch and turn off Rx Vectorisation. vhost-vhost will drop >> >>~15% as per 1. and there should be nothing significant for phy-vhost-phy. >> >>We would lose the 10% gain that rx vectorisation gave us for phy-phy. >> >>There should be no impact for dpdkr ports. >> >> >> >> >> >>In terms of not knowing whether the hw hash is valid or not if the flag is >> >>not checked, I would have expected the pmd to return an error on config if >> >>the hash wasn't supported, but I'm not sure that it does. >> >>In the worst case where there was an incorrect hash, it would miss the EMC >> >>which is about a 45% drop for phy-phy. I would think it's pretty safe that >> >>if we configure it, the hash will be correct but I guess there is a >> >>possibility it wouldn't be. >> >> >> >>Even if it is possible to get a smaller patch to fix the underlying issue >> >>in DPDK, it would be in DPDK 2.1 at the earliest meaning the performance >> >>would remain low until sometime in August. If it's DPDK 2.2, then it would >> >>be sometime in December. This would mean any performance drops would be >> >>present in OVS 2.4 and possibly OVS 2.5. >> >> >> >>Sorry :( but based on the performance drop with this patch in isolation it >> >>would be a NAK from me. My preference would be 3 which gives best >>performance, >> >>or 4 which is a bit lower for phy-phy but safer. >> >> >> >>Kevin. > > Thanks for all the testing. I guess it might make sense to stretch our > interpretation of the API in this case, because it wouldn't affect > correctness. > > Unless there any other objection I'm fine with the 3rd approach. > We can use 3rd approach to fix issue on branch 2.4. Then have patch to check the PKT_RX_RSS_HASH flag on master. By the time we release branch 2.5 we will have proper fix in DPDK and performance will bounce back. >>> >>> I think this is probably a reasonable compromise. I think it's better >>> to not keep a workaround in for an unbounded amount of time, otherwise >>> we'll forget about it and it will come back to bite us in the future. >> >> ok, Once the DPDK fix is backported to DPDK 2.0, we can remove the >> workaround
Re: [ovs-dev] [PATCH] dpif-netdev: Check for PKT_RX_RSS_HASH flag.
On Tue, Jun 23, 2015 at 7:22 PM, Pravin Shelar wrote: > On Tue, Jun 23, 2015 at 7:09 PM, Jesse Gross wrote: >> On Tue, Jun 23, 2015 at 7:06 PM, Pravin Shelar wrote: >>> On Tue, Jun 23, 2015 at 2:51 PM, Jesse Gross wrote: On Mon, Jun 22, 2015 at 8:08 PM, Pravin Shelar wrote: > On Fri, Jun 19, 2015 at 11:24 AM, Daniele Di Proietto > wrote: >> >> >> On 18/06/2015 23:57, "Traynor, Kevin" wrote: >> >>> >>> -Original Message- >>> From: dev [mailto:dev-boun...@openvswitch.org] On Behalf Of Daniele Di >>> Proietto >>> Sent: Tuesday, June 16, 2015 7:39 PM >>> To: dev@openvswitch.org >>> Subject: [ovs-dev] [PATCH] dpif-netdev: Check for PKT_RX_RSS_HASH flag. >>> >>> DPDK mbufs contain a valid RSS hash only if PKT_RX_RSS_HASH is >>> set in 'ol_flags'. Otherwise the hash is garbage and doesn't >>> relate to the packet. >>> >>> This fixes an issue with vhost, which, being a virtual NIC, doesn't >>> compute the hash. >>> >>> Unfortunately the ixgbe vPMD doesn't set the PKT_RX_RSS_HASH, forcing >>> OVS to compute an hash is software. This has a significant impact on >>> performance (-30% throughput in a single flow setup) which can be >>> mitigated in the CPU supports crc32c instructions. >>> >>> >>> >>>As per the other thread on this I'm a bit concerned about the performance >>> >>>drop from this patch, so I did some testing of this and alternative/ >>> >>>complimentary solutions. >>> >>> >>> >>>Here's the options I looked at and some comments: >>> >>>1. This patch in isolation: vhost drops about ~15% vhost-vhost and >>> >>>phy-vhost-phy (because of sw hash) but also there is drops of ~25% for >>> >>>phy-phy and ~15% drop for phy-ivshmem-phy. >>> >>> >>> >>>2. Leave the code as is and let EMC misses happen for vhost rx pkts: >>> >>>I measure this at ~35% drop if missed *everytime* for vhost-vhost. We >>> >>>see in testing that it can also never happen, but this is not realistic. >>> >>>There should be no impact to other DPDK interfaces. >>> >>> >>> >>>3. Add hash reset for packets from vhost: This is another way of forcing >>> >>>the software hash for vhost rx and it is roughly equivalent in >>>performance >>> >>>to 1. for vhost-vhost (~15% drop). While there is a no significant drop >>> >>>for phy-vhost-phy. There should be no impact to other DPDK interfaces. >>> >>> >>> >>>4. Apply this patch and turn off Rx Vectorisation. vhost-vhost will drop >>> >>>~15% as per 1. and there should be nothing significant for phy-vhost-phy. >>> >>>We would lose the 10% gain that rx vectorisation gave us for phy-phy. >>> >>>There should be no impact for dpdkr ports. >>> >>> >>> >>> >>> >>>In terms of not knowing whether the hw hash is valid or not if the flag >>>is >>> >>>not checked, I would have expected the pmd to return an error on config >>>if >>> >>>the hash wasn't supported, but I'm not sure that it does. >>> >>>In the worst case where there was an incorrect hash, it would miss the >>>EMC >>> >>>which is about a 45% drop for phy-phy. I would think it's pretty safe >>>that >>> >>>if we configure it, the hash will be correct but I guess there is a >>> >>>possibility it wouldn't be. >>> >>> >>> >>>Even if it is possible to get a smaller patch to fix the underlying issue >>> >>>in DPDK, it would be in DPDK 2.1 at the earliest meaning the performance >>> >>>would remain low until sometime in August. If it's DPDK 2.2, then it >>>would >>> >>>be sometime in December. This would mean any performance drops would be >>> >>>present in OVS 2.4 and possibly OVS 2.5. >>> >>> >>> >>>Sorry :( but based on the performance drop with this patch in isolation >>>it >>> >>>would be a NAK from me. My preference would be 3 which gives best >>>performance, >>> >>>or 4 which is a bit lower for phy-phy but safer. >>> >>> >>> >>>Kevin. >> >> Thanks for all the testing. I guess it might make sense to stretch our >> interpretation of the API in this case, because it wouldn't affect >> correctness. >> >> Unless there any other objection I'm fine with the 3rd approach. >> > > We can use 3rd approach to fix issue on branch 2.4. Then have patch to > check the PKT_RX_RSS_HASH flag on master. By the time we release > branch 2.5 we will have proper fix in DPDK and performance will bounce > back. I think this is probably a reasonable comp
