| Message ID | 20180712215545.21410-2-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,1/5] netdev-dpdk: Fix incorrect byte order conversion in log message. | expand |
Thanks for the fix. I am wondering why there was no running issue when dl_type is compared with wrong byte order. Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> On Thu, Jul 12, 2018 at 2:55 PM, Ben Pfaff <blp@ovn.org> wrote: > Neither of these is a real problem. > > Signed-off-by: Ben Pfaff <blp@ovn.org> > --- > lib/netdev-dpdk.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > index b4ed4ad5919c..d485a53dacf1 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -4384,7 +4384,7 @@ netdev_dpdk_add_rte_flow_offload(struct netdev > *netdev, > struct rte_flow_item_ipv4 ipv4_mask; > memset(&ipv4_spec, 0, sizeof(ipv4_spec)); > memset(&ipv4_mask, 0, sizeof(ipv4_mask)); > - if (match->flow.dl_type == ntohs(ETH_TYPE_IP)) { > + if (match->flow.dl_type == htons(ETH_TYPE_IP)) { > > ipv4_spec.hdr.type_of_service = match->flow.nw_tos; > ipv4_spec.hdr.time_to_live = match->flow.nw_ttl; > @@ -4419,8 +4419,8 @@ netdev_dpdk_add_rte_flow_offload(struct netdev > *netdev, > goto out; > } > > - if ((match->wc.masks.tp_src && match->wc.masks.tp_src != 0xffff) || > - (match->wc.masks.tp_dst && match->wc.masks.tp_dst != 0xffff)) { > + if ((match->wc.masks.tp_src && match->wc.masks.tp_src != > OVS_BE16_MAX) || > + (match->wc.masks.tp_dst && match->wc.masks.tp_dst != > OVS_BE16_MAX)) { > ret = -1; > goto out; > } > -- > 2.16.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 7/16/2018 7:06 PM, Yifeng Sun wrote: > Thanks for the fix. I am wondering why there was no running issue when > dl_type is compared with wrong byte order. > > Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> > > On Thu, Jul 12, 2018 at 2:55 PM, Ben Pfaff <blp@ovn.org> wrote: > >> Neither of these is a real problem. >> Seems ok to me, adding Shahaf and Finn as they had worked on this so may have input. If not then I'll add this to the next pull request. Thanks Ian >> Signed-off-by: Ben Pfaff <blp@ovn.org> >> --- >> lib/netdev-dpdk.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >> index b4ed4ad5919c..d485a53dacf1 100644 >> --- a/lib/netdev-dpdk.c >> +++ b/lib/netdev-dpdk.c >> @@ -4384,7 +4384,7 @@ netdev_dpdk_add_rte_flow_offload(struct netdev >> *netdev, >> struct rte_flow_item_ipv4 ipv4_mask; >> memset(&ipv4_spec, 0, sizeof(ipv4_spec)); >> memset(&ipv4_mask, 0, sizeof(ipv4_mask)); >> - if (match->flow.dl_type == ntohs(ETH_TYPE_IP)) { >> + if (match->flow.dl_type == htons(ETH_TYPE_IP)) { >> >> ipv4_spec.hdr.type_of_service = match->flow.nw_tos; >> ipv4_spec.hdr.time_to_live = match->flow.nw_ttl; >> @@ -4419,8 +4419,8 @@ netdev_dpdk_add_rte_flow_offload(struct netdev >> *netdev, >> goto out; >> } >> >> - if ((match->wc.masks.tp_src && match->wc.masks.tp_src != 0xffff) || >> - (match->wc.masks.tp_dst && match->wc.masks.tp_dst != 0xffff)) { >> + if ((match->wc.masks.tp_src && match->wc.masks.tp_src != >> OVS_BE16_MAX) || >> + (match->wc.masks.tp_dst && match->wc.masks.tp_dst != >> OVS_BE16_MAX)) { >> ret = -1; >> goto out; >> } >> -- >> 2.16.1 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
It's because htons() and ntohs() actually do the same thing (swap bytes). On Mon, Jul 16, 2018 at 11:06:56AM -0700, Yifeng Sun wrote: > Thanks for the fix. I am wondering why there was no running issue when > dl_type is compared with wrong byte order. > > Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com> > > On Thu, Jul 12, 2018 at 2:55 PM, Ben Pfaff <blp@ovn.org> wrote: > > > Neither of these is a real problem. > > > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > --- > > lib/netdev-dpdk.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c > > index b4ed4ad5919c..d485a53dacf1 100644 > > --- a/lib/netdev-dpdk.c > > +++ b/lib/netdev-dpdk.c > > @@ -4384,7 +4384,7 @@ netdev_dpdk_add_rte_flow_offload(struct netdev > > *netdev, > > struct rte_flow_item_ipv4 ipv4_mask; > > memset(&ipv4_spec, 0, sizeof(ipv4_spec)); > > memset(&ipv4_mask, 0, sizeof(ipv4_mask)); > > - if (match->flow.dl_type == ntohs(ETH_TYPE_IP)) { > > + if (match->flow.dl_type == htons(ETH_TYPE_IP)) { > > > > ipv4_spec.hdr.type_of_service = match->flow.nw_tos; > > ipv4_spec.hdr.time_to_live = match->flow.nw_ttl; > > @@ -4419,8 +4419,8 @@ netdev_dpdk_add_rte_flow_offload(struct netdev > > *netdev, > > goto out; > > } > > > > - if ((match->wc.masks.tp_src && match->wc.masks.tp_src != 0xffff) || > > - (match->wc.masks.tp_dst && match->wc.masks.tp_dst != 0xffff)) { > > + if ((match->wc.masks.tp_src && match->wc.masks.tp_src != > > OVS_BE16_MAX) || > > + (match->wc.masks.tp_dst && match->wc.masks.tp_dst != > > OVS_BE16_MAX)) { > > ret = -1; > > goto out; > > } > > -- > > 2.16.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index b4ed4ad5919c..d485a53dacf1 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -4384,7 +4384,7 @@ netdev_dpdk_add_rte_flow_offload(struct netdev *netdev, struct rte_flow_item_ipv4 ipv4_mask; memset(&ipv4_spec, 0, sizeof(ipv4_spec)); memset(&ipv4_mask, 0, sizeof(ipv4_mask)); - if (match->flow.dl_type == ntohs(ETH_TYPE_IP)) { + if (match->flow.dl_type == htons(ETH_TYPE_IP)) { ipv4_spec.hdr.type_of_service = match->flow.nw_tos; ipv4_spec.hdr.time_to_live = match->flow.nw_ttl; @@ -4419,8 +4419,8 @@ netdev_dpdk_add_rte_flow_offload(struct netdev *netdev, goto out; } - if ((match->wc.masks.tp_src && match->wc.masks.tp_src != 0xffff) || - (match->wc.masks.tp_dst && match->wc.masks.tp_dst != 0xffff)) { + if ((match->wc.masks.tp_src && match->wc.masks.tp_src != OVS_BE16_MAX) || + (match->wc.masks.tp_dst && match->wc.masks.tp_dst != OVS_BE16_MAX)) { ret = -1; goto out; }
Neither of these is a real problem. Signed-off-by: Ben Pfaff <blp@ovn.org> --- lib/netdev-dpdk.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)