Re: [ovs-dev] [RFC v3 3/5] ovn: Add a new OVN field icmp4.frag_mtu
On Thu, Mar 21, 2019 at 01:53:39AM +0530, Numan Siddique wrote: > On Wed, Mar 20, 2019, 11:43 PM Ben Pfaff wrote: > > > On Wed, Mar 20, 2019 at 05:06:36PM +0530, Numan Siddique wrote: > > > On Tue, Feb 12, 2019 at 8:41 AM Ben Pfaff wrote: > > > > I don't understand why icmp4.frag_mtu is so heavily special-cased that > > > > it only works inside nested actions. > > > > > > The idea of this patch is to add new OVN fields which can't be translated > > > to OVS fields. > > > We cannot use icmp4.frag_mtu as a normal action. > > > > > > Eg. > > > > > > match=..., action=(eth.src = 30:54:00:00:00:03; icmp4.code = 3; > > > icmp4.frag_mtu = 1442; ...) > > > > > > We can use these fields only with in OVN actions which gets translated to > > > "controller" OVS action. > > > When ovn-controller gets the packet-in, it can set these fields. > > > > The assignment itself could be translated to a controller OVS action. > > > > Now I understand. Thanks. I will work on it. Thanks. For what it's worth, when it's possible and efficient enough, I like to have things be general-purpose: then you enable people with new ideas to use what you've built in clever new ways, and you also don't have to explain restrictions to people, especially when the restrictions are ones that only make sense in the solution domain rather than the problem domain. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC v3 3/5] ovn: Add a new OVN field icmp4.frag_mtu
On Wed, Mar 20, 2019, 11:43 PM Ben Pfaff wrote: > On Wed, Mar 20, 2019 at 05:06:36PM +0530, Numan Siddique wrote: > > On Tue, Feb 12, 2019 at 8:41 AM Ben Pfaff wrote: > > > I don't understand why icmp4.frag_mtu is so heavily special-cased that > > > it only works inside nested actions. > > > > The idea of this patch is to add new OVN fields which can't be translated > > to OVS fields. > > We cannot use icmp4.frag_mtu as a normal action. > > > > Eg. > > > > match=..., action=(eth.src = 30:54:00:00:00:03; icmp4.code = 3; > > icmp4.frag_mtu = 1442; ...) > > > > We can use these fields only with in OVN actions which gets translated to > > "controller" OVS action. > > When ovn-controller gets the packet-in, it can set these fields. > > The assignment itself could be translated to a controller OVS action. > Now I understand. Thanks. I will work on it. Numan ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC v3 3/5] ovn: Add a new OVN field icmp4.frag_mtu
On Wed, Mar 20, 2019 at 05:06:36PM +0530, Numan Siddique wrote: > On Tue, Feb 12, 2019 at 8:41 AM Ben Pfaff wrote: > > I don't understand why icmp4.frag_mtu is so heavily special-cased that > > it only works inside nested actions. > > The idea of this patch is to add new OVN fields which can't be translated > to OVS fields. > We cannot use icmp4.frag_mtu as a normal action. > > Eg. > > match=..., action=(eth.src = 30:54:00:00:00:03; icmp4.code = 3; > icmp4.frag_mtu = 1442; ...) > > We can use these fields only with in OVN actions which gets translated to > "controller" OVS action. > When ovn-controller gets the packet-in, it can set these fields. The assignment itself could be translated to a controller OVS action. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC v3 3/5] ovn: Add a new OVN field icmp4.frag_mtu
On Tue, Feb 12, 2019 at 8:41 AM Ben Pfaff wrote:
> On Thu, Jan 10, 2019 at 11:30:58PM +0530, nusid...@redhat.com wrote:
> > From: Numan Siddique
> >
> > In order to support OVN specific fields (which are not yet
> > supported in OpenvSwitch to set or modify values) a generic
> > OVN field support is added in this patch. These OVN fields are
> > expected to be used as nested OVN actions inside OVN actions
> > like icmp4, icmp6 etc. This patch adds only one field for now
> > - icmp4.frag_mtu. It should be fairly straightforward to
> > add similar fields in the near future.
> >
> > This field is expected to be used as an inner field with in
> > the 'icmp4' action.
> >
> > Eg.
> > "icmp4 {"eth.dst <-> eth.src; "
> > "icmp4.type = 3; /* Destination Unreachable */ "
> > "icmp4.code = 4; /* Fragmentation Needed */ "
> > icmp4.frag_mtu = 1442;
> > ...
> > "next; };",
> >
> > pinctrl module of ovn-controller will set the specified value
> > in the the low-order 16 bits of the ICMP4 header field that is
> > labelled "unused" in the ICMP specification as defined in the RFC 1191.
> >
> > Upcoming patch will use it to send an icmp4 packet if the
> > source IPv4 packet destined to go via external gateway needs to
> > be fragmented.
> >
> > Signed-off-by: Numan Siddique
>
> Thanks!
>
>
I am replying very late. My apologies.
> This ntohs in encode_OVNFIELD_LOAD should be htons:
> oah->len = ntohs(sizeof(ovs_be16));
>
> In encode_nested_actions(), it's unsafe to use the pointer
> n_ovnfields_acts after calling ovnacts_encode(), because the buffer
> might have been reallocated.
>
>
I have addressed these and submitted v4 -
https://patchwork.ozlabs.org/project/openvswitch/list/?series=98190
> I don't understand why icmp4.frag_mtu is so heavily special-cased that
> it only works inside nested actions.
>
The idea of this patch is to add new OVN fields which can't be translated
to OVS fields.
We cannot use icmp4.frag_mtu as a normal action.
Eg.
match=..., action=(eth.src = 30:54:00:00:00:03; icmp4.code = 3;
icmp4.frag_mtu = 1442; ...)
We can use these fields only with in OVN actions which gets translated to
"controller" OVS action.
When ovn-controller gets the packet-in, it can set these fields.
Thanks
Numan
> Thanks,
>
> Ben.
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [RFC v3 3/5] ovn: Add a new OVN field icmp4.frag_mtu
On Thu, Jan 10, 2019 at 11:30:58PM +0530, nusid...@redhat.com wrote:
> From: Numan Siddique
>
> In order to support OVN specific fields (which are not yet
> supported in OpenvSwitch to set or modify values) a generic
> OVN field support is added in this patch. These OVN fields are
> expected to be used as nested OVN actions inside OVN actions
> like icmp4, icmp6 etc. This patch adds only one field for now
> - icmp4.frag_mtu. It should be fairly straightforward to
> add similar fields in the near future.
>
> This field is expected to be used as an inner field with in
> the 'icmp4' action.
>
> Eg.
> "icmp4 {"eth.dst <-> eth.src; "
> "icmp4.type = 3; /* Destination Unreachable */ "
> "icmp4.code = 4; /* Fragmentation Needed */ "
> icmp4.frag_mtu = 1442;
> ...
> "next; };",
>
> pinctrl module of ovn-controller will set the specified value
> in the the low-order 16 bits of the ICMP4 header field that is
> labelled "unused" in the ICMP specification as defined in the RFC 1191.
>
> Upcoming patch will use it to send an icmp4 packet if the
> source IPv4 packet destined to go via external gateway needs to
> be fragmented.
>
> Signed-off-by: Numan Siddique
Thanks!
This ntohs in encode_OVNFIELD_LOAD should be htons:
oah->len = ntohs(sizeof(ovs_be16));
In encode_nested_actions(), it's unsafe to use the pointer
n_ovnfields_acts after calling ovnacts_encode(), because the buffer
might have been reallocated.
I don't understand why icmp4.frag_mtu is so heavily special-cased that
it only works inside nested actions.
Thanks,
Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
