Re: [ovs-dev] [PATCH] ofproto: ofp_packet_out messages in bundles
I've sent the new patch. I wasn't sure how to do it so I just sent the
first patch with subject [PATCH 1/2] and the new patch with [PATCH 2/2].
Hope it doesn't cause much problem. Alternatively I can send a pull request
in github if you want.
Thanks.
Jarno Rajahalme escreveu no dia quinta, 14/07/2016 às 10:11:
> I've been working on adding group support for bundles for the last week or
> so, so I have most of the related code fresh in my head now. So, please
> submit a new RFC patch with the changes you can make and I'll take it from
> there.
>
> Jarno
>
> On Jul 13, 2016, at 5:35 PM, André Mantas wrote:
>
> Thanks for the review.
>
> I'm not sure if I will be able to address your second point tho.
> Would this be a problem?
>
> As for the third point, would ofproto.c be a good place for the helper
> function?
> Something like:
> static enum ofperr
> ofproto_extract_packet_out(struct ofproto *p, const struct ofp_header *oh,
>struct ofputil_packet_out *po,
>struct dp_packet *payload, struct flow *flow)
>
> Basically it receives pointers and validates + fills them.
> The caller can then invoke ofproto_class->packet_out() passing the args
> that were filled.
>
> Jarno Rajahalme escreveu no dia quarta, 29/06/2016 às
> 12:36:
>
>>
>> > On Jun 23, 2016, at 4:51 AM, Andre Mantas
>> wrote:
>> >
>> > This patch allows ofp_packet_out messages to be added to bundles. In a
>> > multi controller scenario, packet_out messages inside bundles can serve
>> > as a commit_reply for other controllers - since the original
>> commit_reply
>> > is only forwarded to the controller that sent the commit_request.
>> >
>> > Tested with make check and external controller adding packet_out
>> messages
>> > to a bundle with different destinations (hosts and controllers). Also
>> tested
>> > grouping packet_out with port_mod messages in the same bundle. After
>> > committing the bundle, the destinations received the packet_out.
>> >
>> > Signed-off-by: Andre Mantas
>> > Submitted-at: https://github.com/openvswitch/ovs/pull/117
>> > ---
>> > lib/ofp-util.c | 2 +-
>> > ofproto/bundles.h | 5
>> > ofproto/ofproto-provider.h | 7 +
>> > ofproto/ofproto.c | 70
>> --
>> > 4 files changed, 81 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/lib/ofp-util.c b/lib/ofp-util.c
>> > index c5353cc..6725220 100644
>> > --- a/lib/ofp-util.c
>> > +++ b/lib/ofp-util.c
>> > @@ -9578,6 +9578,7 @@ ofputil_is_bundlable(enum ofptype type)
>> > /* Minimum required by OpenFlow 1.4. */
>> > case OFPTYPE_PORT_MOD:
>> > case OFPTYPE_FLOW_MOD:
>> > +case OFPTYPE_PACKET_OUT:
>> > return true;
>> >
>>
>> You should update the preceding comment, too.
>>
>> > /* Nice to have later. */
>> > @@ -9585,7 +9586,6 @@ ofputil_is_bundlable(enum ofptype type)
>> > case OFPTYPE_GROUP_MOD:
>> > case OFPTYPE_TABLE_MOD:
>> > case OFPTYPE_METER_MOD:
>> > -case OFPTYPE_PACKET_OUT:
>> > case OFPTYPE_NXT_TLV_TABLE_MOD:
>> >
>> > /* Not to be bundlable. */
>> > diff --git a/ofproto/bundles.h b/ofproto/bundles.h
>> > index d045031..61c1b48 100644
>> > --- a/ofproto/bundles.h
>> > +++ b/ofproto/bundles.h
>> > @@ -22,6 +22,7 @@
>> > #include
>> >
>> > #include "connmgr.h"
>> > +#include "dp-packet.h"
>> > #include "ofproto-provider.h"
>> > #include "openvswitch/ofp-msgs.h"
>> > #include "openvswitch/ofp-util.h"
>> > @@ -37,6 +38,7 @@ struct ofp_bundle_entry {
>> > union {
>> > struct ofproto_flow_mod ofm; /* ofm.fm.ofpacts must be
>> malloced. */
>> > struct ofproto_port_mod opm;
>> > +struct ofproto_packet_out opo; /* opo.po.ofpacts must be
>> malloced */
>> > };
>> >
>> > /* OpenFlow header and some of the message contents for error
>> reporting. */
>> > @@ -89,6 +91,9 @@ ofp_bundle_entry_free(struct ofp_bundle_entry *entry)
>> > if (entry) {
>> > if (entry->type == OFPTYPE_FLOW_MOD) {
>> > free(entry->ofm.fm.ofpacts);
>> > +} else if (entry->type == OFPTYPE_PACKET_OUT) {
Re: [ovs-dev] [PATCH] ofproto: ofp_packet_out messages in bundles
Thanks for the review.
I'm not sure if I will be able to address your second point tho.
Would this be a problem?
As for the third point, would ofproto.c be a good place for the helper
function?
Something like:
static enum ofperr
ofproto_extract_packet_out(struct ofproto *p, const struct ofp_header *oh,
struct ofputil_packet_out *po,
struct dp_packet *payload, struct flow *flow)
Basically it receives pointers and validates + fills them.
The caller can then invoke ofproto_class->packet_out() passing the args
that were filled.
Jarno Rajahalme escreveu no dia quarta, 29/06/2016 às 12:36:
>
> > On Jun 23, 2016, at 4:51 AM, Andre Mantas
> wrote:
> >
> > This patch allows ofp_packet_out messages to be added to bundles. In a
> > multi controller scenario, packet_out messages inside bundles can serve
> > as a commit_reply for other controllers - since the original commit_reply
> > is only forwarded to the controller that sent the commit_request.
> >
> > Tested with make check and external controller adding packet_out messages
> > to a bundle with different destinations (hosts and controllers). Also
> tested
> > grouping packet_out with port_mod messages in the same bundle. After
> > committing the bundle, the destinations received the packet_out.
> >
> > Signed-off-by: Andre Mantas
> > Submitted-at: https://github.com/openvswitch/ovs/pull/117
> > ---
> > lib/ofp-util.c | 2 +-
> > ofproto/bundles.h | 5
> > ofproto/ofproto-provider.h | 7 +
> > ofproto/ofproto.c | 70
> --
> > 4 files changed, 81 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> > index c5353cc..6725220 100644
> > --- a/lib/ofp-util.c
> > +++ b/lib/ofp-util.c
> > @@ -9578,6 +9578,7 @@ ofputil_is_bundlable(enum ofptype type)
> > /* Minimum required by OpenFlow 1.4. */
> > case OFPTYPE_PORT_MOD:
> > case OFPTYPE_FLOW_MOD:
> > +case OFPTYPE_PACKET_OUT:
> > return true;
> >
>
> You should update the preceding comment, too.
>
> > /* Nice to have later. */
> > @@ -9585,7 +9586,6 @@ ofputil_is_bundlable(enum ofptype type)
> > case OFPTYPE_GROUP_MOD:
> > case OFPTYPE_TABLE_MOD:
> > case OFPTYPE_METER_MOD:
> > -case OFPTYPE_PACKET_OUT:
> > case OFPTYPE_NXT_TLV_TABLE_MOD:
> >
> > /* Not to be bundlable. */
> > diff --git a/ofproto/bundles.h b/ofproto/bundles.h
> > index d045031..61c1b48 100644
> > --- a/ofproto/bundles.h
> > +++ b/ofproto/bundles.h
> > @@ -22,6 +22,7 @@
> > #include
> >
> > #include "connmgr.h"
> > +#include "dp-packet.h"
> > #include "ofproto-provider.h"
> > #include "openvswitch/ofp-msgs.h"
> > #include "openvswitch/ofp-util.h"
> > @@ -37,6 +38,7 @@ struct ofp_bundle_entry {
> > union {
> > struct ofproto_flow_mod ofm; /* ofm.fm.ofpacts must be
> malloced. */
> > struct ofproto_port_mod opm;
> > +struct ofproto_packet_out opo; /* opo.po.ofpacts must be
> malloced */
> > };
> >
> > /* OpenFlow header and some of the message contents for error
> reporting. */
> > @@ -89,6 +91,9 @@ ofp_bundle_entry_free(struct ofp_bundle_entry *entry)
> > if (entry) {
> > if (entry->type == OFPTYPE_FLOW_MOD) {
> > free(entry->ofm.fm.ofpacts);
> > +} else if (entry->type == OFPTYPE_PACKET_OUT) {
> > +dp_packet_delete(entry->opo.payload);
> > +free(entry->opo.po.ofpacts);
> > }
> > free(entry);
> > }
> > diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> > index daa0077..c98f110 100644
> > --- a/ofproto/ofproto-provider.h
> > +++ b/ofproto/ofproto-provider.h
> > @@ -1814,6 +1814,13 @@ struct ofproto_port_mod {
> > struct ofport *port;/* Affected port. */
> > };
> >
> > +/* packet_out with execution context. */
> > +struct ofproto_packet_out {
> > +struct ofputil_packet_out po;
> > +struct dp_packet *payload;
> > +struct flow flow;
> > +};
> > +
> > enum ofperr ofproto_flow_mod(struct ofproto *, struct ofproto_flow_mod *)
> > OVS_EXCLUDED(ofproto_mutex);
> > void ofproto_add_flow(struct ofproto *, const struct match *, int
> priority,
> > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> > index ff6affd..c5a6097 100644
> > --- a/ofproto/ofproto.c
> > +++ b/ofproto/ofproto.c
> > @@ -6996,6 +6996,8 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t
> id, uint16_t flags)
> > * effect. */
> > be->ofm.version = version;
> > error = ofproto_flow_mod_start(ofproto, &be->ofm);
> > +} else if (be->type == OFPTYPE_PACKET_OUT) {
> > +prev_is_port_mod = false;
> > } else {
> > OVS_NOT_REACHED();
> > }
> > @@ -7016,7 +7018,7 @@ do_bundle_commit(struct ofconn *ofconn, uint32_t
> id, uint16_t flags)
> > if (be->type == OFPTYPE_FLOW_MOD)
Re: [ovs-dev] Implementation of Packet-Out in Bundles
Hi again. I've sent an email with the patch using git send-email, I hope it reaches the mailing list. Here is the git send-email output: OK. Log says: Server: smtp.googlemail.com MAIL FROM: RCPT TO: RCPT TO: From: Andre Mantas To: dev@openvswitch.org Cc: Andre Mantas Subject: [PATCH] ofproto: ofp_packet_out messages in bundles Date: Thu, 23 Jun 2016 12:51:57 +0100 Message-Id: <1466682717-7645-1-git-send-email-aman...@lasige.di.fc.ul.pt> X-Mailer: git-send-email 2.5.0 Result: 250 2.0.0 OK 1466682772 b4sm4055474wjd.16 - gsmtp The patch is attached just in case. Let me know if there are any mistakes/problems. Ben Pfaff escreveu no dia quarta, 18/05/2016 às 03:09: > I'm just behind on reviews, as usual. > > Pull requests always get reviewed more slowly than patches emailed to > the ovs-dev, by the way, since they're off to the side and not visible > in patchwork or the email list. For prompt (or more prompt, at least) > reviews, I always recommend emailing patches. > > On Tue, May 17, 2016 at 11:06:24AM +, André Mantas wrote: > > Any news on this? > > > > Ben Pfaff escreveu no dia terça, 19/04/2016 às 00:33: > > > > > Thanks. I see the pull request. > > > > > > We'd be happy to have support for additional OpenFlow request in > > > bundles. > > > > > > On Mon, Apr 18, 2016 at 09:59:33PM +, André Mantas wrote: > > > > Hi. Sorry for the long delay. Just issued a pull request in github, I > > > hope > > > > thats ok. Please let me know if I did something wrong. > > > > > > > > I'm also interested in adding support for group_mod, meter_mod and > > > > table_mod in the future (if that's ok with you). > > > > > > > > Ben Pfaff escreveu no dia terça, 29/03/2016 às 16:22: > > > > > > > > > On Tue, Mar 15, 2016 at 09:21:40PM +, André Mantas wrote: > > > > > > I think one option could be do extra validations depending on the > > > error > > > > > > returned by ofproto_check_ofpacts like checking if previous > entries > > > in > > > > > the > > > > > > bundle would make the validation successful. But since group and > > > meter > > > > > mods > > > > > > are not implemented yet this isn't feasible. > > > > > > I need to focus on my project for now but later I might be able > to > > > help > > > > > > with group and meter mods. > > > > > > > > > > > > To submit the patch is "just" follow CONTRIBUTING.md, right? > > > > > > > > > > Yes, that's correct. > > > > > > > > > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Implementation of Packet-Out in Bundles
Any news on this? Ben Pfaff escreveu no dia terça, 19/04/2016 às 00:33: > Thanks. I see the pull request. > > We'd be happy to have support for additional OpenFlow request in > bundles. > > On Mon, Apr 18, 2016 at 09:59:33PM +, André Mantas wrote: > > Hi. Sorry for the long delay. Just issued a pull request in github, I > hope > > thats ok. Please let me know if I did something wrong. > > > > I'm also interested in adding support for group_mod, meter_mod and > > table_mod in the future (if that's ok with you). > > > > Ben Pfaff escreveu no dia terça, 29/03/2016 às 16:22: > > > > > On Tue, Mar 15, 2016 at 09:21:40PM +, André Mantas wrote: > > > > I think one option could be do extra validations depending on the > error > > > > returned by ofproto_check_ofpacts like checking if previous entries > in > > > the > > > > bundle would make the validation successful. But since group and > meter > > > mods > > > > are not implemented yet this isn't feasible. > > > > I need to focus on my project for now but later I might be able to > help > > > > with group and meter mods. > > > > > > > > To submit the patch is "just" follow CONTRIBUTING.md, right? > > > > > > Yes, that's correct. > > > > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Implementation of Packet-Out in Bundles
Hi. Sorry for the long delay. Just issued a pull request in github, I hope thats ok. Please let me know if I did something wrong. I'm also interested in adding support for group_mod, meter_mod and table_mod in the future (if that's ok with you). Ben Pfaff escreveu no dia terça, 29/03/2016 às 16:22: > On Tue, Mar 15, 2016 at 09:21:40PM +, André Mantas wrote: > > I think one option could be do extra validations depending on the error > > returned by ofproto_check_ofpacts like checking if previous entries in > the > > bundle would make the validation successful. But since group and meter > mods > > are not implemented yet this isn't feasible. > > I need to focus on my project for now but later I might be able to help > > with group and meter mods. > > > > To submit the patch is "just" follow CONTRIBUTING.md, right? > > Yes, that's correct. > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Implementation of Packet-Out in Bundles
I see. I think one option could be do extra validations depending on the error returned by ofproto_check_ofpacts like checking if previous entries in the bundle would make the validation successful. But since group and meter mods are not implemented yet this isn't feasible. I need to focus on my project for now but later I might be able to help with group and meter mods. To submit the patch is "just" follow CONTRIBUTING.md, right? Jarno Rajahalme escreveu no dia terça, 15/03/2016 às 17:31: > I think I’ll worry about that when I get to adding group and meter support. > > Jarno > > > On Mar 14, 2016, at 8:47 PM, Ben Pfaff wrote: > > > > Jarno, what do you think? > > > > On Mon, Mar 14, 2016 at 11:16:27PM +, André Mantas wrote: > >> Ok. And what about Packet_Out validation complicating the > implementation of > >> group and meter mod in bundles? > >> > >> Should I create new validation functions that don't check for group and > >> meter in the action of Packet_Out? > >> > >> Ben Pfaff escreveu no dia segunda, 14/03/2016 às 16:54: > >> > >>> On Wed, Mar 09, 2016 at 09:10:41AM +, André Mantas wrote: > >>>> By the way, I'm still starting vswitchd with valgrind and detected > some > >>>> memory errors after using "sudo ovs-appctl exit". Is there other way > to > >>>> exit/stop ovs? > >>>> > >>>> To run I used: valgrind --tool=memcheck --leak-check=full ovs-vswitchd > >>>> --pidfile --detach -v --log-file > >>> > >>> These aren't anything to worry about. OVS does a number of one-time > >>> allocations that it doesn't bother to release. Some of them get > >>> released when OVS exits, some of them don't. > >>> > >>> We'd take patches to release them, but it's not an important problem. > >>> > > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Implementation of Packet-Out in Bundles
Ok. And what about Packet_Out validation complicating the implementation of group and meter mod in bundles? Should I create new validation functions that don't check for group and meter in the action of Packet_Out? Ben Pfaff escreveu no dia segunda, 14/03/2016 às 16:54: > On Wed, Mar 09, 2016 at 09:10:41AM +0000, André Mantas wrote: > > By the way, I'm still starting vswitchd with valgrind and detected some > > memory errors after using "sudo ovs-appctl exit". Is there other way to > > exit/stop ovs? > > > > To run I used: valgrind --tool=memcheck --leak-check=full ovs-vswitchd > > --pidfile --detach -v --log-file > > These aren't anything to worry about. OVS does a number of one-time > allocations that it doesn't bother to release. Some of them get > released when OVS exits, some of them don't. > > We'd take patches to release them, but it's not an important problem. > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Implementation of Packet-Out in Bundles
nc-append-aio.c:144) ==2344==by 0x4F3065: vlog_valist (vlog.c:1124) ==2344==by 0x4F28BE: vlog (vlog.c:1148) ==2344==by 0x40F9E8: bridge_run (bridge.c:3011) ==2344==by 0x404FB4: main (ovs-vswitchd.c:120) ==2344== ==2344== 576 bytes in 1 blocks are possibly lost in loss record 361 of 378 ==2344==at 0x4C2DB95: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==2344==by 0x4013504: allocate_dtv (dl-tls.c:322) ==2344==by 0x4013504: _dl_allocate_tls (dl-tls.c:544) ==2344==by 0x4E400D2: allocate_stack (allocatestack.c:588) ==2344==by 0x4E400D2: pthread_create@@GLIBC_2.2.5 (pthread_create.c:537) ==2344==by 0x4C4DDA: ovs_thread_create (ovs-thread.c:389) ==2344==by 0x4C33BE: ovsrcu_quiesced (ovs-rcu.c:111) ==2344==by 0x4E6F63: time_poll (timeval.c:295) ==2344==by 0x4D3B1B: poll_block (poll-loop.c:364) ==2344==by 0x43724E: udpif_upcall_handler (ofproto-dpif-upcall.c:725) ==2344==by 0x4C41C3: ovsthread_wrapper (ovs-thread.c:340) ==2344==by 0x4E3F6A9: start_thread (pthread_create.c:333) ==2344==by 0x566CE9C: clone (clone.S:109) ==2344== ==2344== 3,484 (144 direct, 3,340 indirect) bytes in 2 blocks are definitely lost in loss record 371 of 378 ==2344==at 0x4C2BBCF: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==2344==by 0x4EC244: xmalloc (util.c:112) ==2344==by 0x52CE3C: vconn_stream_new (vconn-stream.c:60) ==2344==by 0x52CF9D: pvconn_pstream_accept (vconn-stream.c:350) ==2344==by 0x4F0909: pvconn_accept (vconn.c:1263) ==2344==by 0x44756C: connmgr_run (connmgr.c:365) ==2344==by 0x41A0A5: ofproto_run (ofproto.c:1762) ==2344==by 0x408C53: bridge_run__ (bridge.c:2885) ==2344==by 0x40E513: bridge_run (bridge.c:2940) ==2344==by 0x404FB4: main (ovs-vswitchd.c:120) ==2344== ==2344== LEAK SUMMARY: ==2344==definitely lost: 144 bytes in 2 blocks ==2344==indirectly lost: 3,340 bytes in 10 blocks ==2344== possibly lost: 1,880 bytes in 8 blocks ==2344==still reachable: 163,193 bytes in 458 blocks ==2344== suppressed: 0 bytes in 0 blocks ==2344== Reachable blocks (those to which a pointer was found) are not shown. ==2344== To see them, rerun with: --leak-check=full --show-leak-kinds=all ==2344== ==2344== For counts of detected and suppressed errors, rerun with: -v ==2344== ERROR SUMMARY: 8 errors from 8 contexts (suppressed: 0 from 0) André Mantas escreveu no dia quarta, 9/03/2016 às 00:35: > You want config=0 mask=1, I believe. >> > > Oh, that was it. It worked as expected. > > The test was also successful. A snoop extract is below, but basically what > I've done is: send port mod to bring down port 2; create bundle with 2 > messages - port mod to bring up port 2 and packet out to port 2; host > connected to port 2 receives the packet out (confirmed with tcpdump on host > xterm). > > From Jarno's reply, I think the last problem to address is: "Group and > meter validation is part of the action validation for packet_out. While the > bundles can’t currently include group or meter mods, I’d like to see that > adding packet_out to a bundle does not unnecessarily complicate adding > group and meter mods to bundles." > > Not sure what to do about this one... > > -- > > snoop extract: > > OFPT_PORT_MOD (OF1.4) (xid=0x46): port: 2: addr:9e:15:f7:8d:6f:df > config: PORT_DOWN > mask: PORT_DOWN > advertise: UNCHANGED > OFPT_PORT_STATUS (OF1.4) (xid=0x0): MOD: 2(s1-eth2): addr:9e:15:f7:8d:6f:df > config: PORT_DOWN > state: LINK_DOWN > current:10GB-FD COPPER > speed: 1 Mbps now, 0 Mbps max > > OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x57): > bundle_id=0xb type=OPEN_REQUEST flags=ordered > OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x57): > bundle_id=0xb type=OPEN_REPLY flags=0 > OFPT_BUNDLE_ADD_MESSAGE (OF1.4) (xid=0x55): > bundle_id=0xb flags=ordered > OFPT_PORT_MOD (OF1.4) (xid=0x55): port: 2: addr:9e:15:f7:8d:6f:df > config: 0 > mask: PORT_DOWN > advertise: UNCHANGED > OFPT_BUNDLE_ADD_MESSAGE (OF1.4) (xid=0x56): > bundle_id=0xb flags=ordered > OFPT_PACKET_OUT (OF1.4) (xid=0x56): in_port=1 actions=output:2 data_len=20 > > vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=0a:ae:d4:f6:70:30,dl_type=0x > OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x5a): > bundle_id=0xb type=CLOSE_REQUEST flags=ordered > OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x5a): > bundle_id=0xb type=CLOSE_REPLY flags=0 > OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x5b): > bundle_id=0xb type=COMMIT_REQUEST flags=ordered > OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x5b): > bundle_id=0xb type=COMMIT_REPLY flags=0 > OFPT_PORT_STATUS (OF1.4) (xid=0x0): MOD: 2(s1-eth2): addr:9e:15:f7:8d:6f:df > config: 0 > state: 0 > current:10GB-FD COPPER > speed: 1 Mbps now, 0 Mbps max > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Implementation of Packet-Out in Bundles
> > You want config=0 mask=1, I believe. > Oh, that was it. It worked as expected. The test was also successful. A snoop extract is below, but basically what I've done is: send port mod to bring down port 2; create bundle with 2 messages - port mod to bring up port 2 and packet out to port 2; host connected to port 2 receives the packet out (confirmed with tcpdump on host xterm). From Jarno's reply, I think the last problem to address is: "Group and meter validation is part of the action validation for packet_out. While the bundles can’t currently include group or meter mods, I’d like to see that adding packet_out to a bundle does not unnecessarily complicate adding group and meter mods to bundles." Not sure what to do about this one... -- snoop extract: OFPT_PORT_MOD (OF1.4) (xid=0x46): port: 2: addr:9e:15:f7:8d:6f:df config: PORT_DOWN mask: PORT_DOWN advertise: UNCHANGED OFPT_PORT_STATUS (OF1.4) (xid=0x0): MOD: 2(s1-eth2): addr:9e:15:f7:8d:6f:df config: PORT_DOWN state: LINK_DOWN current:10GB-FD COPPER speed: 1 Mbps now, 0 Mbps max OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x57): bundle_id=0xb type=OPEN_REQUEST flags=ordered OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x57): bundle_id=0xb type=OPEN_REPLY flags=0 OFPT_BUNDLE_ADD_MESSAGE (OF1.4) (xid=0x55): bundle_id=0xb flags=ordered OFPT_PORT_MOD (OF1.4) (xid=0x55): port: 2: addr:9e:15:f7:8d:6f:df config: 0 mask: PORT_DOWN advertise: UNCHANGED OFPT_BUNDLE_ADD_MESSAGE (OF1.4) (xid=0x56): bundle_id=0xb flags=ordered OFPT_PACKET_OUT (OF1.4) (xid=0x56): in_port=1 actions=output:2 data_len=20 vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=0a:ae:d4:f6:70:30,dl_type=0x OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x5a): bundle_id=0xb type=CLOSE_REQUEST flags=ordered OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x5a): bundle_id=0xb type=CLOSE_REPLY flags=0 OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x5b): bundle_id=0xb type=COMMIT_REQUEST flags=ordered OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x5b): bundle_id=0xb type=COMMIT_REPLY flags=0 OFPT_PORT_STATUS (OF1.4) (xid=0x0): MOD: 2(s1-eth2): addr:9e:15:f7:8d:6f:df config: 0 state: 0 current:10GB-FD COPPER speed: 1 Mbps now, 0 Mbps max ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Implementation of Packet-Out in Bundles
After some more debugging and testing packet outs are being forwarded to both controllers and hosts when added to a bundle. I've tested with 2 controllers, 3 switches and 4 hosts with controllers sending packet outs to each other and pinging between hosts with pingall from mininet. How "good enough" is this from your perspective? I know I still need to write appropriate tests. Then I tried grouping a PortMod and a PacketOut together in the same bundle to test how enabling or disabling a port affects PacketOut validations. However, I'm not being able to set a port to "up" from the controller. I can put it "down" with a PortMod config=1 mask=1. but when I try to send a PortMod with config=1 mask=0, the port stays down. In wireshark I see the config=1 and mask=0 but in ovs-ofctl snoop it says config:0 mask:0. I can set the port down with ovs-ofctl mod-port so maybe I'm doing something wrong? Ben Pfaff escreveu no dia quinta, 3/03/2016 às 23:31: > It should still work, if you use ovs-ctl while testing. Usually, > though, when I'm testing, I just run ovs-vswitchd from the command-line, > and in that case you can just invoke "valgrind ovs-vswitchd ...". > > OVS_CTL_OPTS is just an environment variable so you can define it > wherever that is convenient. > > On Thu, Mar 03, 2016 at 10:51:24PM +, André Mantas wrote: > > Looks like what I need. What's the quickest way to enable valgrind? > > > > I found this: > http://openvswitch.org/pipermail/discuss/2013-May/010113.html > > > > Does it still apply to today? Where can I define OVS_CTL_OPTS? > > > > Ben Pfaff escreveu no dia quinta, 3/03/2016 às 18:16: > > > > > On Thu, Mar 03, 2016 at 05:54:42PM +, André Mantas wrote: > > > > Hi again. > > > > > > > > For now, I'm testing the implementation with a floodlight controller > that > > > > opens a bundle, adds a packet_out message and tries to commit the > bundle. > > > > I'm using mininet to start a simple topology with 1 switch and 2 > hosts > > > and > > > > connect the switch to the remote controller. I followed INSTALL.md to > > > start > > > > ovs before running the mininet script. > > > > > > > > Everything works like before if the controller does not send a > packet_out > > > > in a bundle. However, when I try to send the packet_out to the > bundle, > > > the > > > > switch disconnects from the controller after the commit request > message > > > is > > > > sent and all ovs-ofctl commands fail on that bridge. > > > > > > > > To debug, I added some VLOG_INFO calls in handle_bundle_add and > > > > do_bundle_commit functions. The prints (found in /var/log/syslog) are > > > below > > > > together with the output of "ovs-ofctl snoop s1" > > > > > > > > Looks like adding the message to the bundle and committing the > bundle is > > > ok > > > > but something happens afterwards? > > > > Where would be a good place to look for the problem? > > > > > > Usually when I get an unexpected connection close while debugging, it > > > means that ovs-vswitchd crashed. I recommend enabling core dumps and > > > then running GDB on the core. "valgrind" is also a good approach. > > > > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Implementation of Packet-Out in Bundles
Looks like what I need. What's the quickest way to enable valgrind? I found this: http://openvswitch.org/pipermail/discuss/2013-May/010113.html Does it still apply to today? Where can I define OVS_CTL_OPTS? Ben Pfaff escreveu no dia quinta, 3/03/2016 às 18:16: > On Thu, Mar 03, 2016 at 05:54:42PM +0000, André Mantas wrote: > > Hi again. > > > > For now, I'm testing the implementation with a floodlight controller that > > opens a bundle, adds a packet_out message and tries to commit the bundle. > > I'm using mininet to start a simple topology with 1 switch and 2 hosts > and > > connect the switch to the remote controller. I followed INSTALL.md to > start > > ovs before running the mininet script. > > > > Everything works like before if the controller does not send a packet_out > > in a bundle. However, when I try to send the packet_out to the bundle, > the > > switch disconnects from the controller after the commit request message > is > > sent and all ovs-ofctl commands fail on that bridge. > > > > To debug, I added some VLOG_INFO calls in handle_bundle_add and > > do_bundle_commit functions. The prints (found in /var/log/syslog) are > below > > together with the output of "ovs-ofctl snoop s1" > > > > Looks like adding the message to the bundle and committing the bundle is > ok > > but something happens afterwards? > > Where would be a good place to look for the problem? > > Usually when I get an unexpected connection close while debugging, it > means that ovs-vswitchd crashed. I recommend enabling core dumps and > then running GDB on the core. "valgrind" is also a good approach. > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Implementation of Packet-Out in Bundles
Hi again. For now, I'm testing the implementation with a floodlight controller that opens a bundle, adds a packet_out message and tries to commit the bundle. I'm using mininet to start a simple topology with 1 switch and 2 hosts and connect the switch to the remote controller. I followed INSTALL.md to start ovs before running the mininet script. Everything works like before if the controller does not send a packet_out in a bundle. However, when I try to send the packet_out to the bundle, the switch disconnects from the controller after the commit request message is sent and all ovs-ofctl commands fail on that bridge. To debug, I added some VLOG_INFO calls in handle_bundle_add and do_bundle_commit functions. The prints (found in /var/log/syslog) are below together with the output of "ovs-ofctl snoop s1" Looks like adding the message to the bundle and committing the bundle is ok but something happens afterwards? Where would be a good place to look for the problem? */var/log/syslog:* Mar 3 16:46:08 ubuntu-vbox ovs-vswitchd: ovs|00093|ofproto|INFO|bundle add message - after decode, error: 0 Mar 3 16:46:08 ubuntu-vbox ovs-vswitchd: ovs|00094|ofproto|INFO|bundle add message: In if case for Packet_Out Mar 3 16:46:08 ubuntu-vbox ovs-vswitchd: ovs|00095|ofproto|INFO|bundle add message: no error in Packet_Out if case Mar 3 16:46:08 ubuntu-vbox ovs-vswitchd: ovs|00099|ofproto|INFO|bundle commit - init Mar 3 16:46:08 ubuntu-vbox ovs-vswitchd: ovs|00100|ofproto|INFO|bundle commit - phase 1 - packet out detected Mar 3 16:46:08 ubuntu-vbox ovs-vswitchd: ovs|00102|ofproto|INFO|bundle commit - phase 2 - sending packet out Mar 3 16:46:08 ubuntu-vbox ovs-vswitchd: ovs|00103|ofproto|INFO|bundle commit - phase 2 - packet out sent! Mar 3 16:46:08 ubuntu-vbox ovs-vswitchd: ovs|00104|ofproto|INFO|bundle commit - phase 2 - payload deleted Mar 3 16:46:08 ubuntu-vbox ovs-vswitchd: ovs|00108|ofproto|INFO|bundle commit - phase 2 ended Mar 3 16:46:08 ubuntu-vbox ovs-vswitchd: ovs|00109|ofproto|INFO|bundle commit - flush and mutex unlock done Mar 3 16:46:08 ubuntu-vbox ovs-vswitchd: ovs|00110|ofproto|INFO|bundle commit - run_rule_executes done (no more ovs related lines after this) *ovs-ofctl snoop s1:* OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x27): bundle_id=0xb type=OPEN_REQUEST flags=ordered OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x27): bundle_id=0xb type=OPEN_REPLY flags=0 OFPT_BUNDLE_ADD_MESSAGE (OF1.4) (xid=0x28): bundle_id=0xb flags=ordered OFPT_PACKET_OUT (OF1.4) (xid=0x28): in_port=CONTROLLER actions=CONTROLLER:65535 data_len=25 vlan_tci=0x,dl_src=00:00:00:00:00:01,dl_dst=ff:ff:ff:ff:ff:ff,dl_type=0x OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x2c): bundle_id=0xb type=CLOSE_REQUEST flags=ordered OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x2c): bundle_id=0xb type=CLOSE_REPLY flags=0 OFPT_BUNDLE_CONTROL (OF1.4) (xid=0x2d): bundle_id=0xb type=COMMIT_REQUEST flags=ordered ovs-ofctl: vconn_recv (End of file) Ben Pfaff escreveu no dia domingo, 28/02/2016 às 00:00: > On Sat, Feb 27, 2016 at 11:15:58PM +, André Mantas wrote: > > Ben Pfaff escreveu no dia sábado, 27/02/2016 às 22:30: > > > > > On Sat, Feb 27, 2016 at 06:24:23PM +, André Mantas wrote: > > > > Ok, I'll try to do that. > > > > > > > > Now related with tests for ofproto.at, if I'm not mistaken, in > version > > > 2.3 > > > > we had the "bundle" command in ovs-ofctl and do things like: > > > > > > > > ovs-ofctl bundle "open_bundle_id=,add_bundle_id= > > > > message=[,add_bundle_id= > > > > message=...],close_bundle_id=,commit_bundle_id=" > > > > > > > > Is this not supported any more? The only way to send "bundle-add" > > > messages > > > > is with with the option --bundle for add-flow, mod-flows, etc? > > > > > > OVS 2.3 didn't support bundles. ovs-ofctl never supported a "bundle" > > > command. > > > > Oh, ok. I thought it was supported from here: > > http://www.pica8.com/document/v2.3/html/ovs-commands-reference/#1081528 > > Perhaps Pica8 forked OVS for use on their switches, as is their right. > However, they have never contributed even a single patch, so I guess > they have no desire to be part of the Open vSwitch development > community. > > > > > If so, is the right approach to add a "--bundle" option for the > > > packet-out > > > > command? > > > > > > Well, I suppose you could, and perhaps we should to be thorough, but > > > packet-out only supports sending packet-outs, so it's going to have > > > limited value in practice. > > > > > > For more usefulness in practice, I guess we could add a more general > &
Re: [ovs-dev] Implementation of Packet-Out in Bundles
Ben Pfaff escreveu no dia sábado, 27/02/2016 às 22:30: > On Sat, Feb 27, 2016 at 06:24:23PM +0000, André Mantas wrote: > > Ok, I'll try to do that. > > > > Now related with tests for ofproto.at, if I'm not mistaken, in version > 2.3 > > we had the "bundle" command in ovs-ofctl and do things like: > > > > ovs-ofctl bundle "open_bundle_id=,add_bundle_id= > > message=[,add_bundle_id= > > message=...],close_bundle_id=,commit_bundle_id=" > > > > Is this not supported any more? The only way to send "bundle-add" > messages > > is with with the option --bundle for add-flow, mod-flows, etc? > > OVS 2.3 didn't support bundles. ovs-ofctl never supported a "bundle" > command. > Oh, ok. I thought it was supported from here: http://www.pica8.com/document/v2.3/html/ovs-commands-reference/#1081528 > > If so, is the right approach to add a "--bundle" option for the > packet-out > > command? > > Well, I suppose you could, and perhaps we should to be thorough, but > packet-out only supports sending packet-outs, so it's going to have > limited value in practice. > > For more usefulness in practice, I guess we could add a more general > command that accepts a variety of commands. One could think of this as > an extension of "ovs-ofctl add-flows", which already allows a variety of > OFPT_FLOW_MODs in its input, although it might make more sense to give > it a new name. > So, in which one do you think I should start working right now? ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Implementation of Packet-Out in Bundles
Ok, I'll try to do that. Now related with tests for ofproto.at, if I'm not mistaken, in version 2.3 we had the "bundle" command in ovs-ofctl and do things like: ovs-ofctl bundle "open_bundle_id=,add_bundle_id= message=[,add_bundle_id= message=...],close_bundle_id=,commit_bundle_id=" Is this not supported any more? The only way to send "bundle-add" messages is with with the option --bundle for add-flow, mod-flows, etc? If so, is the right approach to add a "--bundle" option for the packet-out command? Ben Pfaff escreveu no dia sábado, 27/02/2016 às 16:32: > One approach is to use tcpdump, etc. to capture to a pcap file. > > But I usually write the examples in ofp-print.at by hand, while reading > the OpenFlow specs, to assure myself that the formats that the decoders > understand are really correct. That's another part of the purpose of > the tests there, to ensure that if the encoder or decoder functions in > OVS drift away from the OpenFlow spec, the ofp-print tests will catch > the regressions since their contents are fixed points. > > On Sat, Feb 27, 2016 at 01:43:05PM +, André Mantas wrote: > > This may be a beginner question but is there a quick way to get a hexdump > > of an OpenFlow message? > > > > Thanks for your help. > > > > Ben Pfaff escreveu no dia sexta, 26/02/2016 às 22:52: > > > > > Tests like the ones in ofproto.at, which test functionality, are more > > > important than the ones in ofp-print.at, which test message decoding > and > > > printing, but it's best to have both. > > > > > > On Fri, Feb 26, 2016 at 09:57:48PM +, André Mantas wrote: > > > > Ok, makes sense. So I should focus on making similar tests to the > ones in > > > > ofproto.at? Or these are also important? > > > > > > > > Thanks. > > > > > > > > Ben Pfaff escreveu no dia sexta, 26/02/2016 às 20:35: > > > > > > > > > On Fri, Feb 26, 2016 at 08:07:32PM +, André Mantas wrote: > > > > > > I've seen some examples in ofp-print.at, ofproto.at and was > trying > > > to > > > > > > figure them out so I could add more. > > > > > > > > > > > > (ofp-print.at) > > > > > > AT_SETUP([OFPT_BUNDLE_ADD_MESSAGE - PORT_MOD]) > > > > > > AT_KEYWORDS([ofp-print]) > > > > > > AT_CHECK([ovs-ofctl ofp-print "\ > > > > > > 05 22 00 38 00 00 00 03 00 00 00 01 00 00 00 01 \ > > > > > > 05 10 00 28 00 00 00 03 00 00 00 03 00 00 00 00 \ > > > > > > 50 54 00 00 00 01 00 00 00 00 00 01 00 00 00 01 \ > > > > > > 00 00 00 08 00 00 00 01 > > > > > > "], [0], [dnl > > > > > > OFPT_BUNDLE_ADD_MESSAGE (OF1.4) (xid=0x3): > > > > > > bundle_id=0x1 flags=atomic > > > > > > OFPT_PORT_MOD (OF1.4) (xid=0x3): port: 3: addr:50:54:00:00:00:01 > > > > > > config: PORT_DOWN > > > > > > mask: PORT_DOWN > > > > > > advertise: 10MB-HD > > > > > > ]) > > > > > > > > > > > > From ofproto.at I can see that: > > > > > > > > > > > > 05 - OpenFlow14 message > > > > > > 22 - OFPT_BUNDLE_ADD_MESSAGE (?) > > > > > > > > > > > > I also tried to search for ovs-ofctl ofp-print syntax or help but > > > > > couldn't > > > > > > find it. > > > > > > > > > > > > Any help? > > > > > > > > > > ofp-print takes a hexdump of an OpenFlow message and prints it on > > > stdout > > > > > with ofp_print() from lib/ofp-print.c. It's there to test > decoding and > > > > > printing OpenFlow messages. > > > > > > > > > > It's not documented because it's meant for internal testing, not > as an > > > > > end user interface. > > > > > > > > > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Implementation of Packet-Out in Bundles
This may be a beginner question but is there a quick way to get a hexdump of an OpenFlow message? Thanks for your help. Ben Pfaff escreveu no dia sexta, 26/02/2016 às 22:52: > Tests like the ones in ofproto.at, which test functionality, are more > important than the ones in ofp-print.at, which test message decoding and > printing, but it's best to have both. > > On Fri, Feb 26, 2016 at 09:57:48PM +, André Mantas wrote: > > Ok, makes sense. So I should focus on making similar tests to the ones in > > ofproto.at? Or these are also important? > > > > Thanks. > > > > Ben Pfaff escreveu no dia sexta, 26/02/2016 às 20:35: > > > > > On Fri, Feb 26, 2016 at 08:07:32PM +, André Mantas wrote: > > > > I've seen some examples in ofp-print.at, ofproto.at and was trying > to > > > > figure them out so I could add more. > > > > > > > > (ofp-print.at) > > > > AT_SETUP([OFPT_BUNDLE_ADD_MESSAGE - PORT_MOD]) > > > > AT_KEYWORDS([ofp-print]) > > > > AT_CHECK([ovs-ofctl ofp-print "\ > > > > 05 22 00 38 00 00 00 03 00 00 00 01 00 00 00 01 \ > > > > 05 10 00 28 00 00 00 03 00 00 00 03 00 00 00 00 \ > > > > 50 54 00 00 00 01 00 00 00 00 00 01 00 00 00 01 \ > > > > 00 00 00 08 00 00 00 01 > > > > "], [0], [dnl > > > > OFPT_BUNDLE_ADD_MESSAGE (OF1.4) (xid=0x3): > > > > bundle_id=0x1 flags=atomic > > > > OFPT_PORT_MOD (OF1.4) (xid=0x3): port: 3: addr:50:54:00:00:00:01 > > > > config: PORT_DOWN > > > > mask: PORT_DOWN > > > > advertise: 10MB-HD > > > > ]) > > > > > > > > From ofproto.at I can see that: > > > > > > > > 05 - OpenFlow14 message > > > > 22 - OFPT_BUNDLE_ADD_MESSAGE (?) > > > > > > > > I also tried to search for ovs-ofctl ofp-print syntax or help but > > > couldn't > > > > find it. > > > > > > > > Any help? > > > > > > ofp-print takes a hexdump of an OpenFlow message and prints it on > stdout > > > with ofp_print() from lib/ofp-print.c. It's there to test decoding and > > > printing OpenFlow messages. > > > > > > It's not documented because it's meant for internal testing, not as an > > > end user interface. > > > > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Implementation of Packet-Out in Bundles
Ok, makes sense. So I should focus on making similar tests to the ones in ofproto.at? Or these are also important? Thanks. Ben Pfaff escreveu no dia sexta, 26/02/2016 às 20:35: > On Fri, Feb 26, 2016 at 08:07:32PM +0000, André Mantas wrote: > > I've seen some examples in ofp-print.at, ofproto.at and was trying to > > figure them out so I could add more. > > > > (ofp-print.at) > > AT_SETUP([OFPT_BUNDLE_ADD_MESSAGE - PORT_MOD]) > > AT_KEYWORDS([ofp-print]) > > AT_CHECK([ovs-ofctl ofp-print "\ > > 05 22 00 38 00 00 00 03 00 00 00 01 00 00 00 01 \ > > 05 10 00 28 00 00 00 03 00 00 00 03 00 00 00 00 \ > > 50 54 00 00 00 01 00 00 00 00 00 01 00 00 00 01 \ > > 00 00 00 08 00 00 00 01 > > "], [0], [dnl > > OFPT_BUNDLE_ADD_MESSAGE (OF1.4) (xid=0x3): > > bundle_id=0x1 flags=atomic > > OFPT_PORT_MOD (OF1.4) (xid=0x3): port: 3: addr:50:54:00:00:00:01 > > config: PORT_DOWN > > mask: PORT_DOWN > > advertise: 10MB-HD > > ]) > > > > From ofproto.at I can see that: > > > > 05 - OpenFlow14 message > > 22 - OFPT_BUNDLE_ADD_MESSAGE (?) > > > > I also tried to search for ovs-ofctl ofp-print syntax or help but > couldn't > > find it. > > > > Any help? > > ofp-print takes a hexdump of an OpenFlow message and prints it on stdout > with ofp_print() from lib/ofp-print.c. It's there to test decoding and > printing OpenFlow messages. > > It's not documented because it's meant for internal testing, not as an > end user interface. > ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] Implementation of Packet-Out in Bundles
I wanted to right some unit tests to start testing my code and the interactions of packet_outs with other messages in bundles. I've seen some examples in ofp-print.at, ofproto.at and was trying to figure them out so I could add more. (ofp-print.at) AT_SETUP([OFPT_BUNDLE_ADD_MESSAGE - PORT_MOD]) AT_KEYWORDS([ofp-print]) AT_CHECK([ovs-ofctl ofp-print "\ 05 22 00 38 00 00 00 03 00 00 00 01 00 00 00 01 \ 05 10 00 28 00 00 00 03 00 00 00 03 00 00 00 00 \ 50 54 00 00 00 01 00 00 00 00 00 01 00 00 00 01 \ 00 00 00 08 00 00 00 01 "], [0], [dnl OFPT_BUNDLE_ADD_MESSAGE (OF1.4) (xid=0x3): bundle_id=0x1 flags=atomic OFPT_PORT_MOD (OF1.4) (xid=0x3): port: 3: addr:50:54:00:00:00:01 config: PORT_DOWN mask: PORT_DOWN advertise: 10MB-HD ]) From ofproto.at I can see that: 05 - OpenFlow14 message 22 - OFPT_BUNDLE_ADD_MESSAGE (?) I also tried to search for ovs-ofctl ofp-print syntax or help but couldn't find it. Any help? André Mantas escreveu no dia quinta, 18/02/2016 às 20:00: > Thanks for the detailed answer. Comments below. > > Jarno Rajahalme escreveu no dia quarta, 17/02/2016 às > 17:59: > >> >> > On Feb 17, 2016, at 2:39 AM, André Mantas >> wrote: >> > >> > Hi. I'm currently adding support for packet-out messages in bundles. >> > >> > I started to look at *handle_packet_out*, *handle_bundle_add *and >> > *do_bundle_commit >> > *functions to understand how things work. Here is what I did in each >> > function: >> > >> > *handle_bundle_add:* added if case for OFPTYPE_PACKET_OUT which does all >> > the validations performed in handle_packet_out as well as saving the >> > packet_out info in the bundle_entry. For this I added a new >> >> You’d have to make sure that the validations work as expected in the >> context of the bundle. I haven’t checked the details, but if an earlier >> message in the bundle adds a port (that has not been committed yet), will >> the in_port of the packet_out using that port validate correctly? >> > > Good question, will have to look at this. It should allow a packet_out in > that "new" port. > > >> Group and meter validation is part of the action validation for >> packet_out. While the bundles can’t currently include group or meter mods, >> I’d like to see that adding packet_out to a bundle does not unnecessarily >> complicate adding group and meter mods to bundles. >> >> > If this is the case, should a new validation function be made that does > not involve validating group and meters? > > >> > ofproto_packet_out struct to the union in ofp_bundle_entry. This new >> struct >> > (created in the same way as ofproto_flow_mod and ofproto_port_mod) >> contains >> > an ofputil_packet_out, a dp_packet *payload and a flow. If I understand >> > correctly, this three structs are needed later to send the packet_out in >> > the commit phase. >> > >> > *do_bundle_commit: *added if case for OFPTYPE_PACKET_OUT in phase 3 >> > (finish) which calls the ofproto->ofproto_class->packet_out function >> using >> > the data saved in the bundle entry and then deletes de payload (as done >> in >> > handle_packet_out). >> > In phase 1 (begin) I only added prev_is_port_mod = false; to the >> packet-out >> > "if case" because all the validations were done before. >> > >> > Is there a way to verify if the packet can be sent without actually >> sending >> > it? >> >> Currently, no. The packet_out execution can fail either in action >> translation (which may call into arbitrarily complex set of flow tables via >> resubmit action), or in datapath action execution. Datapaths (such as the >> openvswitch Linux kernel module) currently perform action validation as an >> integral part of packet execution, and it may be impossible to create a >> call that would only exercise the validation part. >> >> > If not, the ofproto->ofproto_class->packet_out can still return errors, >> but >> > this will be in the finish phase, which cannot be rolled back. >> >> To make packet_out behave correctly in a bundle, the >> ofproto_class->packet_out should be split into separate validation and >> execution pieces. If you decide to do this, it may be possible to perform >> the action translation piece before the commit phase, leaving only the >> datapath action execution to the ofproto_class->packet_out. However, it may >> be costly to split the datapath action execution to separate validation and >> execution steps. Generally we do a good
Re: [ovs-dev] Implementation of Packet-Out in Bundles
Thanks for the detailed answer. Comments below. Jarno Rajahalme escreveu no dia quarta, 17/02/2016 às 17:59: > > > On Feb 17, 2016, at 2:39 AM, André Mantas > wrote: > > > > Hi. I'm currently adding support for packet-out messages in bundles. > > > > I started to look at *handle_packet_out*, *handle_bundle_add *and > > *do_bundle_commit > > *functions to understand how things work. Here is what I did in each > > function: > > > > *handle_bundle_add:* added if case for OFPTYPE_PACKET_OUT which does all > > the validations performed in handle_packet_out as well as saving the > > packet_out info in the bundle_entry. For this I added a new > > You’d have to make sure that the validations work as expected in the > context of the bundle. I haven’t checked the details, but if an earlier > message in the bundle adds a port (that has not been committed yet), will > the in_port of the packet_out using that port validate correctly? > Good question, will have to look at this. It should allow a packet_out in that "new" port. > Group and meter validation is part of the action validation for > packet_out. While the bundles can’t currently include group or meter mods, > I’d like to see that adding packet_out to a bundle does not unnecessarily > complicate adding group and meter mods to bundles. > > If this is the case, should a new validation function be made that does not involve validating group and meters? > > ofproto_packet_out struct to the union in ofp_bundle_entry. This new > struct > > (created in the same way as ofproto_flow_mod and ofproto_port_mod) > contains > > an ofputil_packet_out, a dp_packet *payload and a flow. If I understand > > correctly, this three structs are needed later to send the packet_out in > > the commit phase. > > > > *do_bundle_commit: *added if case for OFPTYPE_PACKET_OUT in phase 3 > > (finish) which calls the ofproto->ofproto_class->packet_out function > using > > the data saved in the bundle entry and then deletes de payload (as done > in > > handle_packet_out). > > In phase 1 (begin) I only added prev_is_port_mod = false; to the > packet-out > > "if case" because all the validations were done before. > > > > Is there a way to verify if the packet can be sent without actually > sending > > it? > > Currently, no. The packet_out execution can fail either in action > translation (which may call into arbitrarily complex set of flow tables via > resubmit action), or in datapath action execution. Datapaths (such as the > openvswitch Linux kernel module) currently perform action validation as an > integral part of packet execution, and it may be impossible to create a > call that would only exercise the validation part. > > > If not, the ofproto->ofproto_class->packet_out can still return errors, > but > > this will be in the finish phase, which cannot be rolled back. > > To make packet_out behave correctly in a bundle, the > ofproto_class->packet_out should be split into separate validation and > execution pieces. If you decide to do this, it may be possible to perform > the action translation piece before the commit phase, leaving only the > datapath action execution to the ofproto_class->packet_out. However, it may > be costly to split the datapath action execution to separate validation and > execution steps. Generally we do a good job not trying to execute something > that the datapath then rejects, so maybe that is not necessary. > That's good to know. If it comes out that splitting the function is a better solution, I may be able to do it. > > > I think that in this case the bundle should still be committed (because > it > > does not affect other modifications like flow-mods and port-mods) and > > errors can be returned to the controller. Would this be a good option? > > > > As per the OpenFlow bundle spec, the controller does not expect any errors > for a successful bundle commit. So this would not be a good option. > Right, I just thought it would make sense in this case, but it cannot be done. > > However, contrary to the documentation in ofproto-provider.h, our current > implementation of packet_out in ofproto-dpif.c does not pass any errors > back to the caller. If we decide to keep it that way, then all of the above > is kind of moot, as we pretend that the packet_out succeeded even when it > did not. > So, summing up, the most straightforward solution is to assume that after the first validation, a packet-out never fails being sent, right? > > Jarno > > > I would also like to know if I'm going in the right direction or if I did > > something wrong. > > > > Thanks in advance. > > ___ > > 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] Implementation of Packet-Out in Bundles
Hi. I'm currently adding support for packet-out messages in bundles. I started to look at *handle_packet_out*, *handle_bundle_add *and *do_bundle_commit *functions to understand how things work. Here is what I did in each function: *handle_bundle_add:* added if case for OFPTYPE_PACKET_OUT which does all the validations performed in handle_packet_out as well as saving the packet_out info in the bundle_entry. For this I added a new ofproto_packet_out struct to the union in ofp_bundle_entry. This new struct (created in the same way as ofproto_flow_mod and ofproto_port_mod) contains an ofputil_packet_out, a dp_packet *payload and a flow. If I understand correctly, this three structs are needed later to send the packet_out in the commit phase. *do_bundle_commit: *added if case for OFPTYPE_PACKET_OUT in phase 3 (finish) which calls the ofproto->ofproto_class->packet_out function using the data saved in the bundle entry and then deletes de payload (as done in handle_packet_out). In phase 1 (begin) I only added prev_is_port_mod = false; to the packet-out "if case" because all the validations were done before. Is there a way to verify if the packet can be sent without actually sending it? If not, the ofproto->ofproto_class->packet_out can still return errors, but this will be in the finish phase, which cannot be rolled back. I think that in this case the bundle should still be committed (because it does not affect other modifications like flow-mods and port-mods) and errors can be returned to the controller. Would this be a good option? I would also like to know if I'm going in the right direction or if I did something wrong. Thanks in advance. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] PacketOut support in OF Bundles
Hi. From the ovs design page, bundles only support FlowMod and PortMod messages, right? I would like to propose an extension to ovs to support the use of PacketOut messages in OpenFlow Bundles. I think PacketOut in bundles would be a good addition because it allows OF controllers/applications to group modifications (e.g., FlowMods) with PacketOut messages in the same bundle to guarantee order (and atomicity) without resorting to BarrierRequests (as proposed in the OpenFlow spec). Additionally, it would ease the synchronization between controllers - controller A can add a PacketOut message in the bundle to let controller B know when the bundle is committed (this is the most important use case for me that cannot be achieved in other way - from what I know). Can I contribute to ovs with this feature? Thanks in advance. ___ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
