On Tue, May 8, 2012 at 6:27 PM, Pravin B Shelar <pshe...@nicira.com> wrote:
> Use DSCP bits from ToS set on tunnel.
>
> Signed-off-by: Pravin B Shelar <pshe...@nicira.com>
>
> Bug #8822
I received a white space warning when applying this:
Applying: datapath: Fix Tunnel options ToS
/home/jesse/openvswitch/.git/rebase-apply/patch:15: space before tab in indent.
* router expect RT_TOS bits only. */
warning: 1 line adds whitespace errors.
> diff --git a/datapath/tunnel.c b/datapath/tunnel.c
> index c2133bb..74d99d9 100644
> --- a/datapath/tunnel.c
> +++ b/datapath/tunnel.c
> @@ -1032,7 +1035,6 @@ static struct rtable *find_route(struct vport *vport,
> struct tnl_cache *cur_cache = rcu_dereference(tnl_vport->cache);
>
> *cache = NULL;
> - tos = RT_TOS(tos);
>
> if (likely(tos == mutable->tos &&
> check_cache_valid(cur_cache, mutable))) {
I think we want to push down the RT_TOS call in a different way. The
goal of the checks for tos == mutable->tos is to ensure that any
inherited value can't affect the routing table lookup. However, since
not all bits can have an effect we probably just want to do
RT_TOS(mutable->tos) for the comparison.
One other thing that I noticed is that we don't really want to do
lookups using ECN bits even thought there is overlap. I think we can
avoid this by moving the ECN encapsulation after the routing table
lookup.
> @@ -1402,7 +1403,8 @@ static int tnl_set_config(struct net *net, struct
> nlattr *options,
>
> if (a[OVS_TUNNEL_ATTR_TOS]) {
> mutable->tos = nla_get_u8(a[OVS_TUNNEL_ATTR_TOS]);
> - if (mutable->tos != RT_TOS(mutable->tos))
> + /* Check if ECN bits are zero. */
> + if (!INET_ECN_is_not_ect(mutable->tos))
This might be easier to read as just mutable->tos & INET_ECN_MASK to
avoid the double negative.
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index b9b7b97..25d5fce 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -1258,7 +1258,8 @@
>
> <column name="options" key="tos">
> Optional. The value of the ToS bits to be set on the encapsulating
> - packet. It may also be the word <code>inherit</code>, in which case
> + packet. ToS is interpreted as DSCP and ECN bits, ECN part must be
> + zero. It may also be the word \fBinherit\fR, in which case
This isn't a man page, so I think it should continue to use the XML
tags for inherit.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
