Re: [PATCH v2 1/3] ethdev: add ICMPv6 ID and sequence

[Thomas Monjalon <thomas@monjalon.net>]

31/01/2023 07:53, Leo Xu (Networking SW):
> From: Thomas Monjalon <thomas@monjalon.net>
> > 20/12/2022 08:44, Leo Xu:
> > > +/**
> > > + * ICMP6 header
> > > + */
> > > +struct rte_icmp6_hdr {
> > > +     uint8_t type;
> > > +     uint8_t code;
> > > +     rte_be16_t checksum;
> > > +} __rte_packed;
> > > +
> > > +/**
> > > + * ICMP6 echo
> > > + */
> > > +struct rte_icmp6_echo {
> > > +     struct rte_icmp6_hdr hdr;
> > > +     rte_be16_t identifier;
> > > +     rte_be16_t sequence;
> > > +} __rte_packed;
> > 
> > It is exactly the same as struct rte_icmp_hdr.
> > Why not reuse it?
> > Maybe introduce struct rte_icmp_base_hdr and define rte_icmp_echo_hdr as
> > rte_icmp_hdr?
> 
> Hi Thomas,
> Looks like, using rte_icmp_hdr as base header for both icmp and icmpv6 is not that good. 
> since, rte_icmp_hdr default their headers always having id and sequence fields, which is not applicable for most other icmp6/icmp types packets.
> 
> I may suggest to keep icmp and icmp6 structures independent against each other, because, looks like these two protocols definitions do not share common base.

What about introducing rte_icmp_base_hdr?
We should try to avoid duplicating things.