Re: [ovs-dev] [PATCH 4/4] openflow: Support matching and modifying MPLS TTL field.

[Ben Pfaff]

On Mon, Mar 07, 2016 at 05:38:59PM -0800, Justin Pettit wrote:
> 
> > On Mar 7, 2016, at 11:18 AM, Ben Pfaff  wrote:
> > 
> > +/* Modifies 'match' so that the MPLS TTL is wildcarded. */
> > +void
> > +match_set_any_mpls_ttl(struct match *match, int idx)
> > +{
> > +match->wc.masks.mpls_lse[idx] &= ~htonl(MPLS_TTL_MASK);
> > +flow_set_mpls_ttl(>flow, idx, 0);
> > +}
> > +
> > +/* Modifies 'match' so that it matches only packets with an MPLS header 
> > whose
> > + * TTL equals 'mpls_ttl' */
> > +void
> > +match_set_mpls_ttl(struct match *match, int idx, uint8_t mpls_ttl)
> > +{
> > +match->wc.masks.mpls_lse[idx] |= htonl(MPLS_TTL_MASK);
> > +flow_set_mpls_ttl(>flow, idx, mpls_ttl);
> > +}
> 
> Do you think it's worth documenting the "idx" arguments for these two 
> functions?

Fixed, thanks.

> > +/* "mpls_ttl".
> > + *
> > + * The outermost MPLS label's time-to-level (TTL) field, or 0 if no 
> > MPLS
> 
> Shouldn't that be "time-to-live"?  Or is that how MPLS indicates that it 
> wants to have a serious talk.

Fixed, thanks.

> Acked-by: Justin Pettit 

Thanks, I'll rerun the tests and apply this to master in a minute.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 4/4] openflow: Support matching and modifying MPLS TTL field.

[Justin Pettit]

> On Mar 7, 2016, at 11:18 AM, Ben Pfaff  wrote:
> 
> +/* Modifies 'match' so that the MPLS TTL is wildcarded. */
> +void
> +match_set_any_mpls_ttl(struct match *match, int idx)
> +{
> +match->wc.masks.mpls_lse[idx] &= ~htonl(MPLS_TTL_MASK);
> +flow_set_mpls_ttl(>flow, idx, 0);
> +}
> +
> +/* Modifies 'match' so that it matches only packets with an MPLS header whose
> + * TTL equals 'mpls_ttl' */
> +void
> +match_set_mpls_ttl(struct match *match, int idx, uint8_t mpls_ttl)
> +{
> +match->wc.masks.mpls_lse[idx] |= htonl(MPLS_TTL_MASK);
> +flow_set_mpls_ttl(>flow, idx, mpls_ttl);
> +}

Do you think it's worth documenting the "idx" arguments for these two functions?

> +/* "mpls_ttl".
> + *
> + * The outermost MPLS label's time-to-level (TTL) field, or 0 if no MPLS

Shouldn't that be "time-to-live"?  Or is that how MPLS indicates that it wants 
to have a serious talk.

Acked-by: Justin Pettit 

--Justin


___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev