Hi Thomas
PSB
> 31/01/2023 07:53, Leo Xu (Networking SW):
> > From: Thomas Monjalon <tho...@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.
>
You mean introduce rte_icmp_base_hdr like following?
struct rte_icmp_base_hdr {
uint8_t icmp_type;
uint8_t icmp_code;
rte_be16_t icmp_cksum;
} __rte_packed;
And change the existing rte_icmp_hdr to be:
struct rte_icmp_hdr {
rte_icmp_base_hdr bash_hdr;
rte_be16_t icmp_ident;
rte_be16_t icmp_seq_nb;
} __rte_packed;
#define rte_icmp6_echo struct rte_icmp_hdr;
If it is, there will be some compatibilities issues, since we changed existing
structure.
Or, maybe I'm missing something.
Would you help to give more details about above comment?
