mbox series

[RFC,00/14] prefix network structures

Message ID 20181024081833.21432-1-olivier.matz@6wind.com (mailing list archive)
Headers
Series prefix network structures |

Message

Olivier Matz Oct. 24, 2018, 8:18 a.m. UTC
  This RFC targets 19.02.

The rte_net headers conflict with the libc headers, because
some definitions are duplicated, sometimes with few differences.
This was discussed in [1], and more recently at the techboard.

Before sending the deprecation notice (target for this is 18.11),
here is a draft that can be discussed.

This RFC 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.

One question I'm asking is how can we manage the transition.
Initially, I hoped it was possible to have a compat layer during
one release (supporting both prefixed and unprefixed names), but
now that the patch is done, it seems the impact is too big, and
impacts too many libraries.

Few examples:
  - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *)
  - many rte_flow structures use the rte_ prefixed net structures
  - the mac field of virtio_net structure is rte_ether_addr
  - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *)
  ...

Therefore, it is clear that doing this would break the compilation
of many external applications.

Another drawback we need to take in account: it will make the
backport of patches more difficult, although this is something
that could be tempered by a script.

While it is obviously better to have a good namespace convention, 
we need to identify the issues we have today before deciding it's
worth doing the change.

Comments?


Things that are missing in RFC:
- test with FreeBSD
- manually fix some indentation issues


Olivier Matz (14):
  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

 app/pdump/main.c                                   |   2 +-
 app/test-eventdev/test_perf_common.c               |   2 +-
 app/test-eventdev/test_pipeline_common.c           |   2 +-
 app/test-pmd/cmdline.c                             |  66 ++---
 app/test-pmd/cmdline_flow.c                        |  10 +-
 app/test-pmd/config.c                              |  34 +--
 app/test-pmd/csumonly.c                            | 156 +++++-----
 app/test-pmd/flowgen.c                             |  30 +-
 app/test-pmd/icmpecho.c                            | 120 ++++----
 app/test-pmd/ieee1588fwd.c                         |  18 +-
 app/test-pmd/macfwd.c                              |  12 +-
 app/test-pmd/macswap.c                             |  16 +-
 app/test-pmd/parameters.c                          |   6 +-
 app/test-pmd/testpmd.c                             |  22 +-
 app/test-pmd/testpmd.h                             |  18 +-
 app/test-pmd/txonly.c                              |  36 +--
 app/test-pmd/util.c                                |  34 +--
 doc/guides/prog_guide/bbdev.rst                    |   6 +-
 .../prog_guide/packet_classif_access_ctrl.rst      |  18 +-
 doc/guides/prog_guide/rte_flow.rst                 |   4 +-
 doc/guides/sample_app_ug/flow_classify.rst         |  28 +-
 doc/guides/sample_app_ug/flow_filtering.rst        |   6 +-
 doc/guides/sample_app_ug/ip_frag.rst               |  16 +-
 doc/guides/sample_app_ug/ip_reassembly.rst         |  16 +-
 doc/guides/sample_app_ug/ipv4_multicast.rst        |  16 +-
 doc/guides/sample_app_ug/l2_forward_job_stats.rst  |   6 +-
 .../sample_app_ug/l2_forward_real_virtual.rst      |   6 +-
 doc/guides/sample_app_ug/l3_forward.rst            |  12 +-
 doc/guides/sample_app_ug/link_status_intr.rst      |   6 +-
 doc/guides/sample_app_ug/ptpclient.rst             |   6 +-
 doc/guides/sample_app_ug/rxtx_callbacks.rst        |   2 +-
 doc/guides/sample_app_ug/server_node_efd.rst       |  12 +-
 doc/guides/sample_app_ug/skeleton.rst              |   4 +-
 doc/guides/sample_app_ug/vmdq_dcb_forwarding.rst   |   4 +-
 drivers/bus/dpaa/base/fman/fman.c                  |   2 +-
 drivers/bus/dpaa/base/fman/fman_hw.c               |   2 +-
 drivers/bus/dpaa/include/fman.h                    |   2 +-
 drivers/bus/dpaa/include/netcfg.h                  |   4 +-
 drivers/net/af_packet/rte_eth_af_packet.c          |   2 +-
 drivers/net/ark/ark_ethdev.c                       |  16 +-
 drivers/net/ark/ark_ext.h                          |   4 +-
 drivers/net/ark/ark_global.h                       |   4 +-
 drivers/net/atlantic/atl_ethdev.c                  |  20 +-
 drivers/net/atlantic/hw_atl/hw_atl_utils.c         |   8 +-
 drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c    |   4 +-
 drivers/net/avf/avf.h                              |   4 +-
 drivers/net/avf/avf_ethdev.c                       |  50 ++--
 drivers/net/avf/avf_rxtx.c                         |  14 +-
 drivers/net/avf/avf_vchnl.c                        |   8 +-
 drivers/net/avf/base/avf_adminq_cmd.h              |   4 +-
 drivers/net/avf/base/avf_common.c                  |  12 +-
 drivers/net/avf/base/avf_prototype.h               |   4 +-
 drivers/net/avp/avp_ethdev.c                       |  20 +-
 drivers/net/avp/rte_avp_common.h                   |   2 +-
 drivers/net/axgbe/axgbe_dev.c                      |   4 +-
 drivers/net/axgbe/axgbe_ethdev.c                   |  10 +-
 drivers/net/axgbe/axgbe_ethdev.h                   |   4 +-
 drivers/net/axgbe/axgbe_rxtx.c                     |   2 +-
 drivers/net/bnx2x/bnx2x.c                          |  16 +-
 drivers/net/bnx2x/bnx2x_ethdev.c                   |   4 +-
 drivers/net/bnx2x/bnx2x_ethdev.h                   |   2 +-
 drivers/net/bnx2x/bnx2x_vfpf.c                     |   8 +-
 drivers/net/bnx2x/bnx2x_vfpf.h                     |   2 +-
 drivers/net/bnx2x/ecore_sp.h                       |   2 +-
 drivers/net/bnxt/bnxt.h                            |   4 +-
 drivers/net/bnxt/bnxt_ethdev.c                     |  70 ++---
 drivers/net/bnxt/bnxt_filter.c                     |   4 +-
 drivers/net/bnxt/bnxt_filter.h                     |   8 +-
 drivers/net/bnxt/bnxt_flow.c                       |  26 +-
 drivers/net/bnxt/bnxt_hwrm.c                       |  40 +--
 drivers/net/bnxt/bnxt_hwrm.h                       |   2 +-
 drivers/net/bnxt/bnxt_ring.c                       |   8 +-
 drivers/net/bnxt/bnxt_rxq.c                        |   2 +-
 drivers/net/bnxt/bnxt_rxr.c                        |   2 +-
 drivers/net/bnxt/bnxt_vnic.c                       |   2 +-
 drivers/net/bnxt/rte_pmd_bnxt.c                    |  14 +-
 drivers/net/bnxt/rte_pmd_bnxt.h                    |   4 +-
 drivers/net/bonding/rte_eth_bond.h                 |   2 +-
 drivers/net/bonding/rte_eth_bond_8023ad.c          |  26 +-
 drivers/net/bonding/rte_eth_bond_8023ad.h          |  10 +-
 drivers/net/bonding/rte_eth_bond_alb.c             |  78 ++---
 drivers/net/bonding/rte_eth_bond_alb.h             |  10 +-
 drivers/net/bonding/rte_eth_bond_api.c             |   2 +-
 drivers/net/bonding/rte_eth_bond_args.c            |   2 +-
 drivers/net/bonding/rte_eth_bond_pmd.c             | 194 ++++++-------
 drivers/net/bonding/rte_eth_bond_private.h         |   6 +-
 drivers/net/cxgbe/base/adapter.h                   |   6 +-
 drivers/net/cxgbe/base/t4_hw.c                     |   8 +-
 drivers/net/cxgbe/cxgbe.h                          |   4 +-
 drivers/net/cxgbe/cxgbe_ethdev.c                   |  14 +-
 drivers/net/cxgbe/cxgbe_filter.h                   |   2 +-
 drivers/net/cxgbe/cxgbe_flow.c                     |  10 +-
 drivers/net/cxgbe/cxgbe_main.c                     |   4 +-
 drivers/net/cxgbe/cxgbe_pfvf.h                     |   2 +-
 drivers/net/cxgbe/cxgbevf_main.c                   |   2 +-
 drivers/net/cxgbe/l2t.c                            |   8 +-
 drivers/net/cxgbe/l2t.h                            |   2 +-
 drivers/net/cxgbe/mps_tcam.c                       |  14 +-
 drivers/net/cxgbe/mps_tcam.h                       |   4 +-
 drivers/net/cxgbe/sge.c                            |   8 +-
 drivers/net/dpaa/dpaa_ethdev.c                     |  20 +-
 drivers/net/dpaa/dpaa_rxtx.c                       |  22 +-
 drivers/net/dpaa2/dpaa2_ethdev.c                   |  36 +--
 drivers/net/e1000/e1000_ethdev.h                   |   2 +-
 drivers/net/e1000/em_ethdev.c                      |  34 +--
 drivers/net/e1000/em_rxtx.c                        |  22 +-
 drivers/net/e1000/igb_ethdev.c                     |  70 ++---
 drivers/net/e1000/igb_flow.c                       |  12 +-
 drivers/net/e1000/igb_pf.c                         |  16 +-
 drivers/net/e1000/igb_rxtx.c                       |  18 +-
 drivers/net/ena/ena_ethdev.c                       |  16 +-
 drivers/net/ena/ena_ethdev.h                       |   2 +-
 drivers/net/enetc/base/enetc_hw.h                  |   4 +-
 drivers/net/enetc/enetc_ethdev.c                   |   6 +-
 drivers/net/enic/enic.h                            |   2 +-
 drivers/net/enic/enic_clsf.c                       |  40 +--
 drivers/net/enic/enic_ethdev.c                     |   4 +-
 drivers/net/enic/enic_flow.c                       | 100 +++----
 drivers/net/enic/enic_main.c                       |   2 +-
 drivers/net/enic/enic_res.c                        |   4 +-
 drivers/net/failsafe/failsafe.c                    |   6 +-
 drivers/net/failsafe/failsafe_args.c               |   4 +-
 drivers/net/failsafe/failsafe_ether.c              |   6 +-
 drivers/net/failsafe/failsafe_ops.c                |   6 +-
 drivers/net/failsafe/failsafe_private.h            |   4 +-
 drivers/net/fm10k/fm10k.h                          |   2 +-
 drivers/net/fm10k/fm10k_ethdev.c                   |  18 +-
 drivers/net/i40e/base/i40e_adminq_cmd.h            |   4 +-
 drivers/net/i40e/base/i40e_common.c                |  12 +-
 drivers/net/i40e/base/i40e_prototype.h             |   4 +-
 drivers/net/i40e/i40e_ethdev.c                     | 134 ++++-----
 drivers/net/i40e/i40e_ethdev.h                     |  22 +-
 drivers/net/i40e/i40e_ethdev_vf.c                  |  60 ++--
 drivers/net/i40e/i40e_fdir.c                       | 126 ++++----
 drivers/net/i40e/i40e_flow.c                       |  58 ++--
 drivers/net/i40e/i40e_pf.c                         |  18 +-
 drivers/net/i40e/i40e_rxtx.c                       |  28 +-
 drivers/net/i40e/i40e_vf_representor.c             |   2 +-
 drivers/net/i40e/rte_pmd_i40e.c                    |  30 +-
 drivers/net/i40e/rte_pmd_i40e.h                    |   8 +-
 drivers/net/ixgbe/ixgbe_ethdev.c                   |  94 +++---
 drivers/net/ixgbe/ixgbe_ethdev.h                   |   2 +-
 drivers/net/ixgbe/ixgbe_flow.c                     |  22 +-
 drivers/net/ixgbe/ixgbe_pf.c                       |  14 +-
 drivers/net/ixgbe/ixgbe_rxtx.c                     |  14 +-
 drivers/net/ixgbe/ixgbe_vf_representor.c           |   4 +-
 drivers/net/ixgbe/rte_pmd_ixgbe.c                  |  10 +-
 drivers/net/ixgbe/rte_pmd_ixgbe.h                  |   2 +-
 drivers/net/kni/rte_eth_kni.c                      |   4 +-
 drivers/net/liquidio/lio_ethdev.c                  |  22 +-
 drivers/net/mlx4/mlx4.c                            |   4 +-
 drivers/net/mlx4/mlx4.h                            |   8 +-
 drivers/net/mlx4/mlx4_ethdev.c                     |   8 +-
 drivers/net/mlx4/mlx4_flow.c                       |  14 +-
 drivers/net/mlx4/mlx4_rxtx.c                       |   2 +-
 drivers/net/mlx5/mlx5.c                            |   4 +-
 drivers/net/mlx5/mlx5.h                            |  14 +-
 drivers/net/mlx5/mlx5_flow.c                       |  22 +-
 drivers/net/mlx5/mlx5_flow_tcf.c                   |  40 +--
 drivers/net/mlx5/mlx5_flow_verbs.c                 |  26 +-
 drivers/net/mlx5/mlx5_mac.c                        |  18 +-
 drivers/net/mlx5/mlx5_nl.c                         |  28 +-
 drivers/net/mlx5/mlx5_rxtx.c                       |   6 +-
 drivers/net/mlx5/mlx5_rxtx.h                       |   2 +-
 drivers/net/mlx5/mlx5_rxtx_vec_neon.h              |   8 +-
 drivers/net/mlx5/mlx5_rxtx_vec_sse.h               |  10 +-
 drivers/net/mlx5/mlx5_trigger.c                    |   6 +-
 drivers/net/mvneta/mvneta_ethdev.c                 |  22 +-
 drivers/net/mvneta/mvneta_ethdev.h                 |   2 +-
 drivers/net/mvpp2/mrvl_ethdev.c                    |  22 +-
 drivers/net/mvpp2/mrvl_ethdev.h                    |   2 +-
 drivers/net/mvpp2/mrvl_flow.c                      |   4 +-
 drivers/net/netvsc/hn_ethdev.c                     |   4 +-
 drivers/net/netvsc/hn_nvs.c                        |   2 +-
 drivers/net/netvsc/hn_rndis.c                      |   2 +-
 drivers/net/netvsc/hn_rxtx.c                       |  12 +-
 drivers/net/netvsc/hn_var.h                        |   4 +-
 drivers/net/netvsc/hn_vf.c                         |  12 +-
 drivers/net/nfp/nfp_net.c                          |  20 +-
 drivers/net/nfp/nfp_net_pmd.h                      |   2 +-
 drivers/net/null/rte_eth_null.c                    |   6 +-
 drivers/net/octeontx/octeontx_ethdev.c             |   8 +-
 drivers/net/octeontx/octeontx_ethdev.h             |   2 +-
 drivers/net/pcap/rte_eth_pcap.c                    |  22 +-
 drivers/net/qede/base/bcm_osal.h                   |   2 +-
 drivers/net/qede/base/ecore_dev.c                  |   4 +-
 drivers/net/qede/qede_ethdev.c                     |  58 ++--
 drivers/net/qede/qede_ethdev.h                     |   6 +-
 drivers/net/qede/qede_filter.c                     |  66 ++---
 drivers/net/qede/qede_if.h                         |   4 +-
 drivers/net/qede/qede_main.c                       |   6 +-
 drivers/net/qede/qede_rxtx.c                       |  32 +-
 drivers/net/qede/qede_rxtx.h                       |   2 +-
 drivers/net/ring/rte_eth_ring.c                    |   4 +-
 drivers/net/sfc/sfc.h                              |   2 +-
 drivers/net/sfc/sfc_ef10_tx.c                      |   8 +-
 drivers/net/sfc/sfc_ethdev.c                       |  20 +-
 drivers/net/sfc/sfc_flow.c                         |  12 +-
 drivers/net/sfc/sfc_port.c                         |   8 +-
 drivers/net/sfc/sfc_tso.c                          |   8 +-
 drivers/net/softnic/parser.c                       |  18 +-
 drivers/net/softnic/parser.h                       |   2 +-
 drivers/net/softnic/rte_eth_softnic.c              |   2 +-
 drivers/net/softnic/rte_eth_softnic_pipeline.c     |  40 +--
 drivers/net/szedata2/rte_eth_szedata2.c            |   8 +-
 drivers/net/tap/rte_eth_tap.c                      |  58 ++--
 drivers/net/tap/rte_eth_tap.h                      |   2 +-
 drivers/net/tap/tap_bpf_program.c                  |  14 +-
 drivers/net/tap/tap_flow.c                         |  12 +-
 drivers/net/thunderx/base/nicvf_mbox.c             |   4 +-
 drivers/net/thunderx/base/nicvf_plat.h             |   2 +-
 drivers/net/thunderx/nicvf_ethdev.c                |  18 +-
 drivers/net/thunderx/nicvf_struct.h                |   2 +-
 drivers/net/vdev_netvsc/vdev_netvsc.c              |  16 +-
 drivers/net/vhost/rte_eth_vhost.c                  |  12 +-
 drivers/net/virtio/virtio_ethdev.c                 |  70 ++---
 drivers/net/virtio/virtio_pci.h                    |   4 +-
 drivers/net/virtio/virtio_rxtx.c                   |  28 +-
 drivers/net/virtio/virtio_user/vhost_kernel_tap.c  |   2 +-
 drivers/net/virtio/virtio_user/virtio_user_dev.c   |   6 +-
 drivers/net/virtio/virtio_user/virtio_user_dev.h   |   2 +-
 drivers/net/virtio/virtio_user_ethdev.c            |   8 +-
 drivers/net/virtio/virtqueue.h                     |   2 +-
 drivers/net/vmxnet3/vmxnet3_ethdev.c               |  12 +-
 drivers/net/vmxnet3/vmxnet3_ethdev.h               |   2 +-
 drivers/net/vmxnet3/vmxnet3_rxtx.c                 |  44 +--
 examples/bbdev_app/main.c                          |  40 +--
 examples/bond/main.c                               |  78 ++---
 examples/distributor/main.c                        |   4 +-
 examples/ethtool/ethtool-app/ethapp.c              |   8 +-
 examples/ethtool/ethtool-app/main.c                |  10 +-
 examples/ethtool/lib/rte_ethtool.c                 |   8 +-
 examples/ethtool/lib/rte_ethtool.h                 |   6 +-
 examples/eventdev_pipeline/main.c                  |   4 +-
 examples/eventdev_pipeline/pipeline_common.h       |  10 +-
 examples/flow_classify/flow_classify.c             |  30 +-
 examples/flow_filtering/main.c                     |  10 +-
 examples/ip_fragmentation/main.c                   |  62 ++--
 examples/ip_pipeline/cli.c                         |   2 +-
 examples/ip_pipeline/kni.c                         |   2 +-
 examples/ip_pipeline/parser.c                      |  18 +-
 examples/ip_pipeline/parser.h                      |   2 +-
 examples/ip_pipeline/pipeline.c                    |  40 +--
 examples/ip_reassembly/main.c                      |  50 ++--
 examples/ipsec-secgw/esp.c                         |  42 +--
 examples/ipsec-secgw/ipsec-secgw.c                 |  38 +--
 examples/ipsec-secgw/sa.c                          |   6 +-
 examples/ipv4_multicast/main.c                     |  58 ++--
 examples/kni/main.c                                |  14 +-
 examples/l2fwd-cat/l2fwd-cat.c                     |   4 +-
 examples/l2fwd-crypto/main.c                       |  26 +-
 examples/l2fwd-jobstats/main.c                     |   8 +-
 examples/l2fwd-keepalive/main.c                    |   8 +-
 examples/l2fwd/main.c                              |   8 +-
 examples/l3fwd-acl/main.c                          | 102 +++----
 examples/l3fwd-power/main.c                        | 100 +++----
 examples/l3fwd-vf/main.c                           |  68 ++---
 examples/l3fwd/l3fwd.h                             |   8 +-
 examples/l3fwd/l3fwd_altivec.h                     |  14 +-
 examples/l3fwd/l3fwd_common.h                      |   4 +-
 examples/l3fwd/l3fwd_em.c                          |  44 +--
 examples/l3fwd/l3fwd_em.h                          |  20 +-
 examples/l3fwd/l3fwd_em_hlm.h                      |  16 +-
 examples/l3fwd/l3fwd_em_hlm_neon.h                 |  16 +-
 examples/l3fwd/l3fwd_em_hlm_sse.h                  |  16 +-
 examples/l3fwd/l3fwd_em_sequential.h               |  16 +-
 examples/l3fwd/l3fwd_lpm.c                         |  50 ++--
 examples/l3fwd/l3fwd_lpm.h                         |  20 +-
 examples/l3fwd/l3fwd_lpm_altivec.h                 |  20 +-
 examples/l3fwd/l3fwd_lpm_neon.h                    |  30 +-
 examples/l3fwd/l3fwd_lpm_sse.h                     |  20 +-
 examples/l3fwd/l3fwd_neon.h                        |  14 +-
 examples/l3fwd/l3fwd_sse.h                         |  14 +-
 examples/l3fwd/main.c                              |  20 +-
 examples/link_status_interrupt/main.c              |   8 +-
 examples/load_balancer/runtime.c                   |   6 +-
 .../client_server_mp/mp_server/main.c              |   2 +-
 examples/packet_ordering/main.c                    |   2 +-
 examples/performance-thread/l3fwd-thread/main.c    | 322 ++++++++++-----------
 examples/ptpclient/ptpclient.c                     |  32 +-
 examples/qos_meter/main.c                          |   4 +-
 examples/qos_sched/init.c                          |   2 +-
 examples/quota_watermark/qw/main.c                 |   8 +-
 examples/rxtx_callbacks/main.c                     |   4 +-
 examples/server_node_efd/node/node.c               |   6 +-
 examples/server_node_efd/server/main.c             |   8 +-
 examples/skeleton/basicfwd.c                       |   4 +-
 examples/tep_termination/main.c                    |   2 +-
 examples/tep_termination/main.h                    |   2 +-
 examples/tep_termination/vxlan.c                   | 108 +++----
 examples/tep_termination/vxlan.h                   |   8 +-
 examples/tep_termination/vxlan_setup.c             |  30 +-
 examples/tep_termination/vxlan_setup.h             |   2 +-
 examples/vhost/main.c                              |  40 +--
 examples/vhost/main.h                              |   2 +-
 examples/vm_power_manager/channel_monitor.c        |   2 +-
 .../guest_cli/vm_power_cli_guest.c                 |   2 +-
 examples/vm_power_manager/main.c                   |   6 +-
 examples/vmdq/main.c                               |  12 +-
 examples/vmdq_dcb/main.c                           |  12 +-
 lib/librte_cmdline/cmdline_parse_etheraddr.c       |  33 +--
 lib/librte_ethdev/rte_eth_ctrl.h                   |  12 +-
 lib/librte_ethdev/rte_ethdev.c                     |  56 ++--
 lib/librte_ethdev/rte_ethdev.h                     |  12 +-
 lib/librte_ethdev/rte_ethdev_core.h                |  12 +-
 lib/librte_ethdev/rte_flow.h                       |  32 +-
 lib/librte_eventdev/rte_event_eth_rx_adapter.c     |  32 +-
 lib/librte_gro/gro_tcp4.c                          |  26 +-
 lib/librte_gro/gro_tcp4.h                          |  20 +-
 lib/librte_gro/gro_vxlan_tcp4.c                    |  64 ++--
 lib/librte_gro/gro_vxlan_tcp4.h                    |   6 +-
 lib/librte_gso/gso_common.h                        |  16 +-
 lib/librte_gso/gso_tcp4.c                          |  12 +-
 lib/librte_gso/gso_tunnel_tcp4.c                   |  14 +-
 lib/librte_gso/gso_udp4.c                          |   8 +-
 lib/librte_gso/rte_gso.h                           |   8 +-
 lib/librte_hash/rte_thash.h                        |   2 +-
 lib/librte_ip_frag/rte_ip_frag.h                   |  12 +-
 lib/librte_ip_frag/rte_ipv4_fragmentation.c        |  42 +--
 lib/librte_ip_frag/rte_ipv4_reassembly.c           |  14 +-
 lib/librte_ip_frag/rte_ipv6_fragmentation.c        |  26 +-
 lib/librte_ip_frag/rte_ipv6_reassembly.c           |   6 +-
 lib/librte_kni/rte_kni.c                           |   4 +-
 lib/librte_kni/rte_kni.h                           |   2 +-
 lib/librte_net/rte_arp.c                           |  32 +-
 lib/librte_net/rte_arp.h                           |  36 +--
 lib/librte_net/rte_esp.h                           |   2 +-
 lib/librte_net/rte_ether.h                         | 178 ++++++------
 lib/librte_net/rte_gre.h                           |   2 +-
 lib/librte_net/rte_icmp.h                          |   6 +-
 lib/librte_net/rte_ip.h                            |  70 ++---
 lib/librte_net/rte_net.c                           |  90 +++---
 lib/librte_net/rte_net.h                           |  22 +-
 lib/librte_net/rte_sctp.h                          |   2 +-
 lib/librte_net/rte_tcp.h                           |   2 +-
 lib/librte_net/rte_udp.h                           |   2 +-
 lib/librte_pipeline/rte_table_action.c             | 210 +++++++-------
 lib/librte_pipeline/rte_table_action.h             |   4 +-
 lib/librte_port/rte_port_ras.c                     |   8 +-
 lib/librte_port/rte_port_source_sink.c             |   6 +-
 lib/librte_vhost/vhost.h                           |   2 +-
 lib/librte_vhost/virtio_net.c                      |  42 +--
 test/test-acl/main.c                               |   2 +-
 test/test-pipeline/pipeline_acl.c                  |  16 +-
 test/test-pipeline/pipeline_hash.c                 |  12 +-
 test/test/packet_burst_generator.c                 | 126 ++++----
 test/test/packet_burst_generator.h                 |  26 +-
 test/test/test_acl.c                               |   8 +-
 test/test/test_acl.h                               | 122 ++++----
 test/test/test_cmdline_etheraddr.c                 |  16 +-
 test/test/test_efd.c                               |  20 +-
 test/test/test_event_eth_rx_adapter.c              |   2 +-
 test/test/test_event_eth_tx_adapter.c              |   2 +-
 test/test/test_flow_classify.c                     |  68 ++---
 test/test/test_hash.c                              |  20 +-
 test/test/test_link_bonding.c                      | 284 +++++++++---------
 test/test/test_link_bonding_mode4.c                | 116 ++++----
 test/test/test_link_bonding_rssconf.c              |   6 +-
 test/test/test_lpm.c                               |  76 ++---
 test/test/test_lpm_perf.c                          |  10 +-
 test/test/test_member.c                            |  20 +-
 test/test/test_pmd_perf.c                          |  20 +-
 test/test/test_sched.c                             |  20 +-
 test/test/test_table_acl.c                         |   8 +-
 test/test/test_thash.c                             |  12 +-
 test/test/virtual_pmd.c                            |   6 +-
 test/test/virtual_pmd.h                            |   2 +-
 367 files changed, 3906 insertions(+), 3913 deletions(-)
  

Comments

Olivier Matz Oct. 24, 2018, 8:32 a.m. UTC | #1
On Wed, Oct 24, 2018 at 10:18:19AM +0200, Olivier Matz wrote:
> This RFC targets 19.02.
> 
> The rte_net headers conflict with the libc headers, because
> some definitions are duplicated, sometimes with few differences.
> This was discussed in [1], and more recently at the techboard.

Sorry, the reference was missing:
[1] http://mails.dpdk.org/archives/dev/2018-January/087384.html
  
Wiles, Keith Oct. 24, 2018, 2:56 p.m. UTC | #2
> On Oct 24, 2018, at 1:18 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
> 
> This RFC targets 19.02.
> 
> The rte_net headers conflict with the libc headers, because
> some definitions are duplicated, sometimes with few differences.
> This was discussed in [1], and more recently at the techboard.
> 
> Before sending the deprecation notice (target for this is 18.11),
> here is a draft that can be discussed.
> 
> This RFC 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.
> 
> One question I'm asking is how can we manage the transition.
> Initially, I hoped it was possible to have a compat layer during
> one release (supporting both prefixed and unprefixed names), but
> now that the patch is done, it seems the impact is too big, and
> impacts too many libraries.
> 
> Few examples:
>  - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *)
>  - many rte_flow structures use the rte_ prefixed net structures
>  - the mac field of virtio_net structure is rte_ether_addr
>  - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *)
>  ...
> 
> Therefore, it is clear that doing this would break the compilation
> of many external applications.
> 
> Another drawback we need to take in account: it will make the
> backport of patches more difficult, although this is something
> that could be tempered by a script.
> 
> While it is obviously better to have a good namespace convention, 
> we need to identify the issues we have today before deciding it's
> worth doing the change.
> 
> Comments?

I did not see the deprecation notice in the patches below, but I could have missed it.
> 
> 
> Things that are missing in RFC:
> - test with FreeBSD
> - manually fix some indentation issues
> 
> 
> Olivier Matz (14):
>  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
> 
> app/pdump/main.c                                   |   2 +-
> app/test-eventdev/test_perf_common.c               |   2 +-
> app/test-eventdev/test_pipeline_common.c           |   2 +-
> app/test-pmd/cmdline.c                             |  66 ++---
> app/test-pmd/cmdline_flow.c                        |  10 +-
> app/test-pmd/config.c                              |  34 +--
> app/test-pmd/csumonly.c                            | 156 +++++-----
> app/test-pmd/flowgen.c                             |  30 +-
> app/test-pmd/icmpecho.c                            | 120 ++++----
> app/test-pmd/ieee1588fwd.c                         |  18 +-
> app/test-pmd/macfwd.c                              |  12 +-
> app/test-pmd/macswap.c                             |  16 +-
> app/test-pmd/parameters.c                          |   6 +-
> app/test-pmd/testpmd.c                             |  22 +-
> app/test-pmd/testpmd.h                             |  18 +-
> app/test-pmd/txonly.c                              |  36 +--
> app/test-pmd/util.c                                |  34 +--
> doc/guides/prog_guide/bbdev.rst                    |   6 +-
> .../prog_guide/packet_classif_access_ctrl.rst      |  18 +-
> doc/guides/prog_guide/rte_flow.rst                 |   4 +-
> doc/guides/sample_app_ug/flow_classify.rst         |  28 +-
> doc/guides/sample_app_ug/flow_filtering.rst        |   6 +-
> doc/guides/sample_app_ug/ip_frag.rst               |  16 +-
> doc/guides/sample_app_ug/ip_reassembly.rst         |  16 +-
> doc/guides/sample_app_ug/ipv4_multicast.rst        |  16 +-
> doc/guides/sample_app_ug/l2_forward_job_stats.rst  |   6 +-
> .../sample_app_ug/l2_forward_real_virtual.rst      |   6 +-
> doc/guides/sample_app_ug/l3_forward.rst            |  12 +-
> doc/guides/sample_app_ug/link_status_intr.rst      |   6 +-
> doc/guides/sample_app_ug/ptpclient.rst             |   6 +-
> doc/guides/sample_app_ug/rxtx_callbacks.rst        |   2 +-
> doc/guides/sample_app_ug/server_node_efd.rst       |  12 +-
> doc/guides/sample_app_ug/skeleton.rst              |   4 +-
> doc/guides/sample_app_ug/vmdq_dcb_forwarding.rst   |   4 +-
> drivers/bus/dpaa/base/fman/fman.c                  |   2 +-
> drivers/bus/dpaa/base/fman/fman_hw.c               |   2 +-
> drivers/bus/dpaa/include/fman.h                    |   2 +-
> drivers/bus/dpaa/include/netcfg.h                  |   4 +-
> drivers/net/af_packet/rte_eth_af_packet.c          |   2 +-
> drivers/net/ark/ark_ethdev.c                       |  16 +-
> drivers/net/ark/ark_ext.h                          |   4 +-
> drivers/net/ark/ark_global.h                       |   4 +-
> drivers/net/atlantic/atl_ethdev.c                  |  20 +-
> drivers/net/atlantic/hw_atl/hw_atl_utils.c         |   8 +-
> drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c    |   4 +-
> drivers/net/avf/avf.h                              |   4 +-
> drivers/net/avf/avf_ethdev.c                       |  50 ++--
> drivers/net/avf/avf_rxtx.c                         |  14 +-
> drivers/net/avf/avf_vchnl.c                        |   8 +-
> drivers/net/avf/base/avf_adminq_cmd.h              |   4 +-
> drivers/net/avf/base/avf_common.c                  |  12 +-
> drivers/net/avf/base/avf_prototype.h               |   4 +-
> drivers/net/avp/avp_ethdev.c                       |  20 +-
> drivers/net/avp/rte_avp_common.h                   |   2 +-
> drivers/net/axgbe/axgbe_dev.c                      |   4 +-
> drivers/net/axgbe/axgbe_ethdev.c                   |  10 +-
> drivers/net/axgbe/axgbe_ethdev.h                   |   4 +-
> drivers/net/axgbe/axgbe_rxtx.c                     |   2 +-
> drivers/net/bnx2x/bnx2x.c                          |  16 +-
> drivers/net/bnx2x/bnx2x_ethdev.c                   |   4 +-
> drivers/net/bnx2x/bnx2x_ethdev.h                   |   2 +-
> drivers/net/bnx2x/bnx2x_vfpf.c                     |   8 +-
> drivers/net/bnx2x/bnx2x_vfpf.h                     |   2 +-
> drivers/net/bnx2x/ecore_sp.h                       |   2 +-
> drivers/net/bnxt/bnxt.h                            |   4 +-
> drivers/net/bnxt/bnxt_ethdev.c                     |  70 ++---
> drivers/net/bnxt/bnxt_filter.c                     |   4 +-
> drivers/net/bnxt/bnxt_filter.h                     |   8 +-
> drivers/net/bnxt/bnxt_flow.c                       |  26 +-
> drivers/net/bnxt/bnxt_hwrm.c                       |  40 +--
> drivers/net/bnxt/bnxt_hwrm.h                       |   2 +-
> drivers/net/bnxt/bnxt_ring.c                       |   8 +-
> drivers/net/bnxt/bnxt_rxq.c                        |   2 +-
> drivers/net/bnxt/bnxt_rxr.c                        |   2 +-
> drivers/net/bnxt/bnxt_vnic.c                       |   2 +-
> drivers/net/bnxt/rte_pmd_bnxt.c                    |  14 +-
> drivers/net/bnxt/rte_pmd_bnxt.h                    |   4 +-
> drivers/net/bonding/rte_eth_bond.h                 |   2 +-
> drivers/net/bonding/rte_eth_bond_8023ad.c          |  26 +-
> drivers/net/bonding/rte_eth_bond_8023ad.h          |  10 +-
> drivers/net/bonding/rte_eth_bond_alb.c             |  78 ++---
> drivers/net/bonding/rte_eth_bond_alb.h             |  10 +-
> drivers/net/bonding/rte_eth_bond_api.c             |   2 +-
> drivers/net/bonding/rte_eth_bond_args.c            |   2 +-
> drivers/net/bonding/rte_eth_bond_pmd.c             | 194 ++++++-------
> drivers/net/bonding/rte_eth_bond_private.h         |   6 +-
> drivers/net/cxgbe/base/adapter.h                   |   6 +-
> drivers/net/cxgbe/base/t4_hw.c                     |   8 +-
> drivers/net/cxgbe/cxgbe.h                          |   4 +-
> drivers/net/cxgbe/cxgbe_ethdev.c                   |  14 +-
> drivers/net/cxgbe/cxgbe_filter.h                   |   2 +-
> drivers/net/cxgbe/cxgbe_flow.c                     |  10 +-
> drivers/net/cxgbe/cxgbe_main.c                     |   4 +-
> drivers/net/cxgbe/cxgbe_pfvf.h                     |   2 +-
> drivers/net/cxgbe/cxgbevf_main.c                   |   2 +-
> drivers/net/cxgbe/l2t.c                            |   8 +-
> drivers/net/cxgbe/l2t.h                            |   2 +-
> drivers/net/cxgbe/mps_tcam.c                       |  14 +-
> drivers/net/cxgbe/mps_tcam.h                       |   4 +-
> drivers/net/cxgbe/sge.c                            |   8 +-
> drivers/net/dpaa/dpaa_ethdev.c                     |  20 +-
> drivers/net/dpaa/dpaa_rxtx.c                       |  22 +-
> drivers/net/dpaa2/dpaa2_ethdev.c                   |  36 +--
> drivers/net/e1000/e1000_ethdev.h                   |   2 +-
> drivers/net/e1000/em_ethdev.c                      |  34 +--
> drivers/net/e1000/em_rxtx.c                        |  22 +-
> drivers/net/e1000/igb_ethdev.c                     |  70 ++---
> drivers/net/e1000/igb_flow.c                       |  12 +-
> drivers/net/e1000/igb_pf.c                         |  16 +-
> drivers/net/e1000/igb_rxtx.c                       |  18 +-
> drivers/net/ena/ena_ethdev.c                       |  16 +-
> drivers/net/ena/ena_ethdev.h                       |   2 +-
> drivers/net/enetc/base/enetc_hw.h                  |   4 +-
> drivers/net/enetc/enetc_ethdev.c                   |   6 +-
> drivers/net/enic/enic.h                            |   2 +-
> drivers/net/enic/enic_clsf.c                       |  40 +--
> drivers/net/enic/enic_ethdev.c                     |   4 +-
> drivers/net/enic/enic_flow.c                       | 100 +++----
> drivers/net/enic/enic_main.c                       |   2 +-
> drivers/net/enic/enic_res.c                        |   4 +-
> drivers/net/failsafe/failsafe.c                    |   6 +-
> drivers/net/failsafe/failsafe_args.c               |   4 +-
> drivers/net/failsafe/failsafe_ether.c              |   6 +-
> drivers/net/failsafe/failsafe_ops.c                |   6 +-
> drivers/net/failsafe/failsafe_private.h            |   4 +-
> drivers/net/fm10k/fm10k.h                          |   2 +-
> drivers/net/fm10k/fm10k_ethdev.c                   |  18 +-
> drivers/net/i40e/base/i40e_adminq_cmd.h            |   4 +-
> drivers/net/i40e/base/i40e_common.c                |  12 +-
> drivers/net/i40e/base/i40e_prototype.h             |   4 +-
> drivers/net/i40e/i40e_ethdev.c                     | 134 ++++-----
> drivers/net/i40e/i40e_ethdev.h                     |  22 +-
> drivers/net/i40e/i40e_ethdev_vf.c                  |  60 ++--
> drivers/net/i40e/i40e_fdir.c                       | 126 ++++----
> drivers/net/i40e/i40e_flow.c                       |  58 ++--
> drivers/net/i40e/i40e_pf.c                         |  18 +-
> drivers/net/i40e/i40e_rxtx.c                       |  28 +-
> drivers/net/i40e/i40e_vf_representor.c             |   2 +-
> drivers/net/i40e/rte_pmd_i40e.c                    |  30 +-
> drivers/net/i40e/rte_pmd_i40e.h                    |   8 +-
> drivers/net/ixgbe/ixgbe_ethdev.c                   |  94 +++---
> drivers/net/ixgbe/ixgbe_ethdev.h                   |   2 +-
> drivers/net/ixgbe/ixgbe_flow.c                     |  22 +-
> drivers/net/ixgbe/ixgbe_pf.c                       |  14 +-
> drivers/net/ixgbe/ixgbe_rxtx.c                     |  14 +-
> drivers/net/ixgbe/ixgbe_vf_representor.c           |   4 +-
> drivers/net/ixgbe/rte_pmd_ixgbe.c                  |  10 +-
> drivers/net/ixgbe/rte_pmd_ixgbe.h                  |   2 +-
> drivers/net/kni/rte_eth_kni.c                      |   4 +-
> drivers/net/liquidio/lio_ethdev.c                  |  22 +-
> drivers/net/mlx4/mlx4.c                            |   4 +-
> drivers/net/mlx4/mlx4.h                            |   8 +-
> drivers/net/mlx4/mlx4_ethdev.c                     |   8 +-
> drivers/net/mlx4/mlx4_flow.c                       |  14 +-
> drivers/net/mlx4/mlx4_rxtx.c                       |   2 +-
> drivers/net/mlx5/mlx5.c                            |   4 +-
> drivers/net/mlx5/mlx5.h                            |  14 +-
> drivers/net/mlx5/mlx5_flow.c                       |  22 +-
> drivers/net/mlx5/mlx5_flow_tcf.c                   |  40 +--
> drivers/net/mlx5/mlx5_flow_verbs.c                 |  26 +-
> drivers/net/mlx5/mlx5_mac.c                        |  18 +-
> drivers/net/mlx5/mlx5_nl.c                         |  28 +-
> drivers/net/mlx5/mlx5_rxtx.c                       |   6 +-
> drivers/net/mlx5/mlx5_rxtx.h                       |   2 +-
> drivers/net/mlx5/mlx5_rxtx_vec_neon.h              |   8 +-
> drivers/net/mlx5/mlx5_rxtx_vec_sse.h               |  10 +-
> drivers/net/mlx5/mlx5_trigger.c                    |   6 +-
> drivers/net/mvneta/mvneta_ethdev.c                 |  22 +-
> drivers/net/mvneta/mvneta_ethdev.h                 |   2 +-
> drivers/net/mvpp2/mrvl_ethdev.c                    |  22 +-
> drivers/net/mvpp2/mrvl_ethdev.h                    |   2 +-
> drivers/net/mvpp2/mrvl_flow.c                      |   4 +-
> drivers/net/netvsc/hn_ethdev.c                     |   4 +-
> drivers/net/netvsc/hn_nvs.c                        |   2 +-
> drivers/net/netvsc/hn_rndis.c                      |   2 +-
> drivers/net/netvsc/hn_rxtx.c                       |  12 +-
> drivers/net/netvsc/hn_var.h                        |   4 +-
> drivers/net/netvsc/hn_vf.c                         |  12 +-
> drivers/net/nfp/nfp_net.c                          |  20 +-
> drivers/net/nfp/nfp_net_pmd.h                      |   2 +-
> drivers/net/null/rte_eth_null.c                    |   6 +-
> drivers/net/octeontx/octeontx_ethdev.c             |   8 +-
> drivers/net/octeontx/octeontx_ethdev.h             |   2 +-
> drivers/net/pcap/rte_eth_pcap.c                    |  22 +-
> drivers/net/qede/base/bcm_osal.h                   |   2 +-
> drivers/net/qede/base/ecore_dev.c                  |   4 +-
> drivers/net/qede/qede_ethdev.c                     |  58 ++--
> drivers/net/qede/qede_ethdev.h                     |   6 +-
> drivers/net/qede/qede_filter.c                     |  66 ++---
> drivers/net/qede/qede_if.h                         |   4 +-
> drivers/net/qede/qede_main.c                       |   6 +-
> drivers/net/qede/qede_rxtx.c                       |  32 +-
> drivers/net/qede/qede_rxtx.h                       |   2 +-
> drivers/net/ring/rte_eth_ring.c                    |   4 +-
> drivers/net/sfc/sfc.h                              |   2 +-
> drivers/net/sfc/sfc_ef10_tx.c                      |   8 +-
> drivers/net/sfc/sfc_ethdev.c                       |  20 +-
> drivers/net/sfc/sfc_flow.c                         |  12 +-
> drivers/net/sfc/sfc_port.c                         |   8 +-
> drivers/net/sfc/sfc_tso.c                          |   8 +-
> drivers/net/softnic/parser.c                       |  18 +-
> drivers/net/softnic/parser.h                       |   2 +-
> drivers/net/softnic/rte_eth_softnic.c              |   2 +-
> drivers/net/softnic/rte_eth_softnic_pipeline.c     |  40 +--
> drivers/net/szedata2/rte_eth_szedata2.c            |   8 +-
> drivers/net/tap/rte_eth_tap.c                      |  58 ++--
> drivers/net/tap/rte_eth_tap.h                      |   2 +-
> drivers/net/tap/tap_bpf_program.c                  |  14 +-
> drivers/net/tap/tap_flow.c                         |  12 +-
> drivers/net/thunderx/base/nicvf_mbox.c             |   4 +-
> drivers/net/thunderx/base/nicvf_plat.h             |   2 +-
> drivers/net/thunderx/nicvf_ethdev.c                |  18 +-
> drivers/net/thunderx/nicvf_struct.h                |   2 +-
> drivers/net/vdev_netvsc/vdev_netvsc.c              |  16 +-
> drivers/net/vhost/rte_eth_vhost.c                  |  12 +-
> drivers/net/virtio/virtio_ethdev.c                 |  70 ++---
> drivers/net/virtio/virtio_pci.h                    |   4 +-
> drivers/net/virtio/virtio_rxtx.c                   |  28 +-
> drivers/net/virtio/virtio_user/vhost_kernel_tap.c  |   2 +-
> drivers/net/virtio/virtio_user/virtio_user_dev.c   |   6 +-
> drivers/net/virtio/virtio_user/virtio_user_dev.h   |   2 +-
> drivers/net/virtio/virtio_user_ethdev.c            |   8 +-
> drivers/net/virtio/virtqueue.h                     |   2 +-
> drivers/net/vmxnet3/vmxnet3_ethdev.c               |  12 +-
> drivers/net/vmxnet3/vmxnet3_ethdev.h               |   2 +-
> drivers/net/vmxnet3/vmxnet3_rxtx.c                 |  44 +--
> examples/bbdev_app/main.c                          |  40 +--
> examples/bond/main.c                               |  78 ++---
> examples/distributor/main.c                        |   4 +-
> examples/ethtool/ethtool-app/ethapp.c              |   8 +-
> examples/ethtool/ethtool-app/main.c                |  10 +-
> examples/ethtool/lib/rte_ethtool.c                 |   8 +-
> examples/ethtool/lib/rte_ethtool.h                 |   6 +-
> examples/eventdev_pipeline/main.c                  |   4 +-
> examples/eventdev_pipeline/pipeline_common.h       |  10 +-
> examples/flow_classify/flow_classify.c             |  30 +-
> examples/flow_filtering/main.c                     |  10 +-
> examples/ip_fragmentation/main.c                   |  62 ++--
> examples/ip_pipeline/cli.c                         |   2 +-
> examples/ip_pipeline/kni.c                         |   2 +-
> examples/ip_pipeline/parser.c                      |  18 +-
> examples/ip_pipeline/parser.h                      |   2 +-
> examples/ip_pipeline/pipeline.c                    |  40 +--
> examples/ip_reassembly/main.c                      |  50 ++--
> examples/ipsec-secgw/esp.c                         |  42 +--
> examples/ipsec-secgw/ipsec-secgw.c                 |  38 +--
> examples/ipsec-secgw/sa.c                          |   6 +-
> examples/ipv4_multicast/main.c                     |  58 ++--
> examples/kni/main.c                                |  14 +-
> examples/l2fwd-cat/l2fwd-cat.c                     |   4 +-
> examples/l2fwd-crypto/main.c                       |  26 +-
> examples/l2fwd-jobstats/main.c                     |   8 +-
> examples/l2fwd-keepalive/main.c                    |   8 +-
> examples/l2fwd/main.c                              |   8 +-
> examples/l3fwd-acl/main.c                          | 102 +++----
> examples/l3fwd-power/main.c                        | 100 +++----
> examples/l3fwd-vf/main.c                           |  68 ++---
> examples/l3fwd/l3fwd.h                             |   8 +-
> examples/l3fwd/l3fwd_altivec.h                     |  14 +-
> examples/l3fwd/l3fwd_common.h                      |   4 +-
> examples/l3fwd/l3fwd_em.c                          |  44 +--
> examples/l3fwd/l3fwd_em.h                          |  20 +-
> examples/l3fwd/l3fwd_em_hlm.h                      |  16 +-
> examples/l3fwd/l3fwd_em_hlm_neon.h                 |  16 +-
> examples/l3fwd/l3fwd_em_hlm_sse.h                  |  16 +-
> examples/l3fwd/l3fwd_em_sequential.h               |  16 +-
> examples/l3fwd/l3fwd_lpm.c                         |  50 ++--
> examples/l3fwd/l3fwd_lpm.h                         |  20 +-
> examples/l3fwd/l3fwd_lpm_altivec.h                 |  20 +-
> examples/l3fwd/l3fwd_lpm_neon.h                    |  30 +-
> examples/l3fwd/l3fwd_lpm_sse.h                     |  20 +-
> examples/l3fwd/l3fwd_neon.h                        |  14 +-
> examples/l3fwd/l3fwd_sse.h                         |  14 +-
> examples/l3fwd/main.c                              |  20 +-
> examples/link_status_interrupt/main.c              |   8 +-
> examples/load_balancer/runtime.c                   |   6 +-
> .../client_server_mp/mp_server/main.c              |   2 +-
> examples/packet_ordering/main.c                    |   2 +-
> examples/performance-thread/l3fwd-thread/main.c    | 322 ++++++++++-----------
> examples/ptpclient/ptpclient.c                     |  32 +-
> examples/qos_meter/main.c                          |   4 +-
> examples/qos_sched/init.c                          |   2 +-
> examples/quota_watermark/qw/main.c                 |   8 +-
> examples/rxtx_callbacks/main.c                     |   4 +-
> examples/server_node_efd/node/node.c               |   6 +-
> examples/server_node_efd/server/main.c             |   8 +-
> examples/skeleton/basicfwd.c                       |   4 +-
> examples/tep_termination/main.c                    |   2 +-
> examples/tep_termination/main.h                    |   2 +-
> examples/tep_termination/vxlan.c                   | 108 +++----
> examples/tep_termination/vxlan.h                   |   8 +-
> examples/tep_termination/vxlan_setup.c             |  30 +-
> examples/tep_termination/vxlan_setup.h             |   2 +-
> examples/vhost/main.c                              |  40 +--
> examples/vhost/main.h                              |   2 +-
> examples/vm_power_manager/channel_monitor.c        |   2 +-
> .../guest_cli/vm_power_cli_guest.c                 |   2 +-
> examples/vm_power_manager/main.c                   |   6 +-
> examples/vmdq/main.c                               |  12 +-
> examples/vmdq_dcb/main.c                           |  12 +-
> lib/librte_cmdline/cmdline_parse_etheraddr.c       |  33 +--
> lib/librte_ethdev/rte_eth_ctrl.h                   |  12 +-
> lib/librte_ethdev/rte_ethdev.c                     |  56 ++--
> lib/librte_ethdev/rte_ethdev.h                     |  12 +-
> lib/librte_ethdev/rte_ethdev_core.h                |  12 +-
> lib/librte_ethdev/rte_flow.h                       |  32 +-
> lib/librte_eventdev/rte_event_eth_rx_adapter.c     |  32 +-
> lib/librte_gro/gro_tcp4.c                          |  26 +-
> lib/librte_gro/gro_tcp4.h                          |  20 +-
> lib/librte_gro/gro_vxlan_tcp4.c                    |  64 ++--
> lib/librte_gro/gro_vxlan_tcp4.h                    |   6 +-
> lib/librte_gso/gso_common.h                        |  16 +-
> lib/librte_gso/gso_tcp4.c                          |  12 +-
> lib/librte_gso/gso_tunnel_tcp4.c                   |  14 +-
> lib/librte_gso/gso_udp4.c                          |   8 +-
> lib/librte_gso/rte_gso.h                           |   8 +-
> lib/librte_hash/rte_thash.h                        |   2 +-
> lib/librte_ip_frag/rte_ip_frag.h                   |  12 +-
> lib/librte_ip_frag/rte_ipv4_fragmentation.c        |  42 +--
> lib/librte_ip_frag/rte_ipv4_reassembly.c           |  14 +-
> lib/librte_ip_frag/rte_ipv6_fragmentation.c        |  26 +-
> lib/librte_ip_frag/rte_ipv6_reassembly.c           |   6 +-
> lib/librte_kni/rte_kni.c                           |   4 +-
> lib/librte_kni/rte_kni.h                           |   2 +-
> lib/librte_net/rte_arp.c                           |  32 +-
> lib/librte_net/rte_arp.h                           |  36 +--
> lib/librte_net/rte_esp.h                           |   2 +-
> lib/librte_net/rte_ether.h                         | 178 ++++++------
> lib/librte_net/rte_gre.h                           |   2 +-
> lib/librte_net/rte_icmp.h                          |   6 +-
> lib/librte_net/rte_ip.h                            |  70 ++---
> lib/librte_net/rte_net.c                           |  90 +++---
> lib/librte_net/rte_net.h                           |  22 +-
> lib/librte_net/rte_sctp.h                          |   2 +-
> lib/librte_net/rte_tcp.h                           |   2 +-
> lib/librte_net/rte_udp.h                           |   2 +-
> lib/librte_pipeline/rte_table_action.c             | 210 +++++++-------
> lib/librte_pipeline/rte_table_action.h             |   4 +-
> lib/librte_port/rte_port_ras.c                     |   8 +-
> lib/librte_port/rte_port_source_sink.c             |   6 +-
> lib/librte_vhost/vhost.h                           |   2 +-
> lib/librte_vhost/virtio_net.c                      |  42 +--
> test/test-acl/main.c                               |   2 +-
> test/test-pipeline/pipeline_acl.c                  |  16 +-
> test/test-pipeline/pipeline_hash.c                 |  12 +-
> test/test/packet_burst_generator.c                 | 126 ++++----
> test/test/packet_burst_generator.h                 |  26 +-
> test/test/test_acl.c                               |   8 +-
> test/test/test_acl.h                               | 122 ++++----
> test/test/test_cmdline_etheraddr.c                 |  16 +-
> test/test/test_efd.c                               |  20 +-
> test/test/test_event_eth_rx_adapter.c              |   2 +-
> test/test/test_event_eth_tx_adapter.c              |   2 +-
> test/test/test_flow_classify.c                     |  68 ++---
> test/test/test_hash.c                              |  20 +-
> test/test/test_link_bonding.c                      | 284 +++++++++---------
> test/test/test_link_bonding_mode4.c                | 116 ++++----
> test/test/test_link_bonding_rssconf.c              |   6 +-
> test/test/test_lpm.c                               |  76 ++---
> test/test/test_lpm_perf.c                          |  10 +-
> test/test/test_member.c                            |  20 +-
> test/test/test_pmd_perf.c                          |  20 +-
> test/test/test_sched.c                             |  20 +-
> test/test/test_table_acl.c                         |   8 +-
> test/test/test_thash.c                             |  12 +-
> test/test/virtual_pmd.c                            |   6 +-
> test/test/virtual_pmd.h                            |   2 +-
> 367 files changed, 3906 insertions(+), 3913 deletions(-)
> 
> -- 
> 2.11.0
> 

Regards,
Keith
  
Stephen Hemminger Oct. 24, 2018, 4:09 p.m. UTC | #3
On Wed, 24 Oct 2018 10:18:19 +0200
Olivier Matz <olivier.matz@6wind.com> wrote:

> This RFC targets 19.02.
> 
> The rte_net headers conflict with the libc headers, because
> some definitions are duplicated, sometimes with few differences.
> This was discussed in [1], and more recently at the techboard.
> 
> Before sending the deprecation notice (target for this is 18.11),
> here is a draft that can be discussed.
> 
> This RFC 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.
> 
> One question I'm asking is how can we manage the transition.
> Initially, I hoped it was possible to have a compat layer during
> one release (supporting both prefixed and unprefixed names), but
> now that the patch is done, it seems the impact is too big, and
> impacts too many libraries.
> 
> Few examples:
>   - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *)
>   - many rte_flow structures use the rte_ prefixed net structures
>   - the mac field of virtio_net structure is rte_ether_addr
>   - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *)
>   ...
> 
> Therefore, it is clear that doing this would break the compilation
> of many external applications.
> 
> Another drawback we need to take in account: it will make the
> backport of patches more difficult, although this is something
> that could be tempered by a script.
> 
> While it is obviously better to have a good namespace convention, 
> we need to identify the issues we have today before deciding it's
> worth doing the change.
> 
> Comments?
> 
> 
> Things that are missing in RFC:
> - test with FreeBSD
> - manually fix some indentation issues
> 
> 
> Olivier Matz (14):
>   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
> 
>  app/pdump/main.c                                   |   2 +-
>  app/test-eventdev/test_perf_common.c               |   2 +-
>  app/test-eventdev/test_pipeline_common.c           |   2 +-
>  app/test-pmd/cmdline.c                             |  66 ++---
>  app/test-pmd/cmdline_flow.c                        |  10 +-
>  app/test-pmd/config.c                              |  34 +--
>  app/test-pmd/csumonly.c                            | 156 +++++-----
>  app/test-pmd/flowgen.c                             |  30 +-
>  app/test-pmd/icmpecho.c                            | 120 ++++----
>  app/test-pmd/ieee1588fwd.c                         |  18 +-
>  app/test-pmd/macfwd.c                              |  12 +-
>  app/test-pmd/macswap.c                             |  16 +-
>  app/test-pmd/parameters.c                          |   6 +-
>  app/test-pmd/testpmd.c                             |  22 +-
>  app/test-pmd/testpmd.h                             |  18 +-
>  app/test-pmd/txonly.c                              |  36 +--
>  app/test-pmd/util.c                                |  34 +--
>  doc/guides/prog_guide/bbdev.rst                    |   6 +-
>  .../prog_guide/packet_classif_access_ctrl.rst      |  18 +-
>  doc/guides/prog_guide/rte_flow.rst                 |   4 +-
>  doc/guides/sample_app_ug/flow_classify.rst         |  28 +-
>  doc/guides/sample_app_ug/flow_filtering.rst        |   6 +-
>  doc/guides/sample_app_ug/ip_frag.rst               |  16 +-
>  doc/guides/sample_app_ug/ip_reassembly.rst         |  16 +-
>  doc/guides/sample_app_ug/ipv4_multicast.rst        |  16 +-
>  doc/guides/sample_app_ug/l2_forward_job_stats.rst  |   6 +-
>  .../sample_app_ug/l2_forward_real_virtual.rst      |   6 +-
>  doc/guides/sample_app_ug/l3_forward.rst            |  12 +-
>  doc/guides/sample_app_ug/link_status_intr.rst      |   6 +-
>  doc/guides/sample_app_ug/ptpclient.rst             |   6 +-
>  doc/guides/sample_app_ug/rxtx_callbacks.rst        |   2 +-
>  doc/guides/sample_app_ug/server_node_efd.rst       |  12 +-
>  doc/guides/sample_app_ug/skeleton.rst              |   4 +-
>  doc/guides/sample_app_ug/vmdq_dcb_forwarding.rst   |   4 +-
>  drivers/bus/dpaa/base/fman/fman.c                  |   2 +-
>  drivers/bus/dpaa/base/fman/fman_hw.c               |   2 +-
>  drivers/bus/dpaa/include/fman.h                    |   2 +-
>  drivers/bus/dpaa/include/netcfg.h                  |   4 +-
>  drivers/net/af_packet/rte_eth_af_packet.c          |   2 +-
>  drivers/net/ark/ark_ethdev.c                       |  16 +-
>  drivers/net/ark/ark_ext.h                          |   4 +-
>  drivers/net/ark/ark_global.h                       |   4 +-
>  drivers/net/atlantic/atl_ethdev.c                  |  20 +-
>  drivers/net/atlantic/hw_atl/hw_atl_utils.c         |   8 +-
>  drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c    |   4 +-
>  drivers/net/avf/avf.h                              |   4 +-
>  drivers/net/avf/avf_ethdev.c                       |  50 ++--
>  drivers/net/avf/avf_rxtx.c                         |  14 +-
>  drivers/net/avf/avf_vchnl.c                        |   8 +-
>  drivers/net/avf/base/avf_adminq_cmd.h              |   4 +-
>  drivers/net/avf/base/avf_common.c                  |  12 +-
>  drivers/net/avf/base/avf_prototype.h               |   4 +-
>  drivers/net/avp/avp_ethdev.c                       |  20 +-
>  drivers/net/avp/rte_avp_common.h                   |   2 +-
>  drivers/net/axgbe/axgbe_dev.c                      |   4 +-
>  drivers/net/axgbe/axgbe_ethdev.c                   |  10 +-
>  drivers/net/axgbe/axgbe_ethdev.h                   |   4 +-
>  drivers/net/axgbe/axgbe_rxtx.c                     |   2 +-
>  drivers/net/bnx2x/bnx2x.c                          |  16 +-
>  drivers/net/bnx2x/bnx2x_ethdev.c                   |   4 +-
>  drivers/net/bnx2x/bnx2x_ethdev.h                   |   2 +-
>  drivers/net/bnx2x/bnx2x_vfpf.c                     |   8 +-
>  drivers/net/bnx2x/bnx2x_vfpf.h                     |   2 +-
>  drivers/net/bnx2x/ecore_sp.h                       |   2 +-
>  drivers/net/bnxt/bnxt.h                            |   4 +-
>  drivers/net/bnxt/bnxt_ethdev.c                     |  70 ++---
>  drivers/net/bnxt/bnxt_filter.c                     |   4 +-
>  drivers/net/bnxt/bnxt_filter.h                     |   8 +-
>  drivers/net/bnxt/bnxt_flow.c                       |  26 +-
>  drivers/net/bnxt/bnxt_hwrm.c                       |  40 +--
>  drivers/net/bnxt/bnxt_hwrm.h                       |   2 +-
>  drivers/net/bnxt/bnxt_ring.c                       |   8 +-
>  drivers/net/bnxt/bnxt_rxq.c                        |   2 +-
>  drivers/net/bnxt/bnxt_rxr.c                        |   2 +-
>  drivers/net/bnxt/bnxt_vnic.c                       |   2 +-
>  drivers/net/bnxt/rte_pmd_bnxt.c                    |  14 +-
>  drivers/net/bnxt/rte_pmd_bnxt.h                    |   4 +-
>  drivers/net/bonding/rte_eth_bond.h                 |   2 +-
>  drivers/net/bonding/rte_eth_bond_8023ad.c          |  26 +-
>  drivers/net/bonding/rte_eth_bond_8023ad.h          |  10 +-
>  drivers/net/bonding/rte_eth_bond_alb.c             |  78 ++---
>  drivers/net/bonding/rte_eth_bond_alb.h             |  10 +-
>  drivers/net/bonding/rte_eth_bond_api.c             |   2 +-
>  drivers/net/bonding/rte_eth_bond_args.c            |   2 +-
>  drivers/net/bonding/rte_eth_bond_pmd.c             | 194 ++++++-------
>  drivers/net/bonding/rte_eth_bond_private.h         |   6 +-
>  drivers/net/cxgbe/base/adapter.h                   |   6 +-
>  drivers/net/cxgbe/base/t4_hw.c                     |   8 +-
>  drivers/net/cxgbe/cxgbe.h                          |   4 +-
>  drivers/net/cxgbe/cxgbe_ethdev.c                   |  14 +-
>  drivers/net/cxgbe/cxgbe_filter.h                   |   2 +-
>  drivers/net/cxgbe/cxgbe_flow.c                     |  10 +-
>  drivers/net/cxgbe/cxgbe_main.c                     |   4 +-
>  drivers/net/cxgbe/cxgbe_pfvf.h                     |   2 +-
>  drivers/net/cxgbe/cxgbevf_main.c                   |   2 +-
>  drivers/net/cxgbe/l2t.c                            |   8 +-
>  drivers/net/cxgbe/l2t.h                            |   2 +-
>  drivers/net/cxgbe/mps_tcam.c                       |  14 +-
>  drivers/net/cxgbe/mps_tcam.h                       |   4 +-
>  drivers/net/cxgbe/sge.c                            |   8 +-
>  drivers/net/dpaa/dpaa_ethdev.c                     |  20 +-
>  drivers/net/dpaa/dpaa_rxtx.c                       |  22 +-
>  drivers/net/dpaa2/dpaa2_ethdev.c                   |  36 +--
>  drivers/net/e1000/e1000_ethdev.h                   |   2 +-
>  drivers/net/e1000/em_ethdev.c                      |  34 +--
>  drivers/net/e1000/em_rxtx.c                        |  22 +-
>  drivers/net/e1000/igb_ethdev.c                     |  70 ++---
>  drivers/net/e1000/igb_flow.c                       |  12 +-
>  drivers/net/e1000/igb_pf.c                         |  16 +-
>  drivers/net/e1000/igb_rxtx.c                       |  18 +-
>  drivers/net/ena/ena_ethdev.c                       |  16 +-
>  drivers/net/ena/ena_ethdev.h                       |   2 +-
>  drivers/net/enetc/base/enetc_hw.h                  |   4 +-
>  drivers/net/enetc/enetc_ethdev.c                   |   6 +-
>  drivers/net/enic/enic.h                            |   2 +-
>  drivers/net/enic/enic_clsf.c                       |  40 +--
>  drivers/net/enic/enic_ethdev.c                     |   4 +-
>  drivers/net/enic/enic_flow.c                       | 100 +++----
>  drivers/net/enic/enic_main.c                       |   2 +-
>  drivers/net/enic/enic_res.c                        |   4 +-
>  drivers/net/failsafe/failsafe.c                    |   6 +-
>  drivers/net/failsafe/failsafe_args.c               |   4 +-
>  drivers/net/failsafe/failsafe_ether.c              |   6 +-
>  drivers/net/failsafe/failsafe_ops.c                |   6 +-
>  drivers/net/failsafe/failsafe_private.h            |   4 +-
>  drivers/net/fm10k/fm10k.h                          |   2 +-
>  drivers/net/fm10k/fm10k_ethdev.c                   |  18 +-
>  drivers/net/i40e/base/i40e_adminq_cmd.h            |   4 +-
>  drivers/net/i40e/base/i40e_common.c                |  12 +-
>  drivers/net/i40e/base/i40e_prototype.h             |   4 +-
>  drivers/net/i40e/i40e_ethdev.c                     | 134 ++++-----
>  drivers/net/i40e/i40e_ethdev.h                     |  22 +-
>  drivers/net/i40e/i40e_ethdev_vf.c                  |  60 ++--
>  drivers/net/i40e/i40e_fdir.c                       | 126 ++++----
>  drivers/net/i40e/i40e_flow.c                       |  58 ++--
>  drivers/net/i40e/i40e_pf.c                         |  18 +-
>  drivers/net/i40e/i40e_rxtx.c                       |  28 +-
>  drivers/net/i40e/i40e_vf_representor.c             |   2 +-
>  drivers/net/i40e/rte_pmd_i40e.c                    |  30 +-
>  drivers/net/i40e/rte_pmd_i40e.h                    |   8 +-
>  drivers/net/ixgbe/ixgbe_ethdev.c                   |  94 +++---
>  drivers/net/ixgbe/ixgbe_ethdev.h                   |   2 +-
>  drivers/net/ixgbe/ixgbe_flow.c                     |  22 +-
>  drivers/net/ixgbe/ixgbe_pf.c                       |  14 +-
>  drivers/net/ixgbe/ixgbe_rxtx.c                     |  14 +-
>  drivers/net/ixgbe/ixgbe_vf_representor.c           |   4 +-
>  drivers/net/ixgbe/rte_pmd_ixgbe.c                  |  10 +-
>  drivers/net/ixgbe/rte_pmd_ixgbe.h                  |   2 +-
>  drivers/net/kni/rte_eth_kni.c                      |   4 +-
>  drivers/net/liquidio/lio_ethdev.c                  |  22 +-
>  drivers/net/mlx4/mlx4.c                            |   4 +-
>  drivers/net/mlx4/mlx4.h                            |   8 +-
>  drivers/net/mlx4/mlx4_ethdev.c                     |   8 +-
>  drivers/net/mlx4/mlx4_flow.c                       |  14 +-
>  drivers/net/mlx4/mlx4_rxtx.c                       |   2 +-
>  drivers/net/mlx5/mlx5.c                            |   4 +-
>  drivers/net/mlx5/mlx5.h                            |  14 +-
>  drivers/net/mlx5/mlx5_flow.c                       |  22 +-
>  drivers/net/mlx5/mlx5_flow_tcf.c                   |  40 +--
>  drivers/net/mlx5/mlx5_flow_verbs.c                 |  26 +-
>  drivers/net/mlx5/mlx5_mac.c                        |  18 +-
>  drivers/net/mlx5/mlx5_nl.c                         |  28 +-
>  drivers/net/mlx5/mlx5_rxtx.c                       |   6 +-
>  drivers/net/mlx5/mlx5_rxtx.h                       |   2 +-
>  drivers/net/mlx5/mlx5_rxtx_vec_neon.h              |   8 +-
>  drivers/net/mlx5/mlx5_rxtx_vec_sse.h               |  10 +-
>  drivers/net/mlx5/mlx5_trigger.c                    |   6 +-
>  drivers/net/mvneta/mvneta_ethdev.c                 |  22 +-
>  drivers/net/mvneta/mvneta_ethdev.h                 |   2 +-
>  drivers/net/mvpp2/mrvl_ethdev.c                    |  22 +-
>  drivers/net/mvpp2/mrvl_ethdev.h                    |   2 +-
>  drivers/net/mvpp2/mrvl_flow.c                      |   4 +-
>  drivers/net/netvsc/hn_ethdev.c                     |   4 +-
>  drivers/net/netvsc/hn_nvs.c                        |   2 +-
>  drivers/net/netvsc/hn_rndis.c                      |   2 +-
>  drivers/net/netvsc/hn_rxtx.c                       |  12 +-
>  drivers/net/netvsc/hn_var.h                        |   4 +-
>  drivers/net/netvsc/hn_vf.c                         |  12 +-
>  drivers/net/nfp/nfp_net.c                          |  20 +-
>  drivers/net/nfp/nfp_net_pmd.h                      |   2 +-
>  drivers/net/null/rte_eth_null.c                    |   6 +-
>  drivers/net/octeontx/octeontx_ethdev.c             |   8 +-
>  drivers/net/octeontx/octeontx_ethdev.h             |   2 +-
>  drivers/net/pcap/rte_eth_pcap.c                    |  22 +-
>  drivers/net/qede/base/bcm_osal.h                   |   2 +-
>  drivers/net/qede/base/ecore_dev.c                  |   4 +-
>  drivers/net/qede/qede_ethdev.c                     |  58 ++--
>  drivers/net/qede/qede_ethdev.h                     |   6 +-
>  drivers/net/qede/qede_filter.c                     |  66 ++---
>  drivers/net/qede/qede_if.h                         |   4 +-
>  drivers/net/qede/qede_main.c                       |   6 +-
>  drivers/net/qede/qede_rxtx.c                       |  32 +-
>  drivers/net/qede/qede_rxtx.h                       |   2 +-
>  drivers/net/ring/rte_eth_ring.c                    |   4 +-
>  drivers/net/sfc/sfc.h                              |   2 +-
>  drivers/net/sfc/sfc_ef10_tx.c                      |   8 +-
>  drivers/net/sfc/sfc_ethdev.c                       |  20 +-
>  drivers/net/sfc/sfc_flow.c                         |  12 +-
>  drivers/net/sfc/sfc_port.c                         |   8 +-
>  drivers/net/sfc/sfc_tso.c                          |   8 +-
>  drivers/net/softnic/parser.c                       |  18 +-
>  drivers/net/softnic/parser.h                       |   2 +-
>  drivers/net/softnic/rte_eth_softnic.c              |   2 +-
>  drivers/net/softnic/rte_eth_softnic_pipeline.c     |  40 +--
>  drivers/net/szedata2/rte_eth_szedata2.c            |   8 +-
>  drivers/net/tap/rte_eth_tap.c                      |  58 ++--
>  drivers/net/tap/rte_eth_tap.h                      |   2 +-
>  drivers/net/tap/tap_bpf_program.c                  |  14 +-
>  drivers/net/tap/tap_flow.c                         |  12 +-
>  drivers/net/thunderx/base/nicvf_mbox.c             |   4 +-
>  drivers/net/thunderx/base/nicvf_plat.h             |   2 +-
>  drivers/net/thunderx/nicvf_ethdev.c                |  18 +-
>  drivers/net/thunderx/nicvf_struct.h                |   2 +-
>  drivers/net/vdev_netvsc/vdev_netvsc.c              |  16 +-
>  drivers/net/vhost/rte_eth_vhost.c                  |  12 +-
>  drivers/net/virtio/virtio_ethdev.c                 |  70 ++---
>  drivers/net/virtio/virtio_pci.h                    |   4 +-
>  drivers/net/virtio/virtio_rxtx.c                   |  28 +-
>  drivers/net/virtio/virtio_user/vhost_kernel_tap.c  |   2 +-
>  drivers/net/virtio/virtio_user/virtio_user_dev.c   |   6 +-
>  drivers/net/virtio/virtio_user/virtio_user_dev.h   |   2 +-
>  drivers/net/virtio/virtio_user_ethdev.c            |   8 +-
>  drivers/net/virtio/virtqueue.h                     |   2 +-
>  drivers/net/vmxnet3/vmxnet3_ethdev.c               |  12 +-
>  drivers/net/vmxnet3/vmxnet3_ethdev.h               |   2 +-
>  drivers/net/vmxnet3/vmxnet3_rxtx.c                 |  44 +--
>  examples/bbdev_app/main.c                          |  40 +--
>  examples/bond/main.c                               |  78 ++---
>  examples/distributor/main.c                        |   4 +-
>  examples/ethtool/ethtool-app/ethapp.c              |   8 +-
>  examples/ethtool/ethtool-app/main.c                |  10 +-
>  examples/ethtool/lib/rte_ethtool.c                 |   8 +-
>  examples/ethtool/lib/rte_ethtool.h                 |   6 +-
>  examples/eventdev_pipeline/main.c                  |   4 +-
>  examples/eventdev_pipeline/pipeline_common.h       |  10 +-
>  examples/flow_classify/flow_classify.c             |  30 +-
>  examples/flow_filtering/main.c                     |  10 +-
>  examples/ip_fragmentation/main.c                   |  62 ++--
>  examples/ip_pipeline/cli.c                         |   2 +-
>  examples/ip_pipeline/kni.c                         |   2 +-
>  examples/ip_pipeline/parser.c                      |  18 +-
>  examples/ip_pipeline/parser.h                      |   2 +-
>  examples/ip_pipeline/pipeline.c                    |  40 +--
>  examples/ip_reassembly/main.c                      |  50 ++--
>  examples/ipsec-secgw/esp.c                         |  42 +--
>  examples/ipsec-secgw/ipsec-secgw.c                 |  38 +--
>  examples/ipsec-secgw/sa.c                          |   6 +-
>  examples/ipv4_multicast/main.c                     |  58 ++--
>  examples/kni/main.c                                |  14 +-
>  examples/l2fwd-cat/l2fwd-cat.c                     |   4 +-
>  examples/l2fwd-crypto/main.c                       |  26 +-
>  examples/l2fwd-jobstats/main.c                     |   8 +-
>  examples/l2fwd-keepalive/main.c                    |   8 +-
>  examples/l2fwd/main.c                              |   8 +-
>  examples/l3fwd-acl/main.c                          | 102 +++----
>  examples/l3fwd-power/main.c                        | 100 +++----
>  examples/l3fwd-vf/main.c                           |  68 ++---
>  examples/l3fwd/l3fwd.h                             |   8 +-
>  examples/l3fwd/l3fwd_altivec.h                     |  14 +-
>  examples/l3fwd/l3fwd_common.h                      |   4 +-
>  examples/l3fwd/l3fwd_em.c                          |  44 +--
>  examples/l3fwd/l3fwd_em.h                          |  20 +-
>  examples/l3fwd/l3fwd_em_hlm.h                      |  16 +-
>  examples/l3fwd/l3fwd_em_hlm_neon.h                 |  16 +-
>  examples/l3fwd/l3fwd_em_hlm_sse.h                  |  16 +-
>  examples/l3fwd/l3fwd_em_sequential.h               |  16 +-
>  examples/l3fwd/l3fwd_lpm.c                         |  50 ++--
>  examples/l3fwd/l3fwd_lpm.h                         |  20 +-
>  examples/l3fwd/l3fwd_lpm_altivec.h                 |  20 +-
>  examples/l3fwd/l3fwd_lpm_neon.h                    |  30 +-
>  examples/l3fwd/l3fwd_lpm_sse.h                     |  20 +-
>  examples/l3fwd/l3fwd_neon.h                        |  14 +-
>  examples/l3fwd/l3fwd_sse.h                         |  14 +-
>  examples/l3fwd/main.c                              |  20 +-
>  examples/link_status_interrupt/main.c              |   8 +-
>  examples/load_balancer/runtime.c                   |   6 +-
>  .../client_server_mp/mp_server/main.c              |   2 +-
>  examples/packet_ordering/main.c                    |   2 +-
>  examples/performance-thread/l3fwd-thread/main.c    | 322 ++++++++++-----------
>  examples/ptpclient/ptpclient.c                     |  32 +-
>  examples/qos_meter/main.c                          |   4 +-
>  examples/qos_sched/init.c                          |   2 +-
>  examples/quota_watermark/qw/main.c                 |   8 +-
>  examples/rxtx_callbacks/main.c                     |   4 +-
>  examples/server_node_efd/node/node.c               |   6 +-
>  examples/server_node_efd/server/main.c             |   8 +-
>  examples/skeleton/basicfwd.c                       |   4 +-
>  examples/tep_termination/main.c                    |   2 +-
>  examples/tep_termination/main.h                    |   2 +-
>  examples/tep_termination/vxlan.c                   | 108 +++----
>  examples/tep_termination/vxlan.h                   |   8 +-
>  examples/tep_termination/vxlan_setup.c             |  30 +-
>  examples/tep_termination/vxlan_setup.h             |   2 +-
>  examples/vhost/main.c                              |  40 +--
>  examples/vhost/main.h                              |   2 +-
>  examples/vm_power_manager/channel_monitor.c        |   2 +-
>  .../guest_cli/vm_power_cli_guest.c                 |   2 +-
>  examples/vm_power_manager/main.c                   |   6 +-
>  examples/vmdq/main.c                               |  12 +-
>  examples/vmdq_dcb/main.c                           |  12 +-
>  lib/librte_cmdline/cmdline_parse_etheraddr.c       |  33 +--
>  lib/librte_ethdev/rte_eth_ctrl.h                   |  12 +-
>  lib/librte_ethdev/rte_ethdev.c                     |  56 ++--
>  lib/librte_ethdev/rte_ethdev.h                     |  12 +-
>  lib/librte_ethdev/rte_ethdev_core.h                |  12 +-
>  lib/librte_ethdev/rte_flow.h                       |  32 +-
>  lib/librte_eventdev/rte_event_eth_rx_adapter.c     |  32 +-
>  lib/librte_gro/gro_tcp4.c                          |  26 +-
>  lib/librte_gro/gro_tcp4.h                          |  20 +-
>  lib/librte_gro/gro_vxlan_tcp4.c                    |  64 ++--
>  lib/librte_gro/gro_vxlan_tcp4.h                    |   6 +-
>  lib/librte_gso/gso_common.h                        |  16 +-
>  lib/librte_gso/gso_tcp4.c                          |  12 +-
>  lib/librte_gso/gso_tunnel_tcp4.c                   |  14 +-
>  lib/librte_gso/gso_udp4.c                          |   8 +-
>  lib/librte_gso/rte_gso.h                           |   8 +-
>  lib/librte_hash/rte_thash.h                        |   2 +-
>  lib/librte_ip_frag/rte_ip_frag.h                   |  12 +-
>  lib/librte_ip_frag/rte_ipv4_fragmentation.c        |  42 +--
>  lib/librte_ip_frag/rte_ipv4_reassembly.c           |  14 +-
>  lib/librte_ip_frag/rte_ipv6_fragmentation.c        |  26 +-
>  lib/librte_ip_frag/rte_ipv6_reassembly.c           |   6 +-
>  lib/librte_kni/rte_kni.c                           |   4 +-
>  lib/librte_kni/rte_kni.h                           |   2 +-
>  lib/librte_net/rte_arp.c                           |  32 +-
>  lib/librte_net/rte_arp.h                           |  36 +--
>  lib/librte_net/rte_esp.h                           |   2 +-
>  lib/librte_net/rte_ether.h                         | 178 ++++++------
>  lib/librte_net/rte_gre.h                           |   2 +-
>  lib/librte_net/rte_icmp.h                          |   6 +-
>  lib/librte_net/rte_ip.h                            |  70 ++---
>  lib/librte_net/rte_net.c                           |  90 +++---
>  lib/librte_net/rte_net.h                           |  22 +-
>  lib/librte_net/rte_sctp.h                          |   2 +-
>  lib/librte_net/rte_tcp.h                           |   2 +-
>  lib/librte_net/rte_udp.h                           |   2 +-
>  lib/librte_pipeline/rte_table_action.c             | 210 +++++++-------
>  lib/librte_pipeline/rte_table_action.h             |   4 +-
>  lib/librte_port/rte_port_ras.c                     |   8 +-
>  lib/librte_port/rte_port_source_sink.c             |   6 +-
>  lib/librte_vhost/vhost.h                           |   2 +-
>  lib/librte_vhost/virtio_net.c                      |  42 +--
>  test/test-acl/main.c                               |   2 +-
>  test/test-pipeline/pipeline_acl.c                  |  16 +-
>  test/test-pipeline/pipeline_hash.c                 |  12 +-
>  test/test/packet_burst_generator.c                 | 126 ++++----
>  test/test/packet_burst_generator.h                 |  26 +-
>  test/test/test_acl.c                               |   8 +-
>  test/test/test_acl.h                               | 122 ++++----
>  test/test/test_cmdline_etheraddr.c                 |  16 +-
>  test/test/test_efd.c                               |  20 +-
>  test/test/test_event_eth_rx_adapter.c              |   2 +-
>  test/test/test_event_eth_tx_adapter.c              |   2 +-
>  test/test/test_flow_classify.c                     |  68 ++---
>  test/test/test_hash.c                              |  20 +-
>  test/test/test_link_bonding.c                      | 284 +++++++++---------
>  test/test/test_link_bonding_mode4.c                | 116 ++++----
>  test/test/test_link_bonding_rssconf.c              |   6 +-
>  test/test/test_lpm.c                               |  76 ++---
>  test/test/test_lpm_perf.c                          |  10 +-
>  test/test/test_member.c                            |  20 +-
>  test/test/test_pmd_perf.c                          |  20 +-
>  test/test/test_sched.c                             |  20 +-
>  test/test/test_table_acl.c                         |   8 +-
>  test/test/test_thash.c                             |  12 +-
>  test/test/virtual_pmd.c                            |   6 +-
>  test/test/virtual_pmd.h                            |   2 +-
>  367 files changed, 3906 insertions(+), 3913 deletions(-)
> 

The Linux network developers and Glibc have already agreed on how to handle
overlap. Perhaps that policy could be used/extended rather than breaking
every userspace application.
  
Bruce Richardson Oct. 24, 2018, 4:39 p.m. UTC | #4
On Wed, Oct 24, 2018 at 10:18:19AM +0200, Olivier Matz wrote:
> This RFC targets 19.02.
> 
> The rte_net headers conflict with the libc headers, because
> some definitions are duplicated, sometimes with few differences.
> This was discussed in [1], and more recently at the techboard.
> 
> Before sending the deprecation notice (target for this is 18.11),
> here is a draft that can be discussed.
> 
> This RFC 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.
> 
> One question I'm asking is how can we manage the transition.
> Initially, I hoped it was possible to have a compat layer during
> one release (supporting both prefixed and unprefixed names), but
> now that the patch is done, it seems the impact is too big, and
> impacts too many libraries.
> 
> Few examples:
>   - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *)
>   - many rte_flow structures use the rte_ prefixed net structures
>   - the mac field of virtio_net structure is rte_ether_addr
>   - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *)
>   ...
> 
> Therefore, it is clear that doing this would break the compilation
> of many external applications.
> 

Can you clarify a bit as to why we can't keep around compatibility versions
of the headers, alongside the new versions? I'm not following the logic
above. Can we not introduce completely new headers with the replacements
while leaving the old ones intact?

/Bruce
  
Stephen Hemminger Oct. 24, 2018, 6:38 p.m. UTC | #5
On Wed, 24 Oct 2018 10:18:19 +0200
Olivier Matz <olivier.matz@6wind.com> wrote:

> This RFC targets 19.02.
> 
> The rte_net headers conflict with the libc headers, because
> some definitions are duplicated, sometimes with few differences.
> This was discussed in [1], and more recently at the techboard.
> 
> Before sending the deprecation notice (target for this is 18.11),
> here is a draft that can be discussed.
> 
> This RFC 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.
> 
> One question I'm asking is how can we manage the transition.
> Initially, I hoped it was possible to have a compat layer during
> one release (supporting both prefixed and unprefixed names), but
> now that the patch is done, it seems the impact is too big, and
> impacts too many libraries.
> 
> Few examples:
>   - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *)
>   - many rte_flow structures use the rte_ prefixed net structures
>   - the mac field of virtio_net structure is rte_ether_addr
>   - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *)
>   ...
> 
> Therefore, it is clear that doing this would break the compilation
> of many external applications.
> 
> Another drawback we need to take in account: it will make the
> backport of patches more difficult, although this is something
> that could be tempered by a script.
> 
> While it is obviously better to have a good namespace convention, 
> we need to identify the issues we have today before deciding it's
> worth doing the change.
> 
> Comments?
> 
> 
> Things that are missing in RFC:
> - test with FreeBSD
> - manually fix some indentation issues
> 
> 
> Olivier Matz (14):
>   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
> 
>  app/pdump/main.c                                   |   2 +-
>  app/test-eventdev/test_perf_common.c               |   2 +-
>  app/test-eventdev/test_pipeline_common.c           |   2 +-
>  app/test-pmd/cmdline.c                             |  66 ++---
>  app/test-pmd/cmdline_flow.c                        |  10 +-
>  app/test-pmd/config.c                              |  34 +--
>  app/test-pmd/csumonly.c                            | 156 +++++-----
>  app/test-pmd/flowgen.c                             |  30 +-
>  app/test-pmd/icmpecho.c                            | 120 ++++----
>  app/test-pmd/ieee1588fwd.c                         |  18 +-
>  app/test-pmd/macfwd.c                              |  12 +-
>  app/test-pmd/macswap.c                             |  16 +-
>  app/test-pmd/parameters.c                          |   6 +-
>  app/test-pmd/testpmd.c                             |  22 +-
>  app/test-pmd/testpmd.h                             |  18 +-
>  app/test-pmd/txonly.c                              |  36 +--
>  app/test-pmd/util.c                                |  34 +--
>  doc/guides/prog_guide/bbdev.rst                    |   6 +-
>  .../prog_guide/packet_classif_access_ctrl.rst      |  18 +-
>  doc/guides/prog_guide/rte_flow.rst                 |   4 +-
>  doc/guides/sample_app_ug/flow_classify.rst         |  28 +-
>  doc/guides/sample_app_ug/flow_filtering.rst        |   6 +-
>  doc/guides/sample_app_ug/ip_frag.rst               |  16 +-
>  doc/guides/sample_app_ug/ip_reassembly.rst         |  16 +-
>  doc/guides/sample_app_ug/ipv4_multicast.rst        |  16 +-
>  doc/guides/sample_app_ug/l2_forward_job_stats.rst  |   6 +-
>  .../sample_app_ug/l2_forward_real_virtual.rst      |   6 +-
>  doc/guides/sample_app_ug/l3_forward.rst            |  12 +-
>  doc/guides/sample_app_ug/link_status_intr.rst      |   6 +-
>  doc/guides/sample_app_ug/ptpclient.rst             |   6 +-
>  doc/guides/sample_app_ug/rxtx_callbacks.rst        |   2 +-
>  doc/guides/sample_app_ug/server_node_efd.rst       |  12 +-
>  doc/guides/sample_app_ug/skeleton.rst              |   4 +-
>  doc/guides/sample_app_ug/vmdq_dcb_forwarding.rst   |   4 +-
>  drivers/bus/dpaa/base/fman/fman.c                  |   2 +-
>  drivers/bus/dpaa/base/fman/fman_hw.c               |   2 +-
>  drivers/bus/dpaa/include/fman.h                    |   2 +-
>  drivers/bus/dpaa/include/netcfg.h                  |   4 +-
>  drivers/net/af_packet/rte_eth_af_packet.c          |   2 +-
>  drivers/net/ark/ark_ethdev.c                       |  16 +-
>  drivers/net/ark/ark_ext.h                          |   4 +-
>  drivers/net/ark/ark_global.h                       |   4 +-
>  drivers/net/atlantic/atl_ethdev.c                  |  20 +-
>  drivers/net/atlantic/hw_atl/hw_atl_utils.c         |   8 +-
>  drivers/net/atlantic/hw_atl/hw_atl_utils_fw2x.c    |   4 +-
>  drivers/net/avf/avf.h                              |   4 +-
>  drivers/net/avf/avf_ethdev.c                       |  50 ++--
>  drivers/net/avf/avf_rxtx.c                         |  14 +-
>  drivers/net/avf/avf_vchnl.c                        |   8 +-
>  drivers/net/avf/base/avf_adminq_cmd.h              |   4 +-
>  drivers/net/avf/base/avf_common.c                  |  12 +-
>  drivers/net/avf/base/avf_prototype.h               |   4 +-
>  drivers/net/avp/avp_ethdev.c                       |  20 +-
>  drivers/net/avp/rte_avp_common.h                   |   2 +-
>  drivers/net/axgbe/axgbe_dev.c                      |   4 +-
>  drivers/net/axgbe/axgbe_ethdev.c                   |  10 +-
>  drivers/net/axgbe/axgbe_ethdev.h                   |   4 +-
>  drivers/net/axgbe/axgbe_rxtx.c                     |   2 +-
>  drivers/net/bnx2x/bnx2x.c                          |  16 +-
>  drivers/net/bnx2x/bnx2x_ethdev.c                   |   4 +-
>  drivers/net/bnx2x/bnx2x_ethdev.h                   |   2 +-
>  drivers/net/bnx2x/bnx2x_vfpf.c                     |   8 +-
>  drivers/net/bnx2x/bnx2x_vfpf.h                     |   2 +-
>  drivers/net/bnx2x/ecore_sp.h                       |   2 +-
>  drivers/net/bnxt/bnxt.h                            |   4 +-
>  drivers/net/bnxt/bnxt_ethdev.c                     |  70 ++---
>  drivers/net/bnxt/bnxt_filter.c                     |   4 +-
>  drivers/net/bnxt/bnxt_filter.h                     |   8 +-
>  drivers/net/bnxt/bnxt_flow.c                       |  26 +-
>  drivers/net/bnxt/bnxt_hwrm.c                       |  40 +--
>  drivers/net/bnxt/bnxt_hwrm.h                       |   2 +-
>  drivers/net/bnxt/bnxt_ring.c                       |   8 +-
>  drivers/net/bnxt/bnxt_rxq.c                        |   2 +-
>  drivers/net/bnxt/bnxt_rxr.c                        |   2 +-
>  drivers/net/bnxt/bnxt_vnic.c                       |   2 +-
>  drivers/net/bnxt/rte_pmd_bnxt.c                    |  14 +-
>  drivers/net/bnxt/rte_pmd_bnxt.h                    |   4 +-
>  drivers/net/bonding/rte_eth_bond.h                 |   2 +-
>  drivers/net/bonding/rte_eth_bond_8023ad.c          |  26 +-
>  drivers/net/bonding/rte_eth_bond_8023ad.h          |  10 +-
>  drivers/net/bonding/rte_eth_bond_alb.c             |  78 ++---
>  drivers/net/bonding/rte_eth_bond_alb.h             |  10 +-
>  drivers/net/bonding/rte_eth_bond_api.c             |   2 +-
>  drivers/net/bonding/rte_eth_bond_args.c            |   2 +-
>  drivers/net/bonding/rte_eth_bond_pmd.c             | 194 ++++++-------
>  drivers/net/bonding/rte_eth_bond_private.h         |   6 +-
>  drivers/net/cxgbe/base/adapter.h                   |   6 +-
>  drivers/net/cxgbe/base/t4_hw.c                     |   8 +-
>  drivers/net/cxgbe/cxgbe.h                          |   4 +-
>  drivers/net/cxgbe/cxgbe_ethdev.c                   |  14 +-
>  drivers/net/cxgbe/cxgbe_filter.h                   |   2 +-
>  drivers/net/cxgbe/cxgbe_flow.c                     |  10 +-
>  drivers/net/cxgbe/cxgbe_main.c                     |   4 +-
>  drivers/net/cxgbe/cxgbe_pfvf.h                     |   2 +-
>  drivers/net/cxgbe/cxgbevf_main.c                   |   2 +-
>  drivers/net/cxgbe/l2t.c                            |   8 +-
>  drivers/net/cxgbe/l2t.h                            |   2 +-
>  drivers/net/cxgbe/mps_tcam.c                       |  14 +-
>  drivers/net/cxgbe/mps_tcam.h                       |   4 +-
>  drivers/net/cxgbe/sge.c                            |   8 +-
>  drivers/net/dpaa/dpaa_ethdev.c                     |  20 +-
>  drivers/net/dpaa/dpaa_rxtx.c                       |  22 +-
>  drivers/net/dpaa2/dpaa2_ethdev.c                   |  36 +--
>  drivers/net/e1000/e1000_ethdev.h                   |   2 +-
>  drivers/net/e1000/em_ethdev.c                      |  34 +--
>  drivers/net/e1000/em_rxtx.c                        |  22 +-
>  drivers/net/e1000/igb_ethdev.c                     |  70 ++---
>  drivers/net/e1000/igb_flow.c                       |  12 +-
>  drivers/net/e1000/igb_pf.c                         |  16 +-
>  drivers/net/e1000/igb_rxtx.c                       |  18 +-
>  drivers/net/ena/ena_ethdev.c                       |  16 +-
>  drivers/net/ena/ena_ethdev.h                       |   2 +-
>  drivers/net/enetc/base/enetc_hw.h                  |   4 +-
>  drivers/net/enetc/enetc_ethdev.c                   |   6 +-
>  drivers/net/enic/enic.h                            |   2 +-
>  drivers/net/enic/enic_clsf.c                       |  40 +--
>  drivers/net/enic/enic_ethdev.c                     |   4 +-
>  drivers/net/enic/enic_flow.c                       | 100 +++----
>  drivers/net/enic/enic_main.c                       |   2 +-
>  drivers/net/enic/enic_res.c                        |   4 +-
>  drivers/net/failsafe/failsafe.c                    |   6 +-
>  drivers/net/failsafe/failsafe_args.c               |   4 +-
>  drivers/net/failsafe/failsafe_ether.c              |   6 +-
>  drivers/net/failsafe/failsafe_ops.c                |   6 +-
>  drivers/net/failsafe/failsafe_private.h            |   4 +-
>  drivers/net/fm10k/fm10k.h                          |   2 +-
>  drivers/net/fm10k/fm10k_ethdev.c                   |  18 +-
>  drivers/net/i40e/base/i40e_adminq_cmd.h            |   4 +-
>  drivers/net/i40e/base/i40e_common.c                |  12 +-
>  drivers/net/i40e/base/i40e_prototype.h             |   4 +-
>  drivers/net/i40e/i40e_ethdev.c                     | 134 ++++-----
>  drivers/net/i40e/i40e_ethdev.h                     |  22 +-
>  drivers/net/i40e/i40e_ethdev_vf.c                  |  60 ++--
>  drivers/net/i40e/i40e_fdir.c                       | 126 ++++----
>  drivers/net/i40e/i40e_flow.c                       |  58 ++--
>  drivers/net/i40e/i40e_pf.c                         |  18 +-
>  drivers/net/i40e/i40e_rxtx.c                       |  28 +-
>  drivers/net/i40e/i40e_vf_representor.c             |   2 +-
>  drivers/net/i40e/rte_pmd_i40e.c                    |  30 +-
>  drivers/net/i40e/rte_pmd_i40e.h                    |   8 +-
>  drivers/net/ixgbe/ixgbe_ethdev.c                   |  94 +++---
>  drivers/net/ixgbe/ixgbe_ethdev.h                   |   2 +-
>  drivers/net/ixgbe/ixgbe_flow.c                     |  22 +-
>  drivers/net/ixgbe/ixgbe_pf.c                       |  14 +-
>  drivers/net/ixgbe/ixgbe_rxtx.c                     |  14 +-
>  drivers/net/ixgbe/ixgbe_vf_representor.c           |   4 +-
>  drivers/net/ixgbe/rte_pmd_ixgbe.c                  |  10 +-
>  drivers/net/ixgbe/rte_pmd_ixgbe.h                  |   2 +-
>  drivers/net/kni/rte_eth_kni.c                      |   4 +-
>  drivers/net/liquidio/lio_ethdev.c                  |  22 +-
>  drivers/net/mlx4/mlx4.c                            |   4 +-
>  drivers/net/mlx4/mlx4.h                            |   8 +-
>  drivers/net/mlx4/mlx4_ethdev.c                     |   8 +-
>  drivers/net/mlx4/mlx4_flow.c                       |  14 +-
>  drivers/net/mlx4/mlx4_rxtx.c                       |   2 +-
>  drivers/net/mlx5/mlx5.c                            |   4 +-
>  drivers/net/mlx5/mlx5.h                            |  14 +-
>  drivers/net/mlx5/mlx5_flow.c                       |  22 +-
>  drivers/net/mlx5/mlx5_flow_tcf.c                   |  40 +--
>  drivers/net/mlx5/mlx5_flow_verbs.c                 |  26 +-
>  drivers/net/mlx5/mlx5_mac.c                        |  18 +-
>  drivers/net/mlx5/mlx5_nl.c                         |  28 +-
>  drivers/net/mlx5/mlx5_rxtx.c                       |   6 +-
>  drivers/net/mlx5/mlx5_rxtx.h                       |   2 +-
>  drivers/net/mlx5/mlx5_rxtx_vec_neon.h              |   8 +-
>  drivers/net/mlx5/mlx5_rxtx_vec_sse.h               |  10 +-
>  drivers/net/mlx5/mlx5_trigger.c                    |   6 +-
>  drivers/net/mvneta/mvneta_ethdev.c                 |  22 +-
>  drivers/net/mvneta/mvneta_ethdev.h                 |   2 +-
>  drivers/net/mvpp2/mrvl_ethdev.c                    |  22 +-
>  drivers/net/mvpp2/mrvl_ethdev.h                    |   2 +-
>  drivers/net/mvpp2/mrvl_flow.c                      |   4 +-
>  drivers/net/netvsc/hn_ethdev.c                     |   4 +-
>  drivers/net/netvsc/hn_nvs.c                        |   2 +-
>  drivers/net/netvsc/hn_rndis.c                      |   2 +-
>  drivers/net/netvsc/hn_rxtx.c                       |  12 +-
>  drivers/net/netvsc/hn_var.h                        |   4 +-
>  drivers/net/netvsc/hn_vf.c                         |  12 +-
>  drivers/net/nfp/nfp_net.c                          |  20 +-
>  drivers/net/nfp/nfp_net_pmd.h                      |   2 +-
>  drivers/net/null/rte_eth_null.c                    |   6 +-
>  drivers/net/octeontx/octeontx_ethdev.c             |   8 +-
>  drivers/net/octeontx/octeontx_ethdev.h             |   2 +-
>  drivers/net/pcap/rte_eth_pcap.c                    |  22 +-
>  drivers/net/qede/base/bcm_osal.h                   |   2 +-
>  drivers/net/qede/base/ecore_dev.c                  |   4 +-
>  drivers/net/qede/qede_ethdev.c                     |  58 ++--
>  drivers/net/qede/qede_ethdev.h                     |   6 +-
>  drivers/net/qede/qede_filter.c                     |  66 ++---
>  drivers/net/qede/qede_if.h                         |   4 +-
>  drivers/net/qede/qede_main.c                       |   6 +-
>  drivers/net/qede/qede_rxtx.c                       |  32 +-
>  drivers/net/qede/qede_rxtx.h                       |   2 +-
>  drivers/net/ring/rte_eth_ring.c                    |   4 +-
>  drivers/net/sfc/sfc.h                              |   2 +-
>  drivers/net/sfc/sfc_ef10_tx.c                      |   8 +-
>  drivers/net/sfc/sfc_ethdev.c                       |  20 +-
>  drivers/net/sfc/sfc_flow.c                         |  12 +-
>  drivers/net/sfc/sfc_port.c                         |   8 +-
>  drivers/net/sfc/sfc_tso.c                          |   8 +-
>  drivers/net/softnic/parser.c                       |  18 +-
>  drivers/net/softnic/parser.h                       |   2 +-
>  drivers/net/softnic/rte_eth_softnic.c              |   2 +-
>  drivers/net/softnic/rte_eth_softnic_pipeline.c     |  40 +--
>  drivers/net/szedata2/rte_eth_szedata2.c            |   8 +-
>  drivers/net/tap/rte_eth_tap.c                      |  58 ++--
>  drivers/net/tap/rte_eth_tap.h                      |   2 +-
>  drivers/net/tap/tap_bpf_program.c                  |  14 +-
>  drivers/net/tap/tap_flow.c                         |  12 +-
>  drivers/net/thunderx/base/nicvf_mbox.c             |   4 +-
>  drivers/net/thunderx/base/nicvf_plat.h             |   2 +-
>  drivers/net/thunderx/nicvf_ethdev.c                |  18 +-
>  drivers/net/thunderx/nicvf_struct.h                |   2 +-
>  drivers/net/vdev_netvsc/vdev_netvsc.c              |  16 +-
>  drivers/net/vhost/rte_eth_vhost.c                  |  12 +-
>  drivers/net/virtio/virtio_ethdev.c                 |  70 ++---
>  drivers/net/virtio/virtio_pci.h                    |   4 +-
>  drivers/net/virtio/virtio_rxtx.c                   |  28 +-
>  drivers/net/virtio/virtio_user/vhost_kernel_tap.c  |   2 +-
>  drivers/net/virtio/virtio_user/virtio_user_dev.c   |   6 +-
>  drivers/net/virtio/virtio_user/virtio_user_dev.h   |   2 +-
>  drivers/net/virtio/virtio_user_ethdev.c            |   8 +-
>  drivers/net/virtio/virtqueue.h                     |   2 +-
>  drivers/net/vmxnet3/vmxnet3_ethdev.c               |  12 +-
>  drivers/net/vmxnet3/vmxnet3_ethdev.h               |   2 +-
>  drivers/net/vmxnet3/vmxnet3_rxtx.c                 |  44 +--
>  examples/bbdev_app/main.c                          |  40 +--
>  examples/bond/main.c                               |  78 ++---
>  examples/distributor/main.c                        |   4 +-
>  examples/ethtool/ethtool-app/ethapp.c              |   8 +-
>  examples/ethtool/ethtool-app/main.c                |  10 +-
>  examples/ethtool/lib/rte_ethtool.c                 |   8 +-
>  examples/ethtool/lib/rte_ethtool.h                 |   6 +-
>  examples/eventdev_pipeline/main.c                  |   4 +-
>  examples/eventdev_pipeline/pipeline_common.h       |  10 +-
>  examples/flow_classify/flow_classify.c             |  30 +-
>  examples/flow_filtering/main.c                     |  10 +-
>  examples/ip_fragmentation/main.c                   |  62 ++--
>  examples/ip_pipeline/cli.c                         |   2 +-
>  examples/ip_pipeline/kni.c                         |   2 +-
>  examples/ip_pipeline/parser.c                      |  18 +-
>  examples/ip_pipeline/parser.h                      |   2 +-
>  examples/ip_pipeline/pipeline.c                    |  40 +--
>  examples/ip_reassembly/main.c                      |  50 ++--
>  examples/ipsec-secgw/esp.c                         |  42 +--
>  examples/ipsec-secgw/ipsec-secgw.c                 |  38 +--
>  examples/ipsec-secgw/sa.c                          |   6 +-
>  examples/ipv4_multicast/main.c                     |  58 ++--
>  examples/kni/main.c                                |  14 +-
>  examples/l2fwd-cat/l2fwd-cat.c                     |   4 +-
>  examples/l2fwd-crypto/main.c                       |  26 +-
>  examples/l2fwd-jobstats/main.c                     |   8 +-
>  examples/l2fwd-keepalive/main.c                    |   8 +-
>  examples/l2fwd/main.c                              |   8 +-
>  examples/l3fwd-acl/main.c                          | 102 +++----
>  examples/l3fwd-power/main.c                        | 100 +++----
>  examples/l3fwd-vf/main.c                           |  68 ++---
>  examples/l3fwd/l3fwd.h                             |   8 +-
>  examples/l3fwd/l3fwd_altivec.h                     |  14 +-
>  examples/l3fwd/l3fwd_common.h                      |   4 +-
>  examples/l3fwd/l3fwd_em.c                          |  44 +--
>  examples/l3fwd/l3fwd_em.h                          |  20 +-
>  examples/l3fwd/l3fwd_em_hlm.h                      |  16 +-
>  examples/l3fwd/l3fwd_em_hlm_neon.h                 |  16 +-
>  examples/l3fwd/l3fwd_em_hlm_sse.h                  |  16 +-
>  examples/l3fwd/l3fwd_em_sequential.h               |  16 +-
>  examples/l3fwd/l3fwd_lpm.c                         |  50 ++--
>  examples/l3fwd/l3fwd_lpm.h                         |  20 +-
>  examples/l3fwd/l3fwd_lpm_altivec.h                 |  20 +-
>  examples/l3fwd/l3fwd_lpm_neon.h                    |  30 +-
>  examples/l3fwd/l3fwd_lpm_sse.h                     |  20 +-
>  examples/l3fwd/l3fwd_neon.h                        |  14 +-
>  examples/l3fwd/l3fwd_sse.h                         |  14 +-
>  examples/l3fwd/main.c                              |  20 +-
>  examples/link_status_interrupt/main.c              |   8 +-
>  examples/load_balancer/runtime.c                   |   6 +-
>  .../client_server_mp/mp_server/main.c              |   2 +-
>  examples/packet_ordering/main.c                    |   2 +-
>  examples/performance-thread/l3fwd-thread/main.c    | 322 ++++++++++-----------
>  examples/ptpclient/ptpclient.c                     |  32 +-
>  examples/qos_meter/main.c                          |   4 +-
>  examples/qos_sched/init.c                          |   2 +-
>  examples/quota_watermark/qw/main.c                 |   8 +-
>  examples/rxtx_callbacks/main.c                     |   4 +-
>  examples/server_node_efd/node/node.c               |   6 +-
>  examples/server_node_efd/server/main.c             |   8 +-
>  examples/skeleton/basicfwd.c                       |   4 +-
>  examples/tep_termination/main.c                    |   2 +-
>  examples/tep_termination/main.h                    |   2 +-
>  examples/tep_termination/vxlan.c                   | 108 +++----
>  examples/tep_termination/vxlan.h                   |   8 +-
>  examples/tep_termination/vxlan_setup.c             |  30 +-
>  examples/tep_termination/vxlan_setup.h             |   2 +-
>  examples/vhost/main.c                              |  40 +--
>  examples/vhost/main.h                              |   2 +-
>  examples/vm_power_manager/channel_monitor.c        |   2 +-
>  .../guest_cli/vm_power_cli_guest.c                 |   2 +-
>  examples/vm_power_manager/main.c                   |   6 +-
>  examples/vmdq/main.c                               |  12 +-
>  examples/vmdq_dcb/main.c                           |  12 +-
>  lib/librte_cmdline/cmdline_parse_etheraddr.c       |  33 +--
>  lib/librte_ethdev/rte_eth_ctrl.h                   |  12 +-
>  lib/librte_ethdev/rte_ethdev.c                     |  56 ++--
>  lib/librte_ethdev/rte_ethdev.h                     |  12 +-
>  lib/librte_ethdev/rte_ethdev_core.h                |  12 +-
>  lib/librte_ethdev/rte_flow.h                       |  32 +-
>  lib/librte_eventdev/rte_event_eth_rx_adapter.c     |  32 +-
>  lib/librte_gro/gro_tcp4.c                          |  26 +-
>  lib/librte_gro/gro_tcp4.h                          |  20 +-
>  lib/librte_gro/gro_vxlan_tcp4.c                    |  64 ++--
>  lib/librte_gro/gro_vxlan_tcp4.h                    |   6 +-
>  lib/librte_gso/gso_common.h                        |  16 +-
>  lib/librte_gso/gso_tcp4.c                          |  12 +-
>  lib/librte_gso/gso_tunnel_tcp4.c                   |  14 +-
>  lib/librte_gso/gso_udp4.c                          |   8 +-
>  lib/librte_gso/rte_gso.h                           |   8 +-
>  lib/librte_hash/rte_thash.h                        |   2 +-
>  lib/librte_ip_frag/rte_ip_frag.h                   |  12 +-
>  lib/librte_ip_frag/rte_ipv4_fragmentation.c        |  42 +--
>  lib/librte_ip_frag/rte_ipv4_reassembly.c           |  14 +-
>  lib/librte_ip_frag/rte_ipv6_fragmentation.c        |  26 +-
>  lib/librte_ip_frag/rte_ipv6_reassembly.c           |   6 +-
>  lib/librte_kni/rte_kni.c                           |   4 +-
>  lib/librte_kni/rte_kni.h                           |   2 +-
>  lib/librte_net/rte_arp.c                           |  32 +-
>  lib/librte_net/rte_arp.h                           |  36 +--
>  lib/librte_net/rte_esp.h                           |   2 +-
>  lib/librte_net/rte_ether.h                         | 178 ++++++------
>  lib/librte_net/rte_gre.h                           |   2 +-
>  lib/librte_net/rte_icmp.h                          |   6 +-
>  lib/librte_net/rte_ip.h                            |  70 ++---
>  lib/librte_net/rte_net.c                           |  90 +++---
>  lib/librte_net/rte_net.h                           |  22 +-
>  lib/librte_net/rte_sctp.h                          |   2 +-
>  lib/librte_net/rte_tcp.h                           |   2 +-
>  lib/librte_net/rte_udp.h                           |   2 +-
>  lib/librte_pipeline/rte_table_action.c             | 210 +++++++-------
>  lib/librte_pipeline/rte_table_action.h             |   4 +-
>  lib/librte_port/rte_port_ras.c                     |   8 +-
>  lib/librte_port/rte_port_source_sink.c             |   6 +-
>  lib/librte_vhost/vhost.h                           |   2 +-
>  lib/librte_vhost/virtio_net.c                      |  42 +--
>  test/test-acl/main.c                               |   2 +-
>  test/test-pipeline/pipeline_acl.c                  |  16 +-
>  test/test-pipeline/pipeline_hash.c                 |  12 +-
>  test/test/packet_burst_generator.c                 | 126 ++++----
>  test/test/packet_burst_generator.h                 |  26 +-
>  test/test/test_acl.c                               |   8 +-
>  test/test/test_acl.h                               | 122 ++++----
>  test/test/test_cmdline_etheraddr.c                 |  16 +-
>  test/test/test_efd.c                               |  20 +-
>  test/test/test_event_eth_rx_adapter.c              |   2 +-
>  test/test/test_event_eth_tx_adapter.c              |   2 +-
>  test/test/test_flow_classify.c                     |  68 ++---
>  test/test/test_hash.c                              |  20 +-
>  test/test/test_link_bonding.c                      | 284 +++++++++---------
>  test/test/test_link_bonding_mode4.c                | 116 ++++----
>  test/test/test_link_bonding_rssconf.c              |   6 +-
>  test/test/test_lpm.c                               |  76 ++---
>  test/test/test_lpm_perf.c                          |  10 +-
>  test/test/test_member.c                            |  20 +-
>  test/test/test_pmd_perf.c                          |  20 +-
>  test/test/test_sched.c                             |  20 +-
>  test/test/test_table_acl.c                         |   8 +-
>  test/test/test_thash.c                             |  12 +-
>  test/test/virtual_pmd.c                            |   6 +-
>  test/test/virtual_pmd.h                            |   2 +-
>  367 files changed, 3906 insertions(+), 3913 deletions(-)
> 

Since BSD structures are available on Linux and BSD why is DPDK reinventing?
There is no value in doing that.
  
Olivier Matz Oct. 26, 2018, 7:20 a.m. UTC | #6
Hi,

On Wed, Oct 24, 2018 at 05:39:09PM +0100, Bruce Richardson wrote:
> On Wed, Oct 24, 2018 at 10:18:19AM +0200, Olivier Matz wrote:
> > This RFC targets 19.02.
> > 
> > The rte_net headers conflict with the libc headers, because
> > some definitions are duplicated, sometimes with few differences.
> > This was discussed in [1], and more recently at the techboard.
> > 
> > Before sending the deprecation notice (target for this is 18.11),
> > here is a draft that can be discussed.
> > 
> > This RFC 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.
> > 
> > One question I'm asking is how can we manage the transition.
> > Initially, I hoped it was possible to have a compat layer during
> > one release (supporting both prefixed and unprefixed names), but
> > now that the patch is done, it seems the impact is too big, and
> > impacts too many libraries.
> > 
> > Few examples:
> >   - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *)
> >   - many rte_flow structures use the rte_ prefixed net structures
> >   - the mac field of virtio_net structure is rte_ether_addr
> >   - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *)
> >   ...
> > 
> > Therefore, it is clear that doing this would break the compilation
> > of many external applications.
> > 
> 
> Can you clarify a bit as to why we can't keep around compatibility versions
> of the headers, alongside the new versions? I'm not following the logic
> above. Can we not introduce completely new headers with the replacements
> while leaving the old ones intact?

This is something I tried to do, it is not in the RFC because it was
not satisfying, but you can find it there:

http://git.droids-corp.org/?p=dpdk.git;a=commitdiff;h=ba1e8e498306

With this patch, the usage of unprefixed structures, defines and
functions in rte net is still possible by an external application,
except if RTE_NET_NO_COMPAT is defined.

However, functions and structures that are not in librte_net (the
examples from my previous mail, quoted above) use the rte_ prefixed
structures in their prototypes. For instance, an application that use
rte_eth_macaddr_get() will no compile anymore because it will pass
a (struct ether_addr *) instead of a (struct rte_ether_addr *).

I don't see any good mean to fix this. Maybe we can do something with
defines, but I don't think it is possible to provide both APIs for
functions like rte_eth_macaddr_get(). I'm also not convinced it will be
that helpful. At the end, if the patchset is applied, we want the
applications to switch to the new API. To ease the transition, we can
provide a script to patch an application, very similar to the one I use
to generate the patchset.

Olivier
  
Olivier Matz Oct. 26, 2018, 7:22 a.m. UTC | #7
Hi,

On Wed, Oct 24, 2018 at 02:56:14PM +0000, Wiles, Keith wrote:
> 
> 
> > On Oct 24, 2018, at 1:18 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
> > 
> > This RFC targets 19.02.
> > 
> > The rte_net headers conflict with the libc headers, because
> > some definitions are duplicated, sometimes with few differences.
> > This was discussed in [1], and more recently at the techboard.
> > 
> > Before sending the deprecation notice (target for this is 18.11),
> > here is a draft that can be discussed.
> > 
> > This RFC 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.
> > 
> > One question I'm asking is how can we manage the transition.
> > Initially, I hoped it was possible to have a compat layer during
> > one release (supporting both prefixed and unprefixed names), but
> > now that the patch is done, it seems the impact is too big, and
> > impacts too many libraries.
> > 
> > Few examples:
> >  - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *)
> >  - many rte_flow structures use the rte_ prefixed net structures
> >  - the mac field of virtio_net structure is rte_ether_addr
> >  - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *)
> >  ...
> > 
> > Therefore, it is clear that doing this would break the compilation
> > of many external applications.
> > 
> > Another drawback we need to take in account: it will make the
> > backport of patches more difficult, although this is something
> > that could be tempered by a script.
> > 
> > While it is obviously better to have a good namespace convention, 
> > we need to identify the issues we have today before deciding it's
> > worth doing the change.
> > 
> > Comments?
> 
> I did not see the deprecation notice in the patches below, but I could have missed it.

I will send it only if we reach a consensus about the need to
apply the patchset.

Regards
Olivier
  
Olivier Matz Oct. 26, 2018, 7:56 a.m. UTC | #8
Hi Stephen,

On Wed, Oct 24, 2018 at 11:38:12AM -0700, Stephen Hemminger wrote:
> On Wed, 24 Oct 2018 10:18:19 +0200
> Olivier Matz <olivier.matz@6wind.com> wrote:
> 
> > This RFC targets 19.02.
> > 
> > The rte_net headers conflict with the libc headers, because
> > some definitions are duplicated, sometimes with few differences.
> > This was discussed in [1], and more recently at the techboard.
> > 
> > Before sending the deprecation notice (target for this is 18.11),
> > here is a draft that can be discussed.
> > 
> > This RFC 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.
> > 
> > One question I'm asking is how can we manage the transition.
> > Initially, I hoped it was possible to have a compat layer during
> > one release (supporting both prefixed and unprefixed names), but
> > now that the patch is done, it seems the impact is too big, and
> > impacts too many libraries.
> > 
> > Few examples:
> >   - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *)
> >   - many rte_flow structures use the rte_ prefixed net structures
> >   - the mac field of virtio_net structure is rte_ether_addr
> >   - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *)
> >   ...
> > 
> > Therefore, it is clear that doing this would break the compilation
> > of many external applications.
> > 
> > Another drawback we need to take in account: it will make the
> > backport of patches more difficult, although this is something
> > that could be tempered by a script.
> > 
> > While it is obviously better to have a good namespace convention, 
> > we need to identify the issues we have today before deciding it's
> > worth doing the change.
> > 
> > Comments?
> > 
> > 
> > Things that are missing in RFC:
> > - test with FreeBSD
> > - manually fix some indentation issues
> > 
> > 
> > Olivier Matz (14):
> >   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
> > 
> 
> Since BSD structures are available on Linux and BSD why is DPDK reinventing?
> There is no value in doing that.

From what I see, some structures or defines are a bit different in Linux
and FreeBSD. Examples:

/* Linux */

	struct ether_addr
	{
	  u_int8_t ether_addr_octet[ETH_ALEN];
	} __attribute__ ((__packed__));



/* FreeBSD */

	struct ether_addr {
	        u_char octet[ETHER_ADDR_LEN];
	} __packed;


That's true the compat between Linux and FreeBSD is better than before
in glibc. For instance with 7011c2622fe3 ("Remove __FAVOR_BSD.") added
in 2013 (glibc 2.19). It seems that MUSL also supports BSD network
structures.

So, I agree that using BSD structure looks possible, at least for
ip/ip6/tcp/udp/icmp/... structures and defines. I think we would still
need to provide some network structures for less usual protocols.

The question is: are we confident that the support of network BSD
struct/defines/funcs is good enough in all libc we (will) want to use?
Since DPDK is a network software, it is not that odd to provide our
own network structures, because we will have control on them.


Olivier
  
Bruce Richardson Oct. 26, 2018, 10:15 a.m. UTC | #9
On Fri, Oct 26, 2018 at 09:20:15AM +0200, Olivier Matz wrote:
> Hi,
> 
> On Wed, Oct 24, 2018 at 05:39:09PM +0100, Bruce Richardson wrote:
> > On Wed, Oct 24, 2018 at 10:18:19AM +0200, Olivier Matz wrote:
> > > This RFC targets 19.02.
> > > 
> > > The rte_net headers conflict with the libc headers, because
> > > some definitions are duplicated, sometimes with few differences.
> > > This was discussed in [1], and more recently at the techboard.
> > > 
> > > Before sending the deprecation notice (target for this is 18.11),
> > > here is a draft that can be discussed.
> > > 
> > > This RFC 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.
> > > 
> > > One question I'm asking is how can we manage the transition.
> > > Initially, I hoped it was possible to have a compat layer during
> > > one release (supporting both prefixed and unprefixed names), but
> > > now that the patch is done, it seems the impact is too big, and
> > > impacts too many libraries.
> > > 
> > > Few examples:
> > >   - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *)
> > >   - many rte_flow structures use the rte_ prefixed net structures
> > >   - the mac field of virtio_net structure is rte_ether_addr
> > >   - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *)
> > >   ...
> > > 
> > > Therefore, it is clear that doing this would break the compilation
> > > of many external applications.
> > > 
> > 
> > Can you clarify a bit as to why we can't keep around compatibility versions
> > of the headers, alongside the new versions? I'm not following the logic
> > above. Can we not introduce completely new headers with the replacements
> > while leaving the old ones intact?
> 
> This is something I tried to do, it is not in the RFC because it was
> not satisfying, but you can find it there:
> 
> http://git.droids-corp.org/?p=dpdk.git;a=commitdiff;h=ba1e8e498306
> 
> With this patch, the usage of unprefixed structures, defines and
> functions in rte net is still possible by an external application,
> except if RTE_NET_NO_COMPAT is defined.
> 
> However, functions and structures that are not in librte_net (the
> examples from my previous mail, quoted above) use the rte_ prefixed
> structures in their prototypes. For instance, an application that use
> rte_eth_macaddr_get() will no compile anymore because it will pass
> a (struct ether_addr *) instead of a (struct rte_ether_addr *).
> 
> I don't see any good mean to fix this. Maybe we can do something with
> defines, but I don't think it is possible to provide both APIs for
> functions like rte_eth_macaddr_get(). I'm also not convinced it will be
> that helpful. At the end, if the patchset is applied, we want the
> applications to switch to the new API. To ease the transition, we can
> provide a script to patch an application, very similar to the one I use
> to generate the patchset.
> 

Out of interest, about how many non rte_net functions are we talking about here?
  
Olivier Matz Oct. 26, 2018, 11:28 a.m. UTC | #10
On Fri, Oct 26, 2018 at 11:15:14AM +0100, Bruce Richardson wrote:
> On Fri, Oct 26, 2018 at 09:20:15AM +0200, Olivier Matz wrote:
> > Hi,
> > 
> > On Wed, Oct 24, 2018 at 05:39:09PM +0100, Bruce Richardson wrote:
> > > On Wed, Oct 24, 2018 at 10:18:19AM +0200, Olivier Matz wrote:
> > > > This RFC targets 19.02.
> > > > 
> > > > The rte_net headers conflict with the libc headers, because
> > > > some definitions are duplicated, sometimes with few differences.
> > > > This was discussed in [1], and more recently at the techboard.
> > > > 
> > > > Before sending the deprecation notice (target for this is 18.11),
> > > > here is a draft that can be discussed.
> > > > 
> > > > This RFC 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.
> > > > 
> > > > One question I'm asking is how can we manage the transition.
> > > > Initially, I hoped it was possible to have a compat layer during
> > > > one release (supporting both prefixed and unprefixed names), but
> > > > now that the patch is done, it seems the impact is too big, and
> > > > impacts too many libraries.
> > > > 
> > > > Few examples:
> > > >   - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *)
> > > >   - many rte_flow structures use the rte_ prefixed net structures
> > > >   - the mac field of virtio_net structure is rte_ether_addr
> > > >   - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *)
> > > >   ...
> > > > 
> > > > Therefore, it is clear that doing this would break the compilation
> > > > of many external applications.
> > > > 
> > > 
> > > Can you clarify a bit as to why we can't keep around compatibility versions
> > > of the headers, alongside the new versions? I'm not following the logic
> > > above. Can we not introduce completely new headers with the replacements
> > > while leaving the old ones intact?
> > 
> > This is something I tried to do, it is not in the RFC because it was
> > not satisfying, but you can find it there:
> > 
> > http://git.droids-corp.org/?p=dpdk.git;a=commitdiff;h=ba1e8e498306
> > 
> > With this patch, the usage of unprefixed structures, defines and
> > functions in rte net is still possible by an external application,
> > except if RTE_NET_NO_COMPAT is defined.
> > 
> > However, functions and structures that are not in librte_net (the
> > examples from my previous mail, quoted above) use the rte_ prefixed
> > structures in their prototypes. For instance, an application that use
> > rte_eth_macaddr_get() will no compile anymore because it will pass
> > a (struct ether_addr *) instead of a (struct rte_ether_addr *).
> > 
> > I don't see any good mean to fix this. Maybe we can do something with
> > defines, but I don't think it is possible to provide both APIs for
> > functions like rte_eth_macaddr_get(). I'm also not convinced it will be
> > that helpful. At the end, if the patchset is applied, we want the
> > applications to switch to the new API. To ease the transition, we can
> > provide a script to patch an application, very similar to the one I use
> > to generate the patchset.
> > 
> 
> Out of interest, about how many non rte_net functions are we talking about here?

I didn't count, but many. And not only functions, also structures.

To give an idea, here is the output of:
  git diff origin/master..HEAD lib/ | filterdiff -i '*.h' -x 'a/lib/librte_net/*'


  diff --git a/lib/librte_ethdev/rte_eth_ctrl.h b/lib/librte_ethdev/rte_eth_ctrl.h
  index 5ea8ae24c..821d971cd 100644
  --- a/lib/librte_ethdev/rte_eth_ctrl.h
  +++ b/lib/librte_ethdev/rte_eth_ctrl.h
  @@ -110,7 +110,7 @@ struct rte_eth_mac_filter {
   	uint8_t is_vf; /**< 1 for VF, 0 for port dev */
   	uint16_t dst_id; /**< VF ID, available when is_vf is 1*/
   	enum rte_mac_filter_type filter_type; /**< MAC filter type */
  -	struct ether_addr mac_addr;
  +	struct rte_ether_addr mac_addr;
   };
   
   /**
  @@ -126,7 +126,7 @@ struct rte_eth_mac_filter {
    * RTE_ETH_FILTER_DELETE and RTE_ETH_FILTER_GET operations.
    */
   struct rte_eth_ethertype_filter {
  -	struct ether_addr mac_addr;   /**< Mac address to match. */
  +	struct rte_ether_addr mac_addr;   /**< Mac address to match. */
   	uint16_t ether_type;          /**< Ether type to match */
   	uint16_t flags;               /**< Flags from RTE_ETHTYPE_FLAGS_* */
   	uint16_t queue;               /**< Queue assigned to when match*/
  @@ -265,8 +265,8 @@ enum rte_tunnel_iptype {
    * Tunneling Packet filter configuration.
    */
   struct rte_eth_tunnel_filter_conf {
  -	struct ether_addr outer_mac;    /**< Outer MAC address to match. */
  -	struct ether_addr inner_mac;    /**< Inner MAC address to match. */
  +	struct rte_ether_addr outer_mac;    /**< Outer MAC address to match. */
  +	struct rte_ether_addr inner_mac;    /**< Inner MAC address to match. */
   	uint16_t inner_vlan;            /**< Inner VLAN to match. */
   	enum rte_tunnel_iptype ip_type; /**< IP address type. */
   	/** Outer destination IP address to match if ETH_TUNNEL_FILTER_OIP
  @@ -473,7 +473,7 @@ struct rte_eth_sctpv6_flow {
    * A structure used to define the input for MAC VLAN flow
    */
   struct rte_eth_mac_vlan_flow {
  -	struct ether_addr mac_addr;  /**< Mac address to match. */
  +	struct rte_ether_addr mac_addr;  /**< Mac address to match. */
   };
   
   /**
  @@ -493,7 +493,7 @@ struct rte_eth_tunnel_flow {
   	enum rte_eth_fdir_tunnel_type tunnel_type; /**< Tunnel type to match. */
   	/** Tunnel ID to match. TNI, VNI... in big endian. */
   	uint32_t tunnel_id;
  -	struct ether_addr mac_addr;                /**< Mac address to match. */
  +	struct rte_ether_addr mac_addr;                /**< Mac address to match. */
   };
   
   /**
  diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
  index fb40c89e0..5deb4e38e 100644
  --- a/lib/librte_ethdev/rte_ethdev.h
  +++ b/lib/librte_ethdev/rte_ethdev.h
  @@ -2159,7 +2159,7 @@ int rte_eth_dev_set_rx_queue_stats_mapping(uint16_t port_id,
    *   A pointer to a structure of type *ether_addr* to be filled with
    *   the Ethernet address of the Ethernet device.
    */
  -void rte_eth_macaddr_get(uint16_t port_id, struct ether_addr *mac_addr);
  +void rte_eth_macaddr_get(uint16_t port_id, struct rte_ether_addr *mac_addr);
   
   /**
    * Retrieve the contextual information of an Ethernet device.
  @@ -2843,7 +2843,7 @@ int rte_eth_dev_priority_flow_ctrl_set(uint16_t port_id,
    *   - (-ENOSPC) if no more MAC addresses can be added.
    *   - (-EINVAL) if MAC address is invalid.
    */
  -int rte_eth_dev_mac_addr_add(uint16_t port_id, struct ether_addr *mac_addr,
  +int rte_eth_dev_mac_addr_add(uint16_t port_id, struct rte_ether_addr *mac_addr,
   				uint32_t pool);
   
   /**
  @@ -2859,7 +2859,7 @@ int rte_eth_dev_mac_addr_add(uint16_t port_id, struct ether_addr *mac_addr,
    *   - (-ENODEV) if *port* invalid.
    *   - (-EADDRINUSE) if attempting to remove the default MAC address
    */
  -int rte_eth_dev_mac_addr_remove(uint16_t port_id, struct ether_addr *mac_addr);
  +int rte_eth_dev_mac_addr_remove(uint16_t port_id, struct rte_ether_addr *mac_addr);
   
   /**
    * Set the default MAC address.
  @@ -2875,7 +2875,7 @@ int rte_eth_dev_mac_addr_remove(uint16_t port_id, struct ether_addr *mac_addr);
    *   - (-EINVAL) if MAC address is invalid.
    */
   int rte_eth_dev_default_mac_addr_set(uint16_t port_id,
  -		struct ether_addr *mac_addr);
  +		struct rte_ether_addr *mac_addr);
   
   /**
    * Update Redirection Table(RETA) of Receive Side Scaling of Ethernet device.
  @@ -2936,7 +2936,7 @@ int rte_eth_dev_rss_reta_query(uint16_t port_id,
    *   - (-EIO) if device is removed.
    *   - (-EINVAL) if bad parameter.
    */
  -int rte_eth_dev_uc_hash_table_set(uint16_t port_id, struct ether_addr *addr,
  +int rte_eth_dev_uc_hash_table_set(uint16_t port_id, struct rte_ether_addr *addr,
   				  uint8_t on);
   
    /**
  @@ -3479,7 +3479,7 @@ rte_eth_dev_get_module_eeprom(uint16_t port_id,
    *   - (-ENOSPC) if *port_id* has not enough multicast filtering resources.
    */
   int rte_eth_dev_set_mc_addr_list(uint16_t port_id,
  -				 struct ether_addr *mc_addr_set,
  +				 struct rte_ether_addr *mc_addr_set,
   				 uint32_t nb_mc_addr);
   
   /**
  diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h
  index 0d28fd902..fa518620e 100644
  --- a/lib/librte_ethdev/rte_ethdev_core.h
  +++ b/lib/librte_ethdev/rte_ethdev_core.h
  @@ -250,17 +250,17 @@ typedef void (*eth_mac_addr_remove_t)(struct rte_eth_dev *dev, uint32_t index);
   /**< @internal Remove MAC address from receive address register */
   
   typedef int (*eth_mac_addr_add_t)(struct rte_eth_dev *dev,
  -				  struct ether_addr *mac_addr,
  +				  struct rte_ether_addr *mac_addr,
   				  uint32_t index,
   				  uint32_t vmdq);
   /**< @internal Set a MAC address into Receive Address Address Register */
   
   typedef int (*eth_mac_addr_set_t)(struct rte_eth_dev *dev,
  -				  struct ether_addr *mac_addr);
  +				  struct rte_ether_addr *mac_addr);
   /**< @internal Set a MAC address into Receive Address Address Register */
   
   typedef int (*eth_uc_hash_table_set_t)(struct rte_eth_dev *dev,
  -				  struct ether_addr *mac_addr,
  +				  struct rte_ether_addr *mac_addr,
   				  uint8_t on);
   /**< @internal Set a Unicast Hash bitmap */
   
  @@ -292,7 +292,7 @@ typedef int (*eth_udp_tunnel_port_del_t)(struct rte_eth_dev *dev,
   /**< @internal Delete tunneling UDP port */
   
   typedef int (*eth_set_mc_addr_list_t)(struct rte_eth_dev *dev,
  -				      struct ether_addr *mc_addr_set,
  +				      struct rte_ether_addr *mc_addr_set,
   				      uint32_t nb_mc_addr);
   /**< @internal set the list of multicast addresses on an Ethernet device */
   
  @@ -597,10 +597,10 @@ struct rte_eth_dev_data {
   	/**< Common rx buffer size handled by all queues */
   
   	uint64_t rx_mbuf_alloc_failed; /**< RX ring mbuf allocation failures. */
  -	struct ether_addr* mac_addrs;/**< Device Ethernet Link address. */
  +	struct rte_ether_addr* mac_addrs;/**< Device Ethernet Link address. */
   	uint64_t mac_pool_sel[ETH_NUM_RECEIVE_MAC_ADDR];
   	/** bitmap array of associating Ethernet MAC addresses to pools */
  -	struct ether_addr* hash_mac_addrs;
  +	struct rte_ether_addr* hash_mac_addrs;
   	/** Device Ethernet MAC addresses of hash filtering. */
   	uint16_t port_id;           /**< Device [external] port identifier. */
   	__extension__
  diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
  index 26e2fcfa0..c27d590a1 100644
  --- a/lib/librte_ethdev/rte_flow.h
  +++ b/lib/librte_ethdev/rte_flow.h
  @@ -577,8 +577,8 @@ static const struct rte_flow_item_raw rte_flow_item_raw_mask = {
    * same order as on the wire.
    */
   struct rte_flow_item_eth {
  -	struct ether_addr dst; /**< Destination MAC. */
  -	struct ether_addr src; /**< Source MAC. */
  +	struct rte_ether_addr dst; /**< Destination MAC. */
  +	struct rte_ether_addr src; /**< Source MAC. */
   	rte_be16_t type; /**< EtherType or TPID. */
   };
   
  @@ -597,7 +597,7 @@ static const struct rte_flow_item_eth rte_flow_item_eth_mask = {
    * Matches an 802.1Q/ad VLAN tag.
    *
    * The corresponding standard outer EtherType (TPID) values are
  - * ETHER_TYPE_VLAN or ETHER_TYPE_QINQ. It can be overridden by the preceding
  + * RTE_ETHER_TYPE_VLAN or RTE_ETHER_TYPE_QINQ. It can be overridden by the preceding
    * pattern item.
    */
   struct rte_flow_item_vlan {
  @@ -621,7 +621,7 @@ static const struct rte_flow_item_vlan rte_flow_item_vlan_mask = {
    * Note: IPv4 options are handled by dedicated pattern items.
    */
   struct rte_flow_item_ipv4 {
  -	struct ipv4_hdr hdr; /**< IPv4 header definition. */
  +	struct rte_ipv4_hdr hdr; /**< IPv4 header definition. */
   };
   
   /** Default mask for RTE_FLOW_ITEM_TYPE_IPV4. */
  @@ -643,7 +643,7 @@ static const struct rte_flow_item_ipv4 rte_flow_item_ipv4_mask = {
    * RTE_FLOW_ITEM_TYPE_IPV6_EXT.
    */
   struct rte_flow_item_ipv6 {
  -	struct ipv6_hdr hdr; /**< IPv6 header definition. */
  +	struct rte_ipv6_hdr hdr; /**< IPv6 header definition. */
   };
   
   /** Default mask for RTE_FLOW_ITEM_TYPE_IPV6. */
  @@ -666,7 +666,7 @@ static const struct rte_flow_item_ipv6 rte_flow_item_ipv6_mask = {
    * Matches an ICMP header.
    */
   struct rte_flow_item_icmp {
  -	struct icmp_hdr hdr; /**< ICMP header definition. */
  +	struct rte_icmp_hdr hdr; /**< ICMP header definition. */
   };
   
   /** Default mask for RTE_FLOW_ITEM_TYPE_ICMP. */
  @@ -685,7 +685,7 @@ static const struct rte_flow_item_icmp rte_flow_item_icmp_mask = {
    * Matches a UDP header.
    */
   struct rte_flow_item_udp {
  -	struct udp_hdr hdr; /**< UDP header definition. */
  +	struct rte_udp_hdr hdr; /**< UDP header definition. */
   };
   
   /** Default mask for RTE_FLOW_ITEM_TYPE_UDP. */
  @@ -704,7 +704,7 @@ static const struct rte_flow_item_udp rte_flow_item_udp_mask = {
    * Matches a TCP header.
    */
   struct rte_flow_item_tcp {
  -	struct tcp_hdr hdr; /**< TCP header definition. */
  +	struct rte_tcp_hdr hdr; /**< TCP header definition. */
   };
   
   /** Default mask for RTE_FLOW_ITEM_TYPE_TCP. */
  @@ -723,7 +723,7 @@ static const struct rte_flow_item_tcp rte_flow_item_tcp_mask = {
    * Matches a SCTP header.
    */
   struct rte_flow_item_sctp {
  -	struct sctp_hdr hdr; /**< SCTP header definition. */
  +	struct rte_sctp_hdr hdr; /**< SCTP header definition. */
   };
   
   /** Default mask for RTE_FLOW_ITEM_TYPE_SCTP. */
  @@ -761,7 +761,7 @@ static const struct rte_flow_item_vxlan rte_flow_item_vxlan_mask = {
    * Matches a E-tag header.
    *
    * The corresponding standard outer EtherType (TPID) value is
  - * ETHER_TYPE_ETAG. It can be overridden by the preceding pattern item.
  + * RTE_ETHER_TYPE_ETAG. It can be overridden by the preceding pattern item.
    */
   struct rte_flow_item_e_tag {
   	/**
  @@ -908,7 +908,7 @@ static const struct rte_flow_item_gtp rte_flow_item_gtp_mask = {
    * Matches an ESP header.
    */
   struct rte_flow_item_esp {
  -	struct esp_hdr hdr; /**< ESP header definition. */
  +	struct rte_esp_hdr hdr; /**< ESP header definition. */
   };
   
   /** Default mask for RTE_FLOW_ITEM_TYPE_ESP. */
  @@ -974,9 +974,9 @@ struct rte_flow_item_arp_eth_ipv4 {
   	uint8_t hln; /**< Hardware address length, normally 6. */
   	uint8_t pln; /**< Protocol address length, normally 4. */
   	rte_be16_t op; /**< Opcode (1 for request, 2 for reply). */
  -	struct ether_addr sha; /**< Sender hardware address. */
  +	struct rte_ether_addr sha; /**< Sender hardware address. */
   	rte_be32_t spa; /**< Sender IPv4 address. */
  -	struct ether_addr tha; /**< Target hardware address. */
  +	struct rte_ether_addr tha; /**< Target hardware address. */
   	rte_be32_t tpa; /**< Target IPv4 address. */
   };
   
  @@ -1120,7 +1120,7 @@ rte_flow_item_icmp6_nd_opt_mask = {
   struct rte_flow_item_icmp6_nd_opt_sla_eth {
   	uint8_t type; /**< ND option type, normally 1. */
   	uint8_t length; /**< ND option length, normally 1. */
  -	struct ether_addr sla; /**< Source Ethernet LLA. */
  +	struct rte_ether_addr sla; /**< Source Ethernet LLA. */
   };
   
   /** Default mask for RTE_FLOW_ITEM_TYPE_ICMP6_ND_OPT_SLA_ETH. */
  @@ -1145,7 +1145,7 @@ rte_flow_item_icmp6_nd_opt_sla_eth_mask = {
   struct rte_flow_item_icmp6_nd_opt_tla_eth {
   	uint8_t type; /**< ND option type, normally 2. */
   	uint8_t length; /**< ND option length, normally 1. */
  -	struct ether_addr tla; /**< Target Ethernet LLA. */
  +	struct rte_ether_addr tla; /**< Target Ethernet LLA. */
   };
   
   /** Default mask for RTE_FLOW_ITEM_TYPE_ICMP6_ND_OPT_TLA_ETH. */
  @@ -2036,7 +2036,7 @@ struct rte_flow_action_set_ttl {
    * Set MAC address from the matched flow
    */
   struct rte_flow_action_set_mac {
  -	uint8_t mac_addr[ETHER_ADDR_LEN];
  +	uint8_t mac_addr[RTE_ETHER_ADDR_LEN];
   };
   
   /*
  diff --git a/lib/librte_gro/gro_tcp4.h b/lib/librte_gro/gro_tcp4.h
  index 6bb30cdb9..63f06bec4 100644
  --- a/lib/librte_gro/gro_tcp4.h
  +++ b/lib/librte_gro/gro_tcp4.h
  @@ -19,8 +19,8 @@
   
   /* Header fields representing a TCP/IPv4 flow */
   struct tcp4_flow_key {
  -	struct ether_addr eth_saddr;
  -	struct ether_addr eth_daddr;
  +	struct rte_ether_addr eth_saddr;
  +	struct rte_ether_addr eth_daddr;
   	uint32_t ip_src_addr;
   	uint32_t ip_dst_addr;
   
  @@ -182,8 +182,8 @@ uint32_t gro_tcp4_tbl_pkt_count(void *tbl);
   static inline int
   is_same_tcp4_flow(struct tcp4_flow_key k1, struct tcp4_flow_key k2)
   {
  -	return (is_same_ether_addr(&k1.eth_saddr, &k2.eth_saddr) &&
  -			is_same_ether_addr(&k1.eth_daddr, &k2.eth_daddr) &&
  +	return (rte_is_same_ether_addr(&k1.eth_saddr, &k2.eth_saddr) &&
  +			rte_is_same_ether_addr(&k1.eth_daddr, &k2.eth_daddr) &&
   			(k1.ip_src_addr == k2.ip_src_addr) &&
   			(k1.ip_dst_addr == k2.ip_dst_addr) &&
   			(k1.recv_ack == k2.recv_ack) &&
  @@ -255,7 +255,7 @@ merge_two_tcp4_packets(struct gro_tcp4_item *item,
    */
   static inline int
   check_seq_option(struct gro_tcp4_item *item,
  -		struct tcp_hdr *tcph,
  +		struct rte_tcp_hdr *tcph,
   		uint32_t sent_seq,
   		uint16_t ip_id,
   		uint16_t tcp_hl,
  @@ -264,17 +264,17 @@ check_seq_option(struct gro_tcp4_item *item,
   		uint8_t is_atomic)
   {
   	struct rte_mbuf *pkt_orig = item->firstseg;
  -	struct ipv4_hdr *iph_orig;
  -	struct tcp_hdr *tcph_orig;
  +	struct rte_ipv4_hdr *iph_orig;
  +	struct rte_tcp_hdr *tcph_orig;
   	uint16_t len, tcp_hl_orig;
   
  -	iph_orig = (struct ipv4_hdr *)(rte_pktmbuf_mtod(pkt_orig, char *) +
  +	iph_orig = (struct rte_ipv4_hdr *)(rte_pktmbuf_mtod(pkt_orig, char *) +
   			l2_offset + pkt_orig->l2_len);
  -	tcph_orig = (struct tcp_hdr *)((char *)iph_orig + pkt_orig->l3_len);
  +	tcph_orig = (struct rte_tcp_hdr *)((char *)iph_orig + pkt_orig->l3_len);
   	tcp_hl_orig = pkt_orig->l4_len;
   
   	/* Check if TCP option fields equal */
  -	len = RTE_MAX(tcp_hl, tcp_hl_orig) - sizeof(struct tcp_hdr);
  +	len = RTE_MAX(tcp_hl, tcp_hl_orig) - sizeof(struct rte_tcp_hdr);
   	if ((tcp_hl != tcp_hl_orig) || ((len > 0) &&
   				(memcmp(tcph + 1, tcph_orig + 1,
   					len) != 0)))
  diff --git a/lib/librte_gro/gro_vxlan_tcp4.h b/lib/librte_gro/gro_vxlan_tcp4.h
  index 0cafb9211..7832942a6 100644
  --- a/lib/librte_gro/gro_vxlan_tcp4.h
  +++ b/lib/librte_gro/gro_vxlan_tcp4.h
  @@ -12,10 +12,10 @@
   /* Header fields representing a VxLAN flow */
   struct vxlan_tcp4_flow_key {
   	struct tcp4_flow_key inner_key;
  -	struct vxlan_hdr vxlan_hdr;
  +	struct rte_vxlan_hdr vxlan_hdr;
   
  -	struct ether_addr outer_eth_saddr;
  -	struct ether_addr outer_eth_daddr;
  +	struct rte_ether_addr outer_eth_saddr;
  +	struct rte_ether_addr outer_eth_daddr;
   
   	uint32_t outer_ip_src_addr;
   	uint32_t outer_ip_dst_addr;
  diff --git a/lib/librte_gso/gso_common.h b/lib/librte_gso/gso_common.h
  index 6cd764ff5..48ad1686f 100644
  --- a/lib/librte_gso/gso_common.h
  +++ b/lib/librte_gso/gso_common.h
  @@ -12,8 +12,8 @@
   #include <rte_tcp.h>
   #include <rte_udp.h>
   
  -#define IS_FRAGMENTED(frag_off) (((frag_off) & IPV4_HDR_OFFSET_MASK) != 0 \
  -		|| ((frag_off) & IPV4_HDR_MF_FLAG) == IPV4_HDR_MF_FLAG)
  +#define IS_FRAGMENTED(frag_off) (((frag_off) & RTE_IPV4_HDR_OFFSET_MASK) != 0 \
  +		|| ((frag_off) & RTE_IPV4_HDR_MF_FLAG) == RTE_IPV4_HDR_MF_FLAG)
   
   #define TCP_HDR_PSH_MASK ((uint8_t)0x08)
   #define TCP_HDR_FIN_MASK ((uint8_t)0x01)
  @@ -46,9 +46,9 @@
   static inline void
   update_udp_header(struct rte_mbuf *pkt, uint16_t udp_offset)
   {
  -	struct udp_hdr *udp_hdr;
  +	struct rte_udp_hdr *udp_hdr;
   
  -	udp_hdr = (struct udp_hdr *)(rte_pktmbuf_mtod(pkt, char *) +
  +	udp_hdr = (struct rte_udp_hdr *)(rte_pktmbuf_mtod(pkt, char *) +
   			udp_offset);
   	udp_hdr->dgram_len = rte_cpu_to_be_16(pkt->pkt_len - udp_offset);
   }
  @@ -71,9 +71,9 @@ static inline void
   update_tcp_header(struct rte_mbuf *pkt, uint16_t l4_offset, uint32_t sent_seq,
   		uint8_t non_tail)
   {
  -	struct tcp_hdr *tcp_hdr;
  +	struct rte_tcp_hdr *tcp_hdr;
   
  -	tcp_hdr = (struct tcp_hdr *)(rte_pktmbuf_mtod(pkt, char *) +
  +	tcp_hdr = (struct rte_tcp_hdr *)(rte_pktmbuf_mtod(pkt, char *) +
   			l4_offset);
   	tcp_hdr->sent_seq = rte_cpu_to_be_32(sent_seq);
   	if (likely(non_tail))
  @@ -98,9 +98,9 @@ update_tcp_header(struct rte_mbuf *pkt, uint16_t l4_offset, uint32_t sent_seq,
   static inline void
   update_ipv4_header(struct rte_mbuf *pkt, uint16_t l3_offset, uint16_t id)
   {
  -	struct ipv4_hdr *ipv4_hdr;
  +	struct rte_ipv4_hdr *ipv4_hdr;
   
  -	ipv4_hdr = (struct ipv4_hdr *)(rte_pktmbuf_mtod(pkt, char *) +
  +	ipv4_hdr = (struct rte_ipv4_hdr *)(rte_pktmbuf_mtod(pkt, char *) +
   			l3_offset);
   	ipv4_hdr->total_length = rte_cpu_to_be_16(pkt->pkt_len - l3_offset);
   	ipv4_hdr->packet_id = rte_cpu_to_be_16(id);
  diff --git a/lib/librte_gso/rte_gso.h b/lib/librte_gso/rte_gso.h
  index a626a11e3..3aab297f4 100644
  --- a/lib/librte_gso/rte_gso.h
  +++ b/lib/librte_gso/rte_gso.h
  @@ -18,12 +18,12 @@ extern "C" {
   #include <rte_mbuf.h>
   
   /* Minimum GSO segment size for TCP based packets. */
  -#define RTE_GSO_SEG_SIZE_MIN (sizeof(struct ether_hdr) + \
  -		sizeof(struct ipv4_hdr) + sizeof(struct tcp_hdr) + 1)
  +#define RTE_GSO_SEG_SIZE_MIN (sizeof(struct rte_ether_hdr) + \
  +		sizeof(struct rte_ipv4_hdr) + sizeof(struct rte_tcp_hdr) + 1)
   
   /* Minimum GSO segment size for UDP based packets. */
  -#define RTE_GSO_UDP_SEG_SIZE_MIN (sizeof(struct ether_hdr) + \
  -		sizeof(struct ipv4_hdr) + sizeof(struct udp_hdr) + 1)
  +#define RTE_GSO_UDP_SEG_SIZE_MIN (sizeof(struct rte_ether_hdr) + \
  +		sizeof(struct rte_ipv4_hdr) + sizeof(struct rte_udp_hdr) + 1)
   
   /* GSO flags for rte_gso_ctx. */
   #define RTE_GSO_FLAG_IPID_FIXED (1ULL << 0)
  diff --git a/lib/librte_hash/rte_thash.h b/lib/librte_hash/rte_thash.h
  index a6ddb7bf7..adbaf8f70 100644
  --- a/lib/librte_hash/rte_thash.h
  +++ b/lib/librte_hash/rte_thash.h
  @@ -168,7 +168,7 @@ rte_convert_rss_key(const uint32_t *orig, uint32_t *targ, int len)
    *   Pointer to rte_ipv6_tuple structure
    */
   static inline void
  -rte_thash_load_v6_addrs(const struct ipv6_hdr *orig, union rte_thash_tuple *targ)
  +rte_thash_load_v6_addrs(const struct rte_ipv6_hdr *orig, union rte_thash_tuple *targ)
   {
   #ifdef RTE_ARCH_X86
   	__m128i ipv6 = _mm_loadu_si128((const __m128i *)orig->src_addr);
  diff --git a/lib/librte_ip_frag/rte_ip_frag.h b/lib/librte_ip_frag/rte_ip_frag.h
  index 7f425f610..28ba33dac 100644
  --- a/lib/librte_ip_frag/rte_ip_frag.h
  +++ b/lib/librte_ip_frag/rte_ip_frag.h
  @@ -210,7 +210,7 @@ rte_ipv6_fragment_packet(struct rte_mbuf *pkt_in,
    */
   struct rte_mbuf *rte_ipv6_frag_reassemble_packet(struct rte_ip_frag_tbl *tbl,
   		struct rte_ip_frag_death_row *dr,
  -		struct rte_mbuf *mb, uint64_t tms, struct ipv6_hdr *ip_hdr,
  +		struct rte_mbuf *mb, uint64_t tms, struct rte_ipv6_hdr *ip_hdr,
   		struct ipv6_extension_fragment *frag_hdr);
   
   /**
  @@ -225,7 +225,7 @@ struct rte_mbuf *rte_ipv6_frag_reassemble_packet(struct rte_ip_frag_tbl *tbl,
    *   present.
    */
   static inline struct ipv6_extension_fragment *
  -rte_ipv6_frag_get_ipv6_fragment_header(struct ipv6_hdr *hdr)
  +rte_ipv6_frag_get_ipv6_fragment_header(struct rte_ipv6_hdr *hdr)
   {
   	if (hdr->proto == IPPROTO_FRAGMENT) {
   		return (struct ipv6_extension_fragment *) ++hdr;
  @@ -284,7 +284,7 @@ int32_t rte_ipv4_fragment_packet(struct rte_mbuf *pkt_in,
    */
   struct rte_mbuf * rte_ipv4_frag_reassemble_packet(struct rte_ip_frag_tbl *tbl,
   		struct rte_ip_frag_death_row *dr,
  -		struct rte_mbuf *mb, uint64_t tms, struct ipv4_hdr *ip_hdr);
  +		struct rte_mbuf *mb, uint64_t tms, struct rte_ipv4_hdr *ip_hdr);
   
   /**
    * Check if the IPv4 packet is fragmented
  @@ -295,12 +295,12 @@ struct rte_mbuf * rte_ipv4_frag_reassemble_packet(struct rte_ip_frag_tbl *tbl,
    *   1 if fragmented, 0 if not fragmented
    */
   static inline int
  -rte_ipv4_frag_pkt_is_fragmented(const struct ipv4_hdr * hdr) {
  +rte_ipv4_frag_pkt_is_fragmented(const struct rte_ipv4_hdr * hdr) {
   	uint16_t flag_offset, ip_flag, ip_ofs;
   
   	flag_offset = rte_be_to_cpu_16(hdr->fragment_offset);
  -	ip_ofs = (uint16_t)(flag_offset & IPV4_HDR_OFFSET_MASK);
  -	ip_flag = (uint16_t)(flag_offset & IPV4_HDR_MF_FLAG);
  +	ip_ofs = (uint16_t)(flag_offset & RTE_IPV4_HDR_OFFSET_MASK);
  +	ip_flag = (uint16_t)(flag_offset & RTE_IPV4_HDR_MF_FLAG);
   
   	return ip_flag != 0 || ip_ofs  != 0;
   }
  diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h
  index 601abdfc6..ce86b19a2 100644
  --- a/lib/librte_kni/rte_kni.h
  +++ b/lib/librte_kni/rte_kni.h
  @@ -68,7 +68,7 @@ struct rte_kni_conf {
   
   	__extension__
   	uint8_t force_bind : 1; /* Flag to bind kernel thread */
  -	char mac_addr[ETHER_ADDR_LEN]; /* MAC address assigned to KNI */
  +	char mac_addr[RTE_ETHER_ADDR_LEN]; /* MAC address assigned to KNI */
   	uint16_t mtu;
   };
   
  diff --git a/lib/librte_pipeline/rte_table_action.h b/lib/librte_pipeline/rte_table_action.h
  index c96061291..400bd5e2c 100644
  --- a/lib/librte_pipeline/rte_table_action.h
  +++ b/lib/librte_pipeline/rte_table_action.h
  @@ -384,8 +384,8 @@ enum rte_table_action_encap_type {
   
   /** Pre-computed Ethernet header fields for encapsulation action. */
   struct rte_table_action_ether_hdr {
  -	struct ether_addr da; /**< Destination address. */
  -	struct ether_addr sa; /**< Source address. */
  +	struct rte_ether_addr da; /**< Destination address. */
  +	struct rte_ether_addr sa; /**< Source address. */
   };
   
   /** Pre-computed VLAN header fields for encapsulation action. */
  diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
  index b4abad30c..064ebb951 100644
  --- a/lib/librte_vhost/vhost.h
  +++ b/lib/librte_vhost/vhost.h
  @@ -356,7 +356,7 @@ struct virtio_net {
   	uint64_t		log_size;
   	uint64_t		log_base;
   	uint64_t		log_addr;
  -	struct ether_addr	mac;
  +	struct rte_ether_addr	mac;
   	uint16_t		mtu;
   
   	struct vhost_device_ops const *notify_ops;
  
Ferruh Yigit Dec. 20, 2018, 9:59 p.m. UTC | #11
On 10/24/2018 9:18 AM, olivier.matz at 6wind.com (Olivier Matz) wrote:
> This RFC targets 19.02.
> 
> The rte_net headers conflict with the libc headers, because
> some definitions are duplicated, sometimes with few differences.
> This was discussed in [1], and more recently at the techboard.
> 
> Before sending the deprecation notice (target for this is 18.11),
> here is a draft that can be discussed.
> 
> This RFC 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.
> 
> One question I'm asking is how can we manage the transition.
> Initially, I hoped it was possible to have a compat layer during
> one release (supporting both prefixed and unprefixed names), but
> now that the patch is done, it seems the impact is too big, and
> impacts too many libraries.
> 
> Few examples:
>   - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *)
>   - many rte_flow structures use the rte_ prefixed net structures
>   - the mac field of virtio_net structure is rte_ether_addr
>   - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *)
>   ...
> 
> Therefore, it is clear that doing this would break the compilation
> of many external applications.
> 
> Another drawback we need to take in account: it will make the
> backport of patches more difficult, although this is something
> that could be tempered by a script.
> 
> While it is obviously better to have a good namespace convention, 
> we need to identify the issues we have today before deciding it's
> worth doing the change.
> 
> Comments?

Is there an consensus about the patchset? There was a decision on techboard to
go with this change (adding rte_ prefix) [1].


This is something that will affect DPDK consumers. Since many APIs are changing
most probably will break API compatibility for many libraries.

Meanwhile the conflict with the libc headers mentioned a few times in the past,
this is something we need to fix.

There are a few comments reluctant to this big modification, but what I
understand from Olivier's response both using BSD defines or having
compatibility headers in DPDK won't solve the problem completely.

And assuming we will continue with this solution, another question is do we
still want to push in 19.02 scope? (And from process point of view I think a
deprecation notice is not merged for this change in 18.11 scope.)

With the prediction of 19.05 will be big and already break API/ABI for some
libraries, can we push this into 19.05 as an early merge into repo?

And I think this patch will affect LTS releases and will break auto backporting
for many fixes because it touches many places, so pushing this change even to
next LTS (19.11) can be an option.


Olivier, Thomas,

What do you think about postponing this to 19.05 or even 19.11 ?



[1]
https://mails.dpdk.org/archives/dev/2018-October/116695.html

> 
> 
> Things that are missing in RFC:
> - test with FreeBSD
> - manually fix some indentation issues
> 
> 
> Olivier Matz (14):
>   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
  
Stephen Hemminger Dec. 20, 2018, 11:48 p.m. UTC | #12
On Thu, 20 Dec 2018 21:59:37 +0000
Ferruh Yigit <ferruh.yigit@intel.com> wrote:

> On 10/24/2018 9:18 AM, olivier.matz at 6wind.com (Olivier Matz) wrote:
> > This RFC targets 19.02.
> > 
> > The rte_net headers conflict with the libc headers, because
> > some definitions are duplicated, sometimes with few differences.
> > This was discussed in [1], and more recently at the techboard.
> > 
> > Before sending the deprecation notice (target for this is 18.11),
> > here is a draft that can be discussed.
> > 
> > This RFC 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.
> > 
> > One question I'm asking is how can we manage the transition.
> > Initially, I hoped it was possible to have a compat layer during
> > one release (supporting both prefixed and unprefixed names), but
> > now that the patch is done, it seems the impact is too big, and
> > impacts too many libraries.
> > 
> > Few examples:
> >   - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *)
> >   - many rte_flow structures use the rte_ prefixed net structures
> >   - the mac field of virtio_net structure is rte_ether_addr
> >   - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *)
> >   ...
> > 
> > Therefore, it is clear that doing this would break the compilation
> > of many external applications.
> > 
> > Another drawback we need to take in account: it will make the
> > backport of patches more difficult, although this is something
> > that could be tempered by a script.
> > 
> > While it is obviously better to have a good namespace convention, 
> > we need to identify the issues we have today before deciding it's
> > worth doing the change.
> > 
> > Comments?  
> 
> Is there an consensus about the patchset? There was a decision on techboard to
> go with this change (adding rte_ prefix) [1].
> 
> 
> This is something that will affect DPDK consumers. Since many APIs are changing
> most probably will break API compatibility for many libraries.
> 
> Meanwhile the conflict with the libc headers mentioned a few times in the past,
> this is something we need to fix.
> 
> There are a few comments reluctant to this big modification, but what I
> understand from Olivier's response both using BSD defines or having
> compatibility headers in DPDK won't solve the problem completely.
> 
> And assuming we will continue with this solution, another question is do we
> still want to push in 19.02 scope? (And from process point of view I think a
> deprecation notice is not merged for this change in 18.11 scope.)
> 
> With the prediction of 19.05 will be big and already break API/ABI for some
> libraries, can we push this into 19.05 as an early merge into repo?
> 
> And I think this patch will affect LTS releases and will break auto backporting
> for many fixes because it touches many places, so pushing this change even to
> next LTS (19.11) can be an option.
> 
> 
> Olivier, Thomas,
> 
> What do you think about postponing this to 19.05 or even 19.11 ?
> 
> 
> 
> [1]
> https://mails.dpdk.org/archives/dev/2018-October/116695.html
> 
> > 
> > 
> > Things that are missing in RFC:
> > - test with FreeBSD
> > - manually fix some indentation issues
> > 
> > 
> > Olivier Matz (14):
> >   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  
> 
> 

Sigh. Another case where DPDK has to reinvent something because
we can't figure out how to do dependent libraries correctly.
I would have rather just using the existing Linux, BSD definitions
and fixing the DPDK code.

It is probably the only viable compromise, but it adds to long
term maintenance, and breaks DPDK applications. Neither of which
is a good thing.

Should this be done by marking the old structure deprecated
first? Ideally, spread over three releases: first, tell the users
in the documentation it is coming; second, mark the old structures
as deprecated causing compiler warnings; third, remove the old
definitions.  Doing at once is introducing user pain for no gain.
  
Wiles, Keith Dec. 21, 2018, 2:38 p.m. UTC | #13
> On Dec 20, 2018, at 5:48 PM, Stephen Hemminger <stephen@networkplumber.org> wrote:
> 
> On Thu, 20 Dec 2018 21:59:37 +0000
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
>> On 10/24/2018 9:18 AM, olivier.matz at 6wind.com (Olivier Matz) wrote:
>>> This RFC targets 19.02.
>>> 
>>> The rte_net headers conflict with the libc headers, because
>>> some definitions are duplicated, sometimes with few differences.
>>> This was discussed in [1], and more recently at the techboard.
>>> 
>>> Before sending the deprecation notice (target for this is 18.11),
>>> here is a draft that can be discussed.
>>> 
>>> This RFC 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.
>>> 
>>> One question I'm asking is how can we manage the transition.
>>> Initially, I hoped it was possible to have a compat layer during
>>> one release (supporting both prefixed and unprefixed names), but
>>> now that the patch is done, it seems the impact is too big, and
>>> impacts too many libraries.
>>> 
>>> Few examples:
>>>  - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *)
>>>  - many rte_flow structures use the rte_ prefixed net structures
>>>  - the mac field of virtio_net structure is rte_ether_addr
>>>  - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *)
>>>  ...
>>> 
>>> Therefore, it is clear that doing this would break the compilation
>>> of many external applications.
>>> 
>>> Another drawback we need to take in account: it will make the
>>> backport of patches more difficult, although this is something
>>> that could be tempered by a script.
>>> 
>>> While it is obviously better to have a good namespace convention, 
>>> we need to identify the issues we have today before deciding it's
>>> worth doing the change.
>>> 
>>> Comments?  
>> 
>> Is there an consensus about the patchset? There was a decision on techboard to
>> go with this change (adding rte_ prefix) [1].
>> 
>> 
>> This is something that will affect DPDK consumers. Since many APIs are changing
>> most probably will break API compatibility for many libraries.
>> 
>> Meanwhile the conflict with the libc headers mentioned a few times in the past,
>> this is something we need to fix.
>> 
>> There are a few comments reluctant to this big modification, but what I
>> understand from Olivier's response both using BSD defines or having
>> compatibility headers in DPDK won't solve the problem completely.
>> 
>> And assuming we will continue with this solution, another question is do we
>> still want to push in 19.02 scope? (And from process point of view I think a
>> deprecation notice is not merged for this change in 18.11 scope.)
>> 
>> With the prediction of 19.05 will be big and already break API/ABI for some
>> libraries, can we push this into 19.05 as an early merge into repo?
>> 
>> And I think this patch will affect LTS releases and will break auto backporting
>> for many fixes because it touches many places, so pushing this change even to
>> next LTS (19.11) can be an option.
>> 
>> 
>> Olivier, Thomas,
>> 
>> What do you think about postponing this to 19.05 or even 19.11 ?
>> 
>> 
>> 
>> [1]
>> https://mails.dpdk.org/archives/dev/2018-October/116695.html
>> 
>>> 
>>> 
>>> Things that are missing in RFC:
>>> - test with FreeBSD
>>> - manually fix some indentation issues
>>> 
>>> 
>>> Olivier Matz (14):
>>>  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  
>> 
>> 
> 
> Sigh. Another case where DPDK has to reinvent something because
> we can't figure out how to do dependent libraries correctly.
> I would have rather just using the existing Linux, BSD definitions
> and fixing the DPDK code.
> 
> It is probably the only viable compromise, but it adds to long
> term maintenance, and breaks DPDK applications. Neither of which
> is a good thing.
> 
> Should this be done by marking the old structure deprecated
> first? Ideally, spread over three releases: first, tell the users
> in the documentation it is coming; second, mark the old structures
> as deprecated causing compiler warnings; third, remove the old
> definitions.  Doing at once is introducing user pain for no gain.

+1

Regards,
Keith
  
Ferruh Yigit Dec. 21, 2018, 3:14 p.m. UTC | #14
On 12/21/2018 2:38 PM, Wiles, Keith wrote:
> 
> 
>> On Dec 20, 2018, at 5:48 PM, Stephen Hemminger <stephen@networkplumber.org> wrote:
>>
>> On Thu, 20 Dec 2018 21:59:37 +0000
>> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>>> On 10/24/2018 9:18 AM, olivier.matz at 6wind.com (Olivier Matz) wrote:
>>>> This RFC targets 19.02.
>>>>
>>>> The rte_net headers conflict with the libc headers, because
>>>> some definitions are duplicated, sometimes with few differences.
>>>> This was discussed in [1], and more recently at the techboard.
>>>>
>>>> Before sending the deprecation notice (target for this is 18.11),
>>>> here is a draft that can be discussed.
>>>>
>>>> This RFC 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.
>>>>
>>>> One question I'm asking is how can we manage the transition.
>>>> Initially, I hoped it was possible to have a compat layer during
>>>> one release (supporting both prefixed and unprefixed names), but
>>>> now that the patch is done, it seems the impact is too big, and
>>>> impacts too many libraries.
>>>>
>>>> Few examples:
>>>>  - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *)
>>>>  - many rte_flow structures use the rte_ prefixed net structures
>>>>  - the mac field of virtio_net structure is rte_ether_addr
>>>>  - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *)
>>>>  ...
>>>>
>>>> Therefore, it is clear that doing this would break the compilation
>>>> of many external applications.
>>>>
>>>> Another drawback we need to take in account: it will make the
>>>> backport of patches more difficult, although this is something
>>>> that could be tempered by a script.
>>>>
>>>> While it is obviously better to have a good namespace convention, 
>>>> we need to identify the issues we have today before deciding it's
>>>> worth doing the change.
>>>>
>>>> Comments?  
>>>
>>> Is there an consensus about the patchset? There was a decision on techboard to
>>> go with this change (adding rte_ prefix) [1].
>>>
>>>
>>> This is something that will affect DPDK consumers. Since many APIs are changing
>>> most probably will break API compatibility for many libraries.
>>>
>>> Meanwhile the conflict with the libc headers mentioned a few times in the past,
>>> this is something we need to fix.
>>>
>>> There are a few comments reluctant to this big modification, but what I
>>> understand from Olivier's response both using BSD defines or having
>>> compatibility headers in DPDK won't solve the problem completely.
>>>
>>> And assuming we will continue with this solution, another question is do we
>>> still want to push in 19.02 scope? (And from process point of view I think a
>>> deprecation notice is not merged for this change in 18.11 scope.)
>>>
>>> With the prediction of 19.05 will be big and already break API/ABI for some
>>> libraries, can we push this into 19.05 as an early merge into repo?
>>>
>>> And I think this patch will affect LTS releases and will break auto backporting
>>> for many fixes because it touches many places, so pushing this change even to
>>> next LTS (19.11) can be an option.
>>>
>>>
>>> Olivier, Thomas,
>>>
>>> What do you think about postponing this to 19.05 or even 19.11 ?
>>>
>>>
>>>
>>> [1]
>>> https://mails.dpdk.org/archives/dev/2018-October/116695.html
>>>
>>>>
>>>>
>>>> Things that are missing in RFC:
>>>> - test with FreeBSD
>>>> - manually fix some indentation issues
>>>>
>>>>
>>>> Olivier Matz (14):
>>>>  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  
>>>
>>>
>>
>> Sigh. Another case where DPDK has to reinvent something because
>> we can't figure out how to do dependent libraries correctly.
>> I would have rather just using the existing Linux, BSD definitions
>> and fixing the DPDK code.
>>
>> It is probably the only viable compromise, but it adds to long
>> term maintenance, and breaks DPDK applications. Neither of which
>> is a good thing.
>>
>> Should this be done by marking the old structure deprecated
>> first? Ideally, spread over three releases: first, tell the users
>> in the documentation it is coming; second, mark the old structures
>> as deprecated causing compiler warnings; third, remove the old
>> definitions.  Doing at once is introducing user pain for no gain.
> 
> +1

With the current timeline, readiness of the patch and comments, at least it
won't able to make this release, I will update the patchset status as 'Deferred'

Should we discuss this again in techboard?
  
Olivier Matz Dec. 27, 2018, 9:35 a.m. UTC | #15
Hi,

On Fri, Dec 21, 2018 at 03:14:29PM +0000, Ferruh Yigit wrote:
> On 12/21/2018 2:38 PM, Wiles, Keith wrote:
> > 
> > 
> >> On Dec 20, 2018, at 5:48 PM, Stephen Hemminger <stephen@networkplumber.org> wrote:
> >>
> >> On Thu, 20 Dec 2018 21:59:37 +0000
> >> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >>
> >>> On 10/24/2018 9:18 AM, olivier.matz at 6wind.com (Olivier Matz) wrote:
> >>>> This RFC targets 19.02.
> >>>>
> >>>> The rte_net headers conflict with the libc headers, because
> >>>> some definitions are duplicated, sometimes with few differences.
> >>>> This was discussed in [1], and more recently at the techboard.
> >>>>
> >>>> Before sending the deprecation notice (target for this is 18.11),
> >>>> here is a draft that can be discussed.
> >>>>
> >>>> This RFC 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.
> >>>>
> >>>> One question I'm asking is how can we manage the transition.
> >>>> Initially, I hoped it was possible to have a compat layer during
> >>>> one release (supporting both prefixed and unprefixed names), but
> >>>> now that the patch is done, it seems the impact is too big, and
> >>>> impacts too many libraries.
> >>>>
> >>>> Few examples:
> >>>>  - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *)
> >>>>  - many rte_flow structures use the rte_ prefixed net structures
> >>>>  - the mac field of virtio_net structure is rte_ether_addr
> >>>>  - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *)
> >>>>  ...
> >>>>
> >>>> Therefore, it is clear that doing this would break the compilation
> >>>> of many external applications.
> >>>>
> >>>> Another drawback we need to take in account: it will make the
> >>>> backport of patches more difficult, although this is something
> >>>> that could be tempered by a script.
> >>>>
> >>>> While it is obviously better to have a good namespace convention, 
> >>>> we need to identify the issues we have today before deciding it's
> >>>> worth doing the change.
> >>>>
> >>>> Comments?  
> >>>
> >>> Is there an consensus about the patchset? There was a decision on techboard to
> >>> go with this change (adding rte_ prefix) [1].
> >>>
> >>>
> >>> This is something that will affect DPDK consumers. Since many APIs are changing
> >>> most probably will break API compatibility for many libraries.
> >>>
> >>> Meanwhile the conflict with the libc headers mentioned a few times in the past,
> >>> this is something we need to fix.
> >>>
> >>> There are a few comments reluctant to this big modification, but what I
> >>> understand from Olivier's response both using BSD defines or having
> >>> compatibility headers in DPDK won't solve the problem completely.
> >>>
> >>> And assuming we will continue with this solution, another question is do we
> >>> still want to push in 19.02 scope? (And from process point of view I think a
> >>> deprecation notice is not merged for this change in 18.11 scope.)
> >>>
> >>> With the prediction of 19.05 will be big and already break API/ABI for some
> >>> libraries, can we push this into 19.05 as an early merge into repo?
> >>>
> >>> And I think this patch will affect LTS releases and will break auto backporting
> >>> for many fixes because it touches many places, so pushing this change even to
> >>> next LTS (19.11) can be an option.
> >>>
> >>>
> >>> Olivier, Thomas,
> >>>
> >>> What do you think about postponing this to 19.05 or even 19.11 ?
> >>>
> >>>
> >>>
> >>> [1]
> >>> https://mails.dpdk.org/archives/dev/2018-October/116695.html
> >>>
> >>>>
> >>>>
> >>>> Things that are missing in RFC:
> >>>> - test with FreeBSD
> >>>> - manually fix some indentation issues
> >>>>
> >>>>
> >>>> Olivier Matz (14):
> >>>>  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  
> >>>
> >>>
> >>
> >> Sigh. Another case where DPDK has to reinvent something because
> >> we can't figure out how to do dependent libraries correctly.
> >> I would have rather just using the existing Linux, BSD definitions
> >> and fixing the DPDK code.


It is not that simple. As I said in [1], there are still some
differences between gnu libc and freebsd libc. Unfortunatly, the struct
ether_addr is one of the most important in dpdk, because it is widely
used in APIs (drivers).

We can find others differences, for instance in constant definitions in
if_arp.h. I also see that some structures are packed in freebsd but not
in glibc (ex: icmp6), this could have performance impact.

Many protocols that are currently defined in dpdk are missing in glibc:
esp, sctp, gre, mpls, ... so we will at least need rte_ structures for
these protocols.

Supporting other OSes or libc in the future could also increase the gaps.

For these reasons think it is reasonable to have a consistent set of
network structures in dpdk.


[1] https://mails.dpdk.org/archives/dev/2018-October/117258.html


> >> It is probably the only viable compromise, but it adds to long
> >> term maintenance, and breaks DPDK applications. Neither of which
> >> is a good thing.
> >>
> >> Should this be done by marking the old structure deprecated
> >> first? Ideally, spread over three releases: first, tell the users
> >> in the documentation it is coming; second, mark the old structures
> >> as deprecated causing compiler warnings; third, remove the old
> >> definitions.  Doing at once is introducing user pain for no gain.
> > 
> > +1

Annoucing the change before doing it is obvious. Marking the old
structures as deprecated before removing them is maybe doable (to be
checked that it does not cause conflicts with new structures), but it
means the conflict with libc headers that we are trying to solve will
remain for one more version, for a limited gain.

> With the current timeline, readiness of the patch and comments, at least it
> won't able to make this release, I will update the patchset status as 'Deferred'
> 
> Should we discuss this again in techboard?

We should surely weigh the pros and cons. Especially the additional
backport troubles it can bring.

Are many people bothered by the current conflict with the libc headers?

Olivier
  
Ferruh Yigit Feb. 13, 2019, 11:48 a.m. UTC | #16
On 12/27/2018 9:35 AM, Olivier Matz wrote:
> Hi,
> 
> On Fri, Dec 21, 2018 at 03:14:29PM +0000, Ferruh Yigit wrote:
>> On 12/21/2018 2:38 PM, Wiles, Keith wrote:
>>>
>>>
>>>> On Dec 20, 2018, at 5:48 PM, Stephen Hemminger <stephen@networkplumber.org> wrote:
>>>>
>>>> On Thu, 20 Dec 2018 21:59:37 +0000
>>>> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>>
>>>>> On 10/24/2018 9:18 AM, olivier.matz at 6wind.com (Olivier Matz) wrote:
>>>>>> This RFC targets 19.02.
>>>>>>
>>>>>> The rte_net headers conflict with the libc headers, because
>>>>>> some definitions are duplicated, sometimes with few differences.
>>>>>> This was discussed in [1], and more recently at the techboard.
>>>>>>
>>>>>> Before sending the deprecation notice (target for this is 18.11),
>>>>>> here is a draft that can be discussed.
>>>>>>
>>>>>> This RFC 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.
>>>>>>
>>>>>> One question I'm asking is how can we manage the transition.
>>>>>> Initially, I hoped it was possible to have a compat layer during
>>>>>> one release (supporting both prefixed and unprefixed names), but
>>>>>> now that the patch is done, it seems the impact is too big, and
>>>>>> impacts too many libraries.
>>>>>>
>>>>>> Few examples:
>>>>>>  - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *)
>>>>>>  - many rte_flow structures use the rte_ prefixed net structures
>>>>>>  - the mac field of virtio_net structure is rte_ether_addr
>>>>>>  - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *)
>>>>>>  ...
>>>>>>
>>>>>> Therefore, it is clear that doing this would break the compilation
>>>>>> of many external applications.
>>>>>>
>>>>>> Another drawback we need to take in account: it will make the
>>>>>> backport of patches more difficult, although this is something
>>>>>> that could be tempered by a script.
>>>>>>
>>>>>> While it is obviously better to have a good namespace convention, 
>>>>>> we need to identify the issues we have today before deciding it's
>>>>>> worth doing the change.
>>>>>>
>>>>>> Comments?  
>>>>>
>>>>> Is there an consensus about the patchset? There was a decision on techboard to
>>>>> go with this change (adding rte_ prefix) [1].
>>>>>
>>>>>
>>>>> This is something that will affect DPDK consumers. Since many APIs are changing
>>>>> most probably will break API compatibility for many libraries.
>>>>>
>>>>> Meanwhile the conflict with the libc headers mentioned a few times in the past,
>>>>> this is something we need to fix.
>>>>>
>>>>> There are a few comments reluctant to this big modification, but what I
>>>>> understand from Olivier's response both using BSD defines or having
>>>>> compatibility headers in DPDK won't solve the problem completely.
>>>>>
>>>>> And assuming we will continue with this solution, another question is do we
>>>>> still want to push in 19.02 scope? (And from process point of view I think a
>>>>> deprecation notice is not merged for this change in 18.11 scope.)
>>>>>
>>>>> With the prediction of 19.05 will be big and already break API/ABI for some
>>>>> libraries, can we push this into 19.05 as an early merge into repo?
>>>>>
>>>>> And I think this patch will affect LTS releases and will break auto backporting
>>>>> for many fixes because it touches many places, so pushing this change even to
>>>>> next LTS (19.11) can be an option.
>>>>>
>>>>>
>>>>> Olivier, Thomas,
>>>>>
>>>>> What do you think about postponing this to 19.05 or even 19.11 ?
>>>>>
>>>>>
>>>>>
>>>>> [1]
>>>>> https://mails.dpdk.org/archives/dev/2018-October/116695.html
>>>>>
>>>>>>
>>>>>>
>>>>>> Things that are missing in RFC:
>>>>>> - test with FreeBSD
>>>>>> - manually fix some indentation issues
>>>>>>
>>>>>>
>>>>>> Olivier Matz (14):
>>>>>>  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  
>>>>>
>>>>>
>>>>
>>>> Sigh. Another case where DPDK has to reinvent something because
>>>> we can't figure out how to do dependent libraries correctly.
>>>> I would have rather just using the existing Linux, BSD definitions
>>>> and fixing the DPDK code.
> 
> 
> It is not that simple. As I said in [1], there are still some
> differences between gnu libc and freebsd libc. Unfortunatly, the struct
> ether_addr is one of the most important in dpdk, because it is widely
> used in APIs (drivers).
> 
> We can find others differences, for instance in constant definitions in
> if_arp.h. I also see that some structures are packed in freebsd but not
> in glibc (ex: icmp6), this could have performance impact.
> 
> Many protocols that are currently defined in dpdk are missing in glibc:
> esp, sctp, gre, mpls, ... so we will at least need rte_ structures for
> these protocols.
> 
> Supporting other OSes or libc in the future could also increase the gaps.
> 
> For these reasons think it is reasonable to have a consistent set of
> network structures in dpdk.
> 
> 
> [1] https://mails.dpdk.org/archives/dev/2018-October/117258.html
> 
> 
>>>> It is probably the only viable compromise, but it adds to long
>>>> term maintenance, and breaks DPDK applications. Neither of which
>>>> is a good thing.
>>>>
>>>> Should this be done by marking the old structure deprecated
>>>> first? Ideally, spread over three releases: first, tell the users
>>>> in the documentation it is coming; second, mark the old structures
>>>> as deprecated causing compiler warnings; third, remove the old
>>>> definitions.  Doing at once is introducing user pain for no gain.
>>>
>>> +1
> 
> Annoucing the change before doing it is obvious. Marking the old
> structures as deprecated before removing them is maybe doable (to be
> checked that it does not cause conflicts with new structures), but it
> means the conflict with libc headers that we are trying to solve will
> remain for one more version, for a limited gain.
> 
>> With the current timeline, readiness of the patch and comments, at least it
>> won't able to make this release, I will update the patchset status as 'Deferred'
>>
>> Should we discuss this again in techboard?
> 
> We should surely weigh the pros and cons. Especially the additional
> backport troubles it can bring.
> 
> Are many people bothered by the current conflict with the libc headers?

This is still open.

If we will get these patchset, I suggest it getting early in the 19.05, patch is
mechanical but it is huge and will affect almost all other patches under
development. So I am not really for pushing this close to RC.

Is there any way to decide on this week, at worst next week?

Thanks,
ferruh
  
Ferruh Yigit Feb. 18, 2019, 12:37 p.m. UTC | #17
On 2/13/2019 11:48 AM, Yigit, Ferruh wrote:
> On 12/27/2018 9:35 AM, Olivier Matz wrote:
>> Hi,
>>
>> On Fri, Dec 21, 2018 at 03:14:29PM +0000, Ferruh Yigit wrote:
>>> On 12/21/2018 2:38 PM, Wiles, Keith wrote:
>>>>
>>>>
>>>>> On Dec 20, 2018, at 5:48 PM, Stephen Hemminger <stephen@networkplumber.org> wrote:
>>>>>
>>>>> On Thu, 20 Dec 2018 21:59:37 +0000
>>>>> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>>>
>>>>>> On 10/24/2018 9:18 AM, olivier.matz at 6wind.com (Olivier Matz) wrote:
>>>>>>> This RFC targets 19.02.
>>>>>>>
>>>>>>> The rte_net headers conflict with the libc headers, because
>>>>>>> some definitions are duplicated, sometimes with few differences.
>>>>>>> This was discussed in [1], and more recently at the techboard.
>>>>>>>
>>>>>>> Before sending the deprecation notice (target for this is 18.11),
>>>>>>> here is a draft that can be discussed.
>>>>>>>
>>>>>>> This RFC 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.
>>>>>>>
>>>>>>> One question I'm asking is how can we manage the transition.
>>>>>>> Initially, I hoped it was possible to have a compat layer during
>>>>>>> one release (supporting both prefixed and unprefixed names), but
>>>>>>> now that the patch is done, it seems the impact is too big, and
>>>>>>> impacts too many libraries.
>>>>>>>
>>>>>>> Few examples:
>>>>>>>  - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *)
>>>>>>>  - many rte_flow structures use the rte_ prefixed net structures
>>>>>>>  - the mac field of virtio_net structure is rte_ether_addr
>>>>>>>  - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *)
>>>>>>>  ...
>>>>>>>
>>>>>>> Therefore, it is clear that doing this would break the compilation
>>>>>>> of many external applications.
>>>>>>>
>>>>>>> Another drawback we need to take in account: it will make the
>>>>>>> backport of patches more difficult, although this is something
>>>>>>> that could be tempered by a script.
>>>>>>>
>>>>>>> While it is obviously better to have a good namespace convention, 
>>>>>>> we need to identify the issues we have today before deciding it's
>>>>>>> worth doing the change.
>>>>>>>
>>>>>>> Comments?  
>>>>>>
>>>>>> Is there an consensus about the patchset? There was a decision on techboard to
>>>>>> go with this change (adding rte_ prefix) [1].
>>>>>>
>>>>>>
>>>>>> This is something that will affect DPDK consumers. Since many APIs are changing
>>>>>> most probably will break API compatibility for many libraries.
>>>>>>
>>>>>> Meanwhile the conflict with the libc headers mentioned a few times in the past,
>>>>>> this is something we need to fix.
>>>>>>
>>>>>> There are a few comments reluctant to this big modification, but what I
>>>>>> understand from Olivier's response both using BSD defines or having
>>>>>> compatibility headers in DPDK won't solve the problem completely.
>>>>>>
>>>>>> And assuming we will continue with this solution, another question is do we
>>>>>> still want to push in 19.02 scope? (And from process point of view I think a
>>>>>> deprecation notice is not merged for this change in 18.11 scope.)
>>>>>>
>>>>>> With the prediction of 19.05 will be big and already break API/ABI for some
>>>>>> libraries, can we push this into 19.05 as an early merge into repo?
>>>>>>
>>>>>> And I think this patch will affect LTS releases and will break auto backporting
>>>>>> for many fixes because it touches many places, so pushing this change even to
>>>>>> next LTS (19.11) can be an option.
>>>>>>
>>>>>>
>>>>>> Olivier, Thomas,
>>>>>>
>>>>>> What do you think about postponing this to 19.05 or even 19.11 ?
>>>>>>
>>>>>>
>>>>>>
>>>>>> [1]
>>>>>> https://mails.dpdk.org/archives/dev/2018-October/116695.html
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Things that are missing in RFC:
>>>>>>> - test with FreeBSD
>>>>>>> - manually fix some indentation issues
>>>>>>>
>>>>>>>
>>>>>>> Olivier Matz (14):
>>>>>>>  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  
>>>>>>
>>>>>>
>>>>>
>>>>> Sigh. Another case where DPDK has to reinvent something because
>>>>> we can't figure out how to do dependent libraries correctly.
>>>>> I would have rather just using the existing Linux, BSD definitions
>>>>> and fixing the DPDK code.
>>
>>
>> It is not that simple. As I said in [1], there are still some
>> differences between gnu libc and freebsd libc. Unfortunatly, the struct
>> ether_addr is one of the most important in dpdk, because it is widely
>> used in APIs (drivers).
>>
>> We can find others differences, for instance in constant definitions in
>> if_arp.h. I also see that some structures are packed in freebsd but not
>> in glibc (ex: icmp6), this could have performance impact.
>>
>> Many protocols that are currently defined in dpdk are missing in glibc:
>> esp, sctp, gre, mpls, ... so we will at least need rte_ structures for
>> these protocols.
>>
>> Supporting other OSes or libc in the future could also increase the gaps.
>>
>> For these reasons think it is reasonable to have a consistent set of
>> network structures in dpdk.
>>
>>
>> [1] https://mails.dpdk.org/archives/dev/2018-October/117258.html
>>
>>
>>>>> It is probably the only viable compromise, but it adds to long
>>>>> term maintenance, and breaks DPDK applications. Neither of which
>>>>> is a good thing.
>>>>>
>>>>> Should this be done by marking the old structure deprecated
>>>>> first? Ideally, spread over three releases: first, tell the users
>>>>> in the documentation it is coming; second, mark the old structures
>>>>> as deprecated causing compiler warnings; third, remove the old
>>>>> definitions.  Doing at once is introducing user pain for no gain.
>>>>
>>>> +1
>>
>> Annoucing the change before doing it is obvious. Marking the old
>> structures as deprecated before removing them is maybe doable (to be
>> checked that it does not cause conflicts with new structures), but it
>> means the conflict with libc headers that we are trying to solve will
>> remain for one more version, for a limited gain.
>>
>>> With the current timeline, readiness of the patch and comments, at least it
>>> won't able to make this release, I will update the patchset status as 'Deferred'
>>>
>>> Should we discuss this again in techboard?
>>
>> We should surely weigh the pros and cons. Especially the additional
>> backport troubles it can bring.
>>
>> Are many people bothered by the current conflict with the libc headers?
> 
> This is still open.
> 
> If we will get these patchset, I suggest it getting early in the 19.05, patch is
> mechanical but it is huge and will affect almost all other patches under
> development. So I am not really for pushing this close to RC.
> 
> Is there any way to decide on this week, at worst next week?

This has been discussed in techboard meeting and decided to go with this patch.
But we are missing the deprecation notice for this.


Olivier,

Can you send a deprecation notice for this in the scope of the 19.05?
And can we target the actual patch for very early days of the 19.08?

Thanks,
ferruh
  
Olivier Matz Feb. 18, 2019, 4:58 p.m. UTC | #18
On Mon, Feb 18, 2019 at 12:37:41PM +0000, Ferruh Yigit wrote:
> On 2/13/2019 11:48 AM, Yigit, Ferruh wrote:
> > On 12/27/2018 9:35 AM, Olivier Matz wrote:
> >> Hi,
> >>
> >> On Fri, Dec 21, 2018 at 03:14:29PM +0000, Ferruh Yigit wrote:
> >>> On 12/21/2018 2:38 PM, Wiles, Keith wrote:
> >>>>
> >>>>
> >>>>> On Dec 20, 2018, at 5:48 PM, Stephen Hemminger <stephen@networkplumber.org> wrote:
> >>>>>
> >>>>> On Thu, 20 Dec 2018 21:59:37 +0000
> >>>>> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >>>>>
> >>>>>> On 10/24/2018 9:18 AM, olivier.matz at 6wind.com (Olivier Matz) wrote:
> >>>>>>> This RFC targets 19.02.
> >>>>>>>
> >>>>>>> The rte_net headers conflict with the libc headers, because
> >>>>>>> some definitions are duplicated, sometimes with few differences.
> >>>>>>> This was discussed in [1], and more recently at the techboard.
> >>>>>>>
> >>>>>>> Before sending the deprecation notice (target for this is 18.11),
> >>>>>>> here is a draft that can be discussed.
> >>>>>>>
> >>>>>>> This RFC 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.
> >>>>>>>
> >>>>>>> One question I'm asking is how can we manage the transition.
> >>>>>>> Initially, I hoped it was possible to have a compat layer during
> >>>>>>> one release (supporting both prefixed and unprefixed names), but
> >>>>>>> now that the patch is done, it seems the impact is too big, and
> >>>>>>> impacts too many libraries.
> >>>>>>>
> >>>>>>> Few examples:
> >>>>>>>  - rte_eth_macaddr_get/add/remove() use a (struct rte_ether_addr *)
> >>>>>>>  - many rte_flow structures use the rte_ prefixed net structures
> >>>>>>>  - the mac field of virtio_net structure is rte_ether_addr
> >>>>>>>  - the first arg of rte_thash_load_v6_addrs is (struct rte_ipv6_hdr *)
> >>>>>>>  ...
> >>>>>>>
> >>>>>>> Therefore, it is clear that doing this would break the compilation
> >>>>>>> of many external applications.
> >>>>>>>
> >>>>>>> Another drawback we need to take in account: it will make the
> >>>>>>> backport of patches more difficult, although this is something
> >>>>>>> that could be tempered by a script.
> >>>>>>>
> >>>>>>> While it is obviously better to have a good namespace convention, 
> >>>>>>> we need to identify the issues we have today before deciding it's
> >>>>>>> worth doing the change.
> >>>>>>>
> >>>>>>> Comments?  
> >>>>>>
> >>>>>> Is there an consensus about the patchset? There was a decision on techboard to
> >>>>>> go with this change (adding rte_ prefix) [1].
> >>>>>>
> >>>>>>
> >>>>>> This is something that will affect DPDK consumers. Since many APIs are changing
> >>>>>> most probably will break API compatibility for many libraries.
> >>>>>>
> >>>>>> Meanwhile the conflict with the libc headers mentioned a few times in the past,
> >>>>>> this is something we need to fix.
> >>>>>>
> >>>>>> There are a few comments reluctant to this big modification, but what I
> >>>>>> understand from Olivier's response both using BSD defines or having
> >>>>>> compatibility headers in DPDK won't solve the problem completely.
> >>>>>>
> >>>>>> And assuming we will continue with this solution, another question is do we
> >>>>>> still want to push in 19.02 scope? (And from process point of view I think a
> >>>>>> deprecation notice is not merged for this change in 18.11 scope.)
> >>>>>>
> >>>>>> With the prediction of 19.05 will be big and already break API/ABI for some
> >>>>>> libraries, can we push this into 19.05 as an early merge into repo?
> >>>>>>
> >>>>>> And I think this patch will affect LTS releases and will break auto backporting
> >>>>>> for many fixes because it touches many places, so pushing this change even to
> >>>>>> next LTS (19.11) can be an option.
> >>>>>>
> >>>>>>
> >>>>>> Olivier, Thomas,
> >>>>>>
> >>>>>> What do you think about postponing this to 19.05 or even 19.11 ?
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> [1]
> >>>>>> https://mails.dpdk.org/archives/dev/2018-October/116695.html
> >>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> Things that are missing in RFC:
> >>>>>>> - test with FreeBSD
> >>>>>>> - manually fix some indentation issues
> >>>>>>>
> >>>>>>>
> >>>>>>> Olivier Matz (14):
> >>>>>>>  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  
> >>>>>>
> >>>>>>
> >>>>>
> >>>>> Sigh. Another case where DPDK has to reinvent something because
> >>>>> we can't figure out how to do dependent libraries correctly.
> >>>>> I would have rather just using the existing Linux, BSD definitions
> >>>>> and fixing the DPDK code.
> >>
> >>
> >> It is not that simple. As I said in [1], there are still some
> >> differences between gnu libc and freebsd libc. Unfortunatly, the struct
> >> ether_addr is one of the most important in dpdk, because it is widely
> >> used in APIs (drivers).
> >>
> >> We can find others differences, for instance in constant definitions in
> >> if_arp.h. I also see that some structures are packed in freebsd but not
> >> in glibc (ex: icmp6), this could have performance impact.
> >>
> >> Many protocols that are currently defined in dpdk are missing in glibc:
> >> esp, sctp, gre, mpls, ... so we will at least need rte_ structures for
> >> these protocols.
> >>
> >> Supporting other OSes or libc in the future could also increase the gaps.
> >>
> >> For these reasons think it is reasonable to have a consistent set of
> >> network structures in dpdk.
> >>
> >>
> >> [1] https://mails.dpdk.org/archives/dev/2018-October/117258.html
> >>
> >>
> >>>>> It is probably the only viable compromise, but it adds to long
> >>>>> term maintenance, and breaks DPDK applications. Neither of which
> >>>>> is a good thing.
> >>>>>
> >>>>> Should this be done by marking the old structure deprecated
> >>>>> first? Ideally, spread over three releases: first, tell the users
> >>>>> in the documentation it is coming; second, mark the old structures
> >>>>> as deprecated causing compiler warnings; third, remove the old
> >>>>> definitions.  Doing at once is introducing user pain for no gain.
> >>>>
> >>>> +1
> >>
> >> Annoucing the change before doing it is obvious. Marking the old
> >> structures as deprecated before removing them is maybe doable (to be
> >> checked that it does not cause conflicts with new structures), but it
> >> means the conflict with libc headers that we are trying to solve will
> >> remain for one more version, for a limited gain.
> >>
> >>> With the current timeline, readiness of the patch and comments, at least it
> >>> won't able to make this release, I will update the patchset status as 'Deferred'
> >>>
> >>> Should we discuss this again in techboard?
> >>
> >> We should surely weigh the pros and cons. Especially the additional
> >> backport troubles it can bring.
> >>
> >> Are many people bothered by the current conflict with the libc headers?
> > 
> > This is still open.
> > 
> > If we will get these patchset, I suggest it getting early in the 19.05, patch is
> > mechanical but it is huge and will affect almost all other patches under
> > development. So I am not really for pushing this close to RC.
> > 
> > Is there any way to decide on this week, at worst next week?
> 
> This has been discussed in techboard meeting and decided to go with this patch.
> But we are missing the deprecation notice for this.
> 
> 
> Olivier,
> 
> Can you send a deprecation notice for this in the scope of the 19.05?
> And can we target the actual patch for very early days of the 19.08?
> 

Hi Ferruh,

OK, will do. Thank you.

Olivier