Re: [dpdk-dev] [PATCH 00/15] prefix network structures

[David Marchand <david.marchand@redhat.com>]

On Wed, May 29, 2019 at 6:50 PM Olivier Matz <olivier.matz@6wind.com> wrote:

> On Wed, May 29, 2019 at 09:59:11AM +0200, David Marchand wrote:
> > On Fri, May 24, 2019 at 1:38 PM Ferruh Yigit <ferruh.yigit@intel.com>
> wrote:
> >
> > > On 5/21/2019 5:13 PM, Olivier Matz wrote:
> > > > The rte_net headers conflict with the libc headers, because
> > > > some definitions are duplicated, sometimes with few differences.
> > > >
> > > > This patchset adds the rte_ (or RTE_) prefix to all structures,
> functions
> > > > and defines in rte_net library. This is a big changeset, that will
> > > > break the API of many functions, but not the ABI.
> > > >
> > > > This was discussed in [1], and requested by the techboard [2].
> > > >
> > > > patch-v1:
> > > > * rease on top of v19.05
> > > > * remove uneeded renames in drivers/net/bonding/rte_eth_bond_pmd.c
> > > >   and app/test-pmd/icmpecho.c (arp-related variables)
> > > > * do not modify base drivers, except cxgbe, thunderx, enetc, qede:
> > > >   only files that already contain dpdk definitions are modified.
> > > > * fix checkpatch issues when it does not impact the code style too
> > > >   much. Big warnings remain about the RTE_IPv4() macros because
> > > >   arguments are separated by ',' instead of ', '.
> > > > * add a release note patch
> > > > * tested compilation on x86_64-linux and x86_64-freebsd.
> > > >
> > > > rfc-v2:
> > > > * rebase on top of v19.05-rc1
> > > >
> > > >
> > > > [1] http://mails.dpdk.org/archives/dev/2018-January/087384.html
> > > > [2] http://mails.dpdk.org/archives/dev/2019-February/125033.html
> > > >
> > > >
> > > > Olivier Matz (15):
> > > >   net: add rte prefix to arp structures
> > > >   net: add rte prefix to arp defines
> > > >   net: add rte prefix to ether structures
> > > >   net: add rte prefix to ether functions
> > > >   net: add rte prefix to ether defines
> > > >   net: add rte prefix to esp structure
> > > >   net: add rte prefix to gre structure
> > > >   net: add rte prefix to icmp structure
> > > >   net: add rte prefix to icmp defines
> > > >   net: add rte prefix to ip structure
> > > >   net: add rte prefix to ip defines
> > > >   net: add rte prefix to sctp structure
> > > >   net: add rte prefix to tcp structure
> > > >   net: add rte prefix to udp structure
> > > >   doc: announce network api change
> > >
> > > Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
> > > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> > >
> > > For series,
> > > Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> > >
> > > Series applied to dpdk-next-net/master, thanks.
> > >
> >
> > Sorry, late to the party...
> >
> > Since we change those defines, we might as well avoid these warnings:
> > CHECK:CAMELCASE: Avoid CamelCase: <RTE_ETHER_TYPE_IPv4>
> > CHECK:CAMELCASE: Avoid CamelCase: <RTE_ETHER_TYPE_IPv6>
> >
> > I can send a patch.
>
> Thanks.
>
> By the way, here is below a script that can be used to ease the
> porting of an application. It includes modifications proposed by
> David [1].
>
> It is probably not 100% reliable, but it will do most of the job.
>
>
> Olivier
>
> [1] https://mails.dpdk.org/archives/dev/2019-May/133060.html
>
> -----------------------
> #!/bin/bash
>
> # Prefix network structure/defines/functions with "rte_".
>
> set -e
>
> if [ $# = 0 ]; then
>         echo 'usage: $0 <files>'
>         exit 1
> fi
>
> files="$@"
>
> # $1: pattern
> # $2: replacement
> replace()
> {
>         local pattern="$1"
>         local replace="$2"
>
>         sed -i -e "s,${pattern},${replace},g" ${files}
> }
>
> # $*: function names
> replace_function()
> {
>         for i in $*; do
>                 replace "\<$i\>(" "rte_$i("
>         done
> }
>
> # $*: define names
> replace_define()
> {
>         for i in $*; do
>                 local pattern
>                 case "$i" in
>                         *'(')
>                                 pattern="\<${i%(}\>("
>                                 ;;
>                         *)
>                                 pattern="\<${i}\>"
>                                 ;;
>                 esac
>                 replace "$pattern" "RTE_${i^^}"
>         done
> }
>
> # $*: struct names
> replace_struct()
> {
>         for i in $*; do
>                 replace "struct\([      ][      ]*\)\<$i\>"
> "struct\1rte_$i"
>         done
> }
>
> replace arp_hrd arp_hardware
> replace arp_pro arp_protocol
> replace arp_hln arp_hlen
> replace arp_pln arp_plen
> replace arp_op arp_opcode
> replace_struct arp_hdr arp_ipv4
> replace_define ARP_HRD_ETHER ARP_OP_REQUEST ARP_OP_REPLY ARP_OP_REVREQUEST
> \
>                ARP_OP_REVREPLY ARP_OP_INVREQUEST ARP_OP_INVREPLY
> replace_struct ether_addr ether_hdr vlan_hdr vxlan_hdr vxlan_gpe_hdr
> replace_function is_same_ether_addr is_zero_ether_addr
> is_unicast_ether_addr \
>                  is_multicast_ether_addr is_broadcast_ether_addr \
>                  is_universal_ether_addr is_local_admin_ether_addr \
>                  is_valid_assigned_ether_addr eth_random_addr
> ether_addr_copy \
>                  ether_format_addr
> replace_define ETHER_ADDR_LEN ETHER_TYPE_LEN ETHER_CRC_LEN ETHER_HDR_LEN \
>                ETHER_MIN_LEN ETHER_MAX_LEN ETHER_MTU
> ETHER_MAX_VLAN_FRAME_LEN \
>                ETHER_MAX_VLAN_ID ETHER_MAX_JUMBO_FRAME_LEN ETHER_MIN_MTU \
>                ETHER_LOCAL_ADMIN_ADDR ETHER_GROUP_ADDR ETHER_TYPE_IPv4 \
>                ETHER_TYPE_IPv6 ETHER_TYPE_ARP ETHER_TYPE_VLAN
> ETHER_TYPE_RARP \
>                ETHER_TYPE_QINQ ETHER_TYPE_ETAG ETHER_TYPE_1588
> ETHER_TYPE_SLOW \
>                ETHER_TYPE_TEB ETHER_TYPE_LLDP ETHER_TYPE_MPLS
> ETHER_TYPE_MPLSM \
>                ETHER_VXLAN_HLEN ETHER_ADDR_FMT_SIZE VXLAN_GPE_TYPE_IPV4 \
>                VXLAN_GPE_TYPE_IPV6 VXLAN_GPE_TYPE_ETH VXLAN_GPE_TYPE_NSH \
>                VXLAN_GPE_TYPE_MPLS VXLAN_GPE_TYPE_GBP VXLAN_GPE_TYPE_VBNG \
>                ETHER_VXLAN_GPE_HLEN
> replace_struct esp_hdr
> replace_struct gre_hdr
> replace_struct icmp_hdr
> replace_define IP_ICMP_ECHO_REPLY IP_ICMP_ECHO_REQUEST
> replace_struct ipv4_hdr ipv6_hdr
> replace_define "IPv4(" IPV4_MAX_PKT_LEN IPV4_HDR_IHL_MASK
> IPV4_IHL_MULTIPLIER \
>                IPV4_HDR_DF_SHIFT IPV4_HDR_MF_SHIFT IPV4_HDR_FO_SHIFT \
>                IPV4_HDR_DF_FLAG IPV4_HDR_MF_FLAG IPV4_HDR_OFFSET_MASK \
>                IPV4_HDR_OFFSET_UNITS IPV4_ANY IPV4_LOOPBACK IPV4_BROADCAST
> \
>                IPV4_ALLHOSTS_GROUP IPV4_ALLRTRS_GROUP IPV4_MAX_LOCAL_GROUP
> \
>                IPV4_MIN_MCAST IPV4_MAX_MCAST IS_IPV4_MCAST
> IPV6_HDR_FL_SHIFT \
>                IPV6_HDR_TC_SHIFT IPV6_HDR_FL_MASK IPV6_HDR_TC_MASK
> replace_struct sctp_hdr
> replace_struct tcp_hdr
> replace_struct udp_hdr
>

Thanks Olivier.

I tested this script against OVS dpdk-latest branch and did not catch any
false positive, so I will submit the changes to OVS ml once my latest
changes are in.


-- 
David Marchand