> On 19 Jan 2023, at 23:17, Jan Klemkow <j.klem...@wemelug.de> wrote: > > On Thu, Jan 19, 2023 at 02:55:29PM +0300, Vitaliy Makkoveev wrote: >> On Thu, Jan 19, 2023 at 10:40:52AM +0100, Jan Klemkow wrote: >>> On Thu, Jan 19, 2023 at 12:02:29PM +0300, Vitaliy Makkoveev wrote: >>>> On Thu, Jan 19, 2023 at 01:55:57AM +0300, Vitaliy Makkoveev wrote: >>>>>> On 19 Jan 2023, at 01:39, Jan Klemkow <j.klem...@wemelug.de> wrote: >>>>>> On Wed, Jan 18, 2023 at 10:50:25AM +0300, Vitaliy Makkoveev wrote: >>>>>>> On Tue, Jan 17, 2023 at 11:09:17PM +0100, Jan Klemkow wrote: >>>>>>>> we have several drivers which have to parse the content of mbufs. This >>>>>>>> diff suggest a central parsing function for this. Thus, we can reduce >>>>>>>> redundant code. >>>>>>>> >>>>>>>> I just start with ix(4) and ixl(4) because it was easy to test for me. >>>>>>>> But, this could also improve em(4), igc(4), ale(4) and oce(4). >>>>>>>> >>>>>>>> I'm not sure about the name, the api nor the place of this code. So, >>>>>>>> if >>>>>>>> someone has a better idea: i'm open to anything. >>>>>>> >>>>>>> I like code this deduplication. >>>>>>> >>>>>>> This newly introduced function doesn't touch ifnet but only extracts >>>>>>> protocol headers from mbuf(9). I guess mbuf_extract_headers() or >>>>>>> something like is much better for name with the ern/uipc_mbuf2.c as >>>>>>> place. >>>>>> >>>>>> Good Point. Updates diff below. >>>>>> >>>>>> + >>>>>> +/* Parse different TCP/IP protocol headers for a quick view inside an >>>>>> mbuf. */ >>>>>> +void >>>>>> +m_exract_headers(struct mbuf *mp, struct ether_header **eh, struct ip >>>>>> **ip4, >>>>>> + struct ip6_hdr **ip6, struct tcphdr **tcp, struct udphdr **udp) >>>>>> + >>>>> >>>>> Should be m_extract_headers(). The rest of the diff looks good to me. >>>> >>>> Please wait. >>>> >>>> The mandatory nullification of `ip4', `ip6' and other variables passed >>>> to m_exract_headers() is not obvious. It is much better to return >>>> the integer result of extraction like m_tag_copy_chain() does. >>> >>> Yes, the mandatory nullification seems to be more errorprone. In my >>> opinion is the number of results it not that useful. You have to check >>> the retuned pointers anyway. >>> >>> I moved the nullification inside of m_exract_headers(). >> >> This is better. I also like the last return statement be removed from >> m_extract_headers() before commit. > > Fixed below. Plus a suggestion from mpi to not pollute the namespace > with all the headers in mbuf.h. Moved them to uipc_mbuf2.c. >
ok by me
