[ovs-discuss] Adding a new header field for flow matching and Set-Field action

Daniele Di Proietto diproiettod at vmware.com
Wed Apr 27 18:50:34 UTC 2016


Short term solution:

Yes, returning "ODP_FIT_TOO_LITTLE" from check_expectations() isn't enough to prevent a flow from being installed in the kernel datapath.  OVS userspace, since the introduction of megaflows, handles compatibility with older kernels in a different way: the kernel will refuse to install a flow with a mask that contains unknown attributes.

You can rely on that by:

* Adding an attribute for your new field in odp-netlink.h (besides, odp-netlink.h is generated from datapath/linux/compat/include/linux/openvswitch.h, so it's better to change that)
* Making sure that your attribute is only interpreted by userspace and rejected by kernel by wrapping it in #ifndef __KERNEL__
* Modifying odp_flow_key_from_flow, odp_flow_key_from_mask, odp_flow_key_to_flow* and odp_flow_key_to_mask* to handle your attribute.

When the userspace will try to install a megaflow with a mask on the new attribute the kernel will discard it.

Long term solution:

We should probably be smarter in userspace and avoid installing a megaflow with a key that returns ODP_FIT_TOO_LITTLE, or (better) avoid installing a megaflow with a mask that can't be supported by the kernel.  This is the only way to support matching on a field which doesn't have a corresponding ODP attribute.



On 27/04/2016 06:18, "Ben Pfaff" <blp at ovn.org> wrote:

>I'm not aware of bugs here, but there could easily be something new,
>since this code path isn't exercised very much lately.
>
>Daniele, did you find anything out?
>
>On Wed, Apr 27, 2016 at 10:13:58AM +0300, Enas Ahmad wrote:
>> I also found a discussion here referring to the same issue, however I am
>> not sure if it was solved or not ?
>> 
>> https://patchwork.ozlabs.org/patch/555337/
>> 
>> 
>> "....However, slowpathing ODP_FIT_TOO_LITTLE flow keys needs to be added in
>> the case where the fields not understood by the datapath are used in the
>> flow translation. Daniele promised to take a stab at it."
>> 
>> 
>> 
>> On Sun, Apr 24, 2016 at 4:07 PM, Enas Ahmad <enas.ahmad at kaust.edu.sa> wrote:
>> 
>> > According to Open vSwitch datapath developer documentation:
>> > https://www.kernel.org/doc/Documentation/networking/openvswitch.txt
>> >
>> >       "If the userspace flow key includes more fields than the
>> >       kernel's, for example if userspace decoded an IPv6 header but
>> >       the kernel stopped at the Ethernet type, then userspace can
>> >       forward the packet manually, without setting up a flow in the
>> >       kernel.  This case is bad for performance because every packet
>> >       that the kernel considers part of the flow must go to userspace,
>> >       but the forwarding behavior is correct.  (If userspace can
>> >       determine that the values of the extra fields would not affect
>> >       forwarding behavior, then it could set up a flow anyway.)"
>> >
>> > However, my new "userspace-only" field is still having flow entries created at the kernel (of course with ignoring my field) which causes inconsistent behavior during the OFPROTO_MAX_IDLE_DEFAULT time until the kernel flow entry expires and the packet is rerouted to the userspace again. I am not planning at this stage to implement my field at the kernel level, therefore I would expect, with a performance cost,  that any packet which has my field present to be always routed to the user space for flow matching, correct?
>> >
>> > I added a key entry for my field in odp-util.c and odp-netlink.h following the existing pattern, which causes the check_expectations() method to return "ODP_FIT_TOO_LITTLE" due to "expected but not present" attribute, which makes perfect sense. However, this was not taken any further and the flow entry is still cached at the kernel.
>> >
>> > Is there any additional logic I need to add ?
>> >
>> >
>> >
>> > On Tue, Mar 22, 2016 at 6:39 PM, Ben Pfaff <blp at ovn.org> wrote:
>> >
>> >> Thanks, I've sent out a patch for the FAQ.
>> >>
>> >> On Tue, Mar 22, 2016 at 11:09:54AM +0300, Enas Ahmad wrote:
>> >> > I am posting here so if someone else is trying to add a new field to ovs
>> >> > maybe this can help and save their time.
>> >> > After debugging I solved my issue by adding the following to
>> >> nx_put_raw()
>> >> > in nx-match.c:
>> >> >
>> >> > if( match->wc.masks.iot_addr){
>> >> >     nxm_put_32m(b, MFF_IOT_ADDR, oxm,
>> >> >     htonl(flow->iot_addr), match->wc.masks.iot_addr);
>> >> > }
>> >> >
>> >> > otherwise my field was not added to ofpbuf buffer. If correct maybe it
>> >> can
>> >> > be added to the FAQ.
>> >> >
>> >> > Best,
>> >> > Enas
>> >> >
>> >> > On Fri, Mar 18, 2016 at 6:50 AM, Ben Pfaff <blp at ovn.org> wrote:
>> >> >
>> >> > > I'm always frankly puzzled by this kind of message that says "I made
>> >> > > some changes to OVS and they didn't work."  The answer is that you
>> >> need
>> >> > > to debug your changes.
>> >> > >
>> >> > > On Thu, Mar 17, 2016 at 06:17:10PM +0300, Enas Ahmad wrote:
>> >> > > > Hi,
>> >> > > > I am still having a problem in successfully defining a new field. My
>> >> > > field
>> >> > > > name is iot_addr. I define it in the meta-flow.h file as the
>> >> following:
>> >> > > >
>> >> > > >   /* "iot_addr".
>> >> > > >     *
>> >> > > >     * IoT address
>> >> > > >     *
>> >> > > >     * Type: be32.
>> >> > > >     * Maskable: bitwise.
>> >> > > >     * Formatting: decimal.
>> >> > > >     * Prerequisites: UDP.
>> >> > > >     * Access: read/write.
>> >> > > >     * NXM: none.
>> >> > > >              * OXM: NXOXM_ET_IOT_ADDR(1) since OF1.5 and v2.5.
>> >> > > >          */
>> >> > > >     MFF_IOT_ADDR,
>> >> > > >
>> >> > > > When I try to create a new flow entry using ovs-ofctl:
>> >> > > >
>> >> > > > sudo ovs-ofctl add-flow s1
>> >> > > >
>> >> > >
>> >> priority=65534,dl_type=0x0800,nw_proto=17,tp_dst=9999,iot_addr=1234567890,actions=output:2
>> >> > > >
>> >> > > > the iot_addr is correctly set in the flow-mod msg.
>> >> > > >
>> >> > > > I also debugged the miniflow_extract() and any incoming packet is
>> >> being
>> >> > > > correctly parsed and the value is set in iot_addr.
>> >> > > >
>> >> > > > However, when sending packets the rule of iot_addr is not applied
>> >> (i.e.
>> >> > > all
>> >> > > > UDP packets on port 9999 are accepted regardless of the iot_addr
>> >> value).
>> >> > > >
>> >> > > > When I dump the flows on s1 it does not return the iot_addr part of
>> >> the
>> >> > > > rule (checked by debugging). So it seems that the switch didn't
>> >> accept my
>> >> > > > new field and created the table entry without it, therefore
>> >> iot_addr is
>> >> > > not
>> >> > > > being matched against when a packet arrives.
>> >> > > >
>> >> > > > I made sure that the switch is running the OpenFlow15 version using
>> >> the
>> >> > > -O
>> >> > > > command.
>> >> > > >
>> >> > > > Is there something I am missing ?
>> >> > > >
>> >> > > >
>> >> > > >
>> >> > > > On Sun, Mar 13, 2016 at 7:16 PM, Ben Pfaff <blp at ovn.org> wrote:
>> >> > > >
>> >> > > > > On Sun, Mar 13, 2016 at 05:32:45PM +0300, Enas Ahmad wrote:
>> >> > > > > > Thanks Ben for the useful answer, following these directions I
>> >> was
>> >> > > able
>> >> > > > > to
>> >> > > > > > add a new field and was able to successfully create a table
>> >> entry
>> >> > > > > > specifying a value for that filed.
>> >> > > > > >
>> >> > > > > > However, now I need to verify the packet extraction process
>> >> done in
>> >> > > > > > miniflow_extract(). My question is: how can I debug this method
>> >> ?
>> >> > > > > > flow.c does not import openvswitch/vlog.h, is there a reason for
>> >> > > that ?
>> >> > > > > > should I just add support for logging in flow.c or is there
>> >> another
>> >> > > way
>> >> > > > > to
>> >> > > > > > debug it ?
>> >> > > > >
>> >> > > > > Just add logging to flow.c
>> >> > > > >
>> >> > > >
>> >> > > > --
>> >> > > >
>> >> > > > ------------------------------
>> >> > > > This message and its contents, including attachments are intended
>> >> solely
>> >> > > > for the original recipient. If you are not the intended recipient
>> >> or have
>> >> > > > received this message in error, please notify me immediately and
>> >> delete
>> >> > > > this message from your computer system. Any unauthorized use or
>> >> > > > distribution is prohibited. Please consider the environment before
>> >> > > printing
>> >> > > > this email.
>> >> > >
>> >> >
>> >> > --
>> >> >
>> >> > ------------------------------
>> >> > This message and its contents, including attachments are intended solely
>> >> > for the original recipient. If you are not the intended recipient or
>> >> have
>> >> > received this message in error, please notify me immediately and delete
>> >> > this message from your computer system. Any unauthorized use or
>> >> > distribution is prohibited. Please consider the environment before
>> >> printing
>> >> > this email.
>> >>
>> >
>> >
>> 
>> -- 
>> 
>> ------------------------------
>> This message and its contents, including attachments are intended solely 
>> for the original recipient. If you are not the intended recipient or have 
>> received this message in error, please notify me immediately and delete 
>> this message from your computer system. Any unauthorized use or 
>> distribution is prohibited. Please consider the environment before printing 
>> this email.


More information about the discuss mailing list